[PATCH,x86] Fix combine for condditional instructions.

[Richard Biener]

On Wed, Dec 12, 2012 at 3:39 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote:
> Guys,
>
> I assume that this is not right way for fixing such simple performance
> anomaly since we need to do redundant work - combine load to
> conditional and then split it back in peephole2? Does it look
> reasonable? Why we should produce non-efficient instrucction that must
> be splitted later?

Well, either don't allow this instruction variant from the start, or allow
the extra freedom for register allocation this creates.  It doesn't make
sense to just reject it being generated by combine - that doesn't address
when it materializes in another way.

Just my 2 cents - I'm not a x86 target maintainer and it's definitely up to them
to decide.

Richard.

> Best regards.
> Yuri.
>
> 2012/12/12 Richard Biener <richard.guenther@gmail.com>:
>> On Wed, Dec 12, 2012 at 1:55 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Wed, Dec 12, 2012 at 12:44 PM, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>
>>>>> This fix is aimed to remove performance degradation introduced by new
>>>>> LRA phase that in fact is combining problem. Gcc combiner does
>>>>> propagation of memory load to if-then-else gimple that was splitted
>>>>> back by old reload phase. LRA does not perform such splitting. To
>>>>> avoid performance slowdown on important benchmark (this is true for
>>>>> all x86 targets) we decided to enhance 'ix86_legitimate_combined_insn'
>>>>> with a check on such propagation and consider such conditional
>>>>> instruction with memory operand as illegal one from performance point
>>>>> of view.
>>>>>
>>>>> The fix was bootstrapped and regtested for x86-64.
>>>>> Is it OK for 4.8 and mainline?
>>>>
>>>> Isn't it a win for -Os though?  Thus, optimize_insn_for_size ()?  It can
>>>> also increase register pressure, no?  So eventually this splitting should
>>>> be done post-reload only.  Not sure what appropriate machinery there is,
>>>> besides from mdreorg (or split itself).
>>>
>>> So, you are proposing to use peephole2 with (match_scratch)
>>> conditional temporary?
>>
>> Yes, if that works.  (sounds backward to me having a peephole split one
>> insn into two ... ;))
>>
>> Richard.
>>
>>> Uros.