[PATCH] D11393: [X86] Allow X86::COND_NE_OR_P and X86::COND_NP_OR_E to be reversed.

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 12 11:07:38 PST 2016


On Tue, Jan 12, 2016 at 11:04 AM, Cong Hou <congh at google.com> wrote:
> On Mon, Jan 11, 2016 at 5:35 PM, Xinliang David Li <davidxl at google.com>
> wrote:
>>
>> On Mon, Jan 11, 2016 at 4:59 PM, Cong Hou <congh at google.com> wrote:
>> > congh added a comment.
>> >
>> > In http://reviews.llvm.org/D11393#323944, @davidxl wrote:
>> >
>> >> Restart discussion on this thread:
>> >>
>> >> For the motivating example where two conditional branches have
>> >> different targets,
>> >>
>> >> jne   .BB1
>> >>  jnp  .BB2
>> >>  .BB1:
>> >>  ...
>> >>  .BB2:
>> >>  ...
>> >>
>> >> Is it possible to teach AnalyzeBranch to recognize the pattern -- with
>> >> opcode COND_NE_OR_P ?
>> >
>> >
>> > Yes, I think this is done in this patch. Or do I misunderstand what you
>> > mean?
>>
>> The patch removes the branch condition reversing optimization in
>> AnalyzeBranch -- is that needed? For the motivating example, no new
>> opcode seems to be needed either ..
>
>
> I think branch condition reversing optimization in AnalyzeBranch is
> unnecessary: we will do it in block-placement anyway. And just as you once
> said, it is a bad idea to modify the branch in a function called
> AnalyzeBranch(), which seems a readonly function.

Yes -- agree with that part. However I think removing transformation
in AnalyzeBranch is an independent issue that should be done
separately.  The main focus of this patch should be to enhance
AnalyzeBranch to recognize more patterns.

David

>
> Cong
>
>>
>>
>> David
>>
>> >
>> >
>> > http://reviews.llvm.org/D11393
>> >
>> >
>> >
>
>


More information about the llvm-commits mailing list