On 06/01/2015 06:26 AM, Ilya Enkovich wrote:
2015-05-29 1:15 GMT+03:00 Jeff Law <l...@redhat.com>:
Right, but you're blindly propagating. The right thing to do is look at
some kind of metric to estimate when it's profitable to propagate the
constant back in vs leave it hoisted out.
No, the patch is not to blindly propagate but to let loop invariant to
be propagated into address. Existing propagation gain estimation
(should_replace_address) still applies.
Agreed. I missed the costing metric in should_replace_address. Sorry
for not looking more deeply at that and delaying progress on this issue.
So looking more closely at the patch itself...
AFAICT, your change to that condition potentially allows fwprop to
propagate any constants to any use, regardless of context (mem vs non-mem).
You're largely getting away with that because you're checking for the
rtx code CONST -- which filters out everything except symbolic
constants. ie, you're filtering out CONST_INT, CONST_DOUBLE, CONST_*.
So the first thing you need to do is clarify that comment a bit. Perhaps:
/* Do not propagate loop invariant definitions into a different loop,
except for symbolic constants which may propagate, subject to cost
testing. */
The second issue is I think the testcase has been compromised. If I
compile the test on the trunk, I don't see any references to GOTOFF. So
I think you need to create a new testcase. One way would be to
instrument a build, detecting anytime you were able to propagate a
symbolic constant into a different loop nest -- if you can discover one,
then use creduce or multidelta to produce a simplified testcase.
Because of the CONST vs CONST_INT stuff, you're not running the risk of
regressing 65768, so no need to build an arm cross a test that :-)
With the comment update and a new testcase this is OK for the trunk.
Again, sorry for delaying progress on this.
Jeff
