[PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386

[Ilya Enkovich]

2015-04-21 8:52 GMT+03:00 Jeff Law <law@redhat.com>:
> On 04/17/2015 02:34 AM, Ilya Enkovich wrote:
>>
>> On 15 Apr 14:07, Ilya Enkovich wrote:
>>>
>>> 2015-04-14 8:22 GMT+03:00 Jeff Law <law@redhat.com>:
>>>>
>>>> On 03/15/2015 02:30 PM, Richard Sandiford wrote:
>>>>>
>>>>>
>>>>> Ilya Enkovich <enkovich.gnu@gmail.com> writes:
>>>>>>
>>>>>>
>>>>>> This patch allows propagation of loop invariants for i386 if
>>>>>> propagated
>>>>>> value is a constant to be used in address operand.  Bootstrapped and
>>>>>> tested on x86_64-unknown-linux-gnu.  OK for trunk or stage 1?
>>>>>
>>>>>
>>>>>
>>>>> Is it necessary for this to be a target hook?  The concept doesn't seem
>>>>> particularly target-specific.  We should only propagate into the
>>>>> address
>>>>> if the new cost is no greater than the old cost, but if the address
>>>>> meets that condition and if propagating at this point in the pipeline
>>>>> is
>>>>> a win on x86, then wouldn't it be a win for other targets too?
>>>>
>>>>
>>>> I agree with Richard here.  I can't see a strong reason why this should
>>>> be a
>>>> target hook.
>>>>
>>>> Perhaps part of the issue here is the address costing metrics may not
>>>> have
>>>> enough context to make good decisions.  In which case what context do
>>>> they
>>>> need?
>>>
>>>
>>> At this point I don't insist on a target hook.  The main reasoning was
>>> to not affect other targets. If we extend propagation for non constant
>>> values different aspects may appear. E.g. possible register pressure
>>> changes may significantly affect ia32. I just wanted to have an
>>> instrument to play with a propagation on x86 not affecting other
>>> targets. I don't have an opportunity to test possible performance
>>> implications on non-x86 targets. Don't expect (significant)
>>> regressions there but who knows...
>>>
>>> I'll remove the hook from this patch. Will probably introduce it later
>>> if some target specific cases are found.
>>>
>>> Thanks,
>>> Ilya
>>>
>>>>
>>>> Jeff
>>
>>
>> Here is a version with no hook.  Bootstrapped and tested on
>> x86_64-unknown-linux-gnu.  Is it OK for trunk?
>>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2015-04-17  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>         PR target/65103
>>         * fwprop.c (forward_propagate_into): Propagate loop
>>         invariants if a target says so.
>>
>> gcc/testsuite/
>>
>> 2015-04-17  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>         PR target/65103
>>         * gcc.target/i386/pr65103-2.c: New.
>
> It seems to me there's a key piece missing here -- metrics.
>
> When is this profitable, when is it not profitable.   Just blindly undoing
> LICM seems wrong here.
>
> The first thought is to look at register pressure through the loop.  I
> thought we had some infrastructure for this kind of query available. It'd
> probably be wise to re-use it.  In fact, one might reasonably ask if LICM
> should have hoisted the expression to start with.
>
>
> I'd also think the cost of the constant may come into play here.  A really
> cheap constant probably should not have been hoisted by LICM to start with
> -- but the code may have been written in such a way that some low cost
> constants are pulled out as loop invariants at the source level.  So this
> isn't strictly an issue of un-doing bad LICM
>
> So I think to go forward we need to be working on solving the "when is this
> a profitable transformation to make".

This patch doesn't force propagation.  The patch just allows
propagation and regular fwprop cost estimation is used to compute if
this is profitable.  For i386 I don't see cases when we shouldn't
propagate. We remove instruction, reduce register pressure and having
constant in memory operand is free which is reflected in address_cost
hook.

Ilya

>
> jeff