This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Conditional negation elimination in tree-ssa-phiopt.c


On Tue, Aug 12, 2014 at 12:31 PM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>
> On 12/08/14 10:39, Richard Biener wrote:
>>
>> On Mon, Aug 11, 2014 at 9:56 PM, Jeff Law <law@redhat.com> wrote:
>>>
>>> On 08/11/14 07:41, Kyrill Tkachov wrote:
>>>>
>>>>
>>>> I haven't been able to get combine to match the comparison+xor+neg+plus
>>>> RTL and it seems like it would be just a workaround to undo the
>>>> tree-level transformation.
>>>
>>> Yea, it'd just be a workaround, but it's probably the easiest way to deal
>>> with this problem.  Can you describe in further detail why you weren't
>>> able
>>> to get this to work?
>>
>> Too many instructions to combine I guess.  You might want to add
>> intermediate "combine" insn-and-splits.  If that's still a no-go then
>> read on.
>
>
> From the combine dump I can see that it tried to combine up to:
> (set (reg/i:SI 0 x0)
>     (plus:SI (xor:SI (neg:SI (reg:SI 84 [ D.2565 ]))
>             (reg:SI 73 [ D.2564 ]))
>         (reg:SI 84 [ D.2565 ])))
>
> What I need is for that reg 84 to be the result of the comparison, something
> like:
> (ne (cc_reg) (const_int 0)) which I couldn't get combine to shove in there.
>
>
> On the other hand, I did manage to write a peephole2 that detected the
> sequence of compare+neg+xor+plus and transformed it into the if_then_else
> form that our current conditional negation pattern has, although I'm not
> sure how flexible this is.
>
>
>>
>> OTOH a suitable place to "undo" would be smarter RTL expansion
>> that detects this pattern (and hope for TER to still apply - thus
>> no CSE opportunities going in your way).  For your testcase TER
>> allows
>>
>> r_5 replace with --> r_5 = (int) _4;
>>
>> _8 replace with --> _8 = _4 != 0;
>>
>> _10 replace with --> _10 = -_9;
>>
>> _11 replace with --> _11 = _10 ^ r_5;
>>
>> _12 replace with --> _12 = _11 + _9;
>>
>> which unfortunately already "breaks" because of the multi-use _9
>> and _4.
>>
>> Now - the way out here is a GIMPLE pass right before expansion
>> that performs pattern detection and generates target specific code
>> suitable
>> for optimal expand.  Fortunately we already have that with
>> pass_optimize_widening_mul (which does widen-mul and fma detection).
>
>
> Maybe we can piggyback on this pass then? (probably renaming it to something
> more generic in the process...)
>
>
>>
>> This is the place where you'd deal with such pattern, generating
>> a suitable compound operation (or two, dependent on expansion
>> and insn pattern details).  This of course usually requires new
>> tree codes or internal functions.  For more arcane stuff like
>> your conditional negate I'd prefer an internal function.
>
>
> What is an internal function in this context? Is that like a target-specific
> builtin?
>
>
>>
>> Of course you then have to add an optab or a target hook to
>> do custom internal function expansion.  I suppose for internal
>> functions a target hook would be nicer than a generic optab?
>
>
> So something like TARGET_EXPAND_CONDITIONAL_NEGATION that gets called from
> the aforementioned GIMPLE pass when it finds a an appropriate sequence of
> compare+neg+xor+plus ?

More like TARGET_CAN_EXPAND_IFN with the new GIMPLE internal-fn
and a TARGET_EXPAND_IFN to actually perform the expansion.

Richard.

> Thanks for the pointers,
> Kyrill
>
>
>>
>> Thanks,
>> Richard.
>>
>>
>>
>>>> What is the most acceptable way of disabling this transformation for a
>>>> target that has a conditional negation instruction?
>>>
>>> In general, we don't want target dependencies in the gimple/ssa
>>> optimizers.
>>>
>>> Jeff
>
>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]