2015-04-21 8:52 GMT+03:00 Jeff Law <l...@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 <l...@redhat.com>: >>>> >>>> On 03/15/2015 02:30 PM, Richard Sandiford wrote: >>>>> >>>>> >>>>> Ilya Enkovich <enkovich....@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.enkov...@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.enkov...@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
