Ping
2015-05-05 14:05 GMT+03:00 Ilya Enkovich <enkovich....@gmail.com>: > 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
