<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jan 12, 2016 at 11:07 AM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Jan 12, 2016 at 11:04 AM, Cong Hou <<a href="mailto:congh@google.com">congh@google.com</a>> wrote:<br>
> On Mon, Jan 11, 2016 at 5:35 PM, Xinliang David Li <<a href="mailto:davidxl@google.com">davidxl@google.com</a>><br>
> wrote:<br>
>><br>
>> On Mon, Jan 11, 2016 at 4:59 PM, Cong Hou <<a href="mailto:congh@google.com">congh@google.com</a>> wrote:<br>
>> > congh added a comment.<br>
>> ><br>
>> > In <a href="http://reviews.llvm.org/D11393#323944" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11393#323944</a>, @davidxl wrote:<br>
>> ><br>
>> >> Restart discussion on this thread:<br>
>> >><br>
>> >> For the motivating example where two conditional branches have<br>
>> >> different targets,<br>
>> >><br>
>> >> jne   .BB1<br>
>> >>  jnp  .BB2<br>
>> >>  .BB1:<br>
>> >>  ...<br>
>> >>  .BB2:<br>
>> >>  ...<br>
>> >><br>
>> >> Is it possible to teach AnalyzeBranch to recognize the pattern -- with<br>
>> >> opcode COND_NE_OR_P ?<br>
>> ><br>
>> ><br>
>> > Yes, I think this is done in this patch. Or do I misunderstand what you<br>
>> > mean?<br>
>><br>
>> The patch removes the branch condition reversing optimization in<br>
>> AnalyzeBranch -- is that needed? For the motivating example, no new<br>
>> opcode seems to be needed either ..<br>
><br>
><br>
> I think branch condition reversing optimization in AnalyzeBranch is<br>
> unnecessary: we will do it in block-placement anyway. And just as you once<br>
> said, it is a bad idea to modify the branch in a function called<br>
> AnalyzeBranch(), which seems a readonly function.<br>
<br>
</div></div>Yes -- agree with that part. However I think removing transformation<br>
in AnalyzeBranch is an independent issue that should be done<br>
separately.  The main focus of this patch should be to enhance<br>
AnalyzeBranch to recognize more patterns.<br></blockquote><div><br></div><div>OK. I can bring back the deleted part. The pattern recognition is still done later in this function. Let me know if I am wrong.</div><div><br></div><div><br></div><div>Cong</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
David<br>
<br>
><br>
> Cong<br>
><br>
>><br>
>><br>
>> David<br>
>><br>
>> ><br>
>> ><br>
>> > <a href="http://reviews.llvm.org/D11393" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11393</a><br>
>> ><br>
>> ><br>
>> ><br>
><br>
><br>
</blockquote></div><br></div></div>