Possible store motion tweak

Roger Sayle roger@eyesopen.com
Thu Mar 18 14:50:00 GMT 2004


On Thu, 18 Mar 2004, Richard Sandiford wrote:
>
> 	* gcse.c (can_assign_to_reg_p): New function, split out from...
> 	(want_to_gcse_p): ...here.
> 	(compute_ld_motion_mems): Use can_assign_to_reg_p to validate
> 	the rhs of a store.
>

The patch looks good and your argument is convincing, which is what
worries me :>.  My GCC/GCSE paranoia usually tells me that a three
line change that looks too good to be true usually is too good to be
true.  Unfortunately, I can't see the catch.


I was wondering if you could try bootstraps and regression tests on
one or two other platforms in addition to mips64-linux-gnu, perhaps
x86 and powerpc?  It might also help if you described your benchmark
results in more detail, or presented a more comprehensive set of
benchmarks.


My apologies for my "tingling maintainer senses" but it isn't clear
why we weren't performing these load/store motions previously.  It
looks like a fabulous improvement, but I worry about breaking bootstraps,
generating incorrect code or serious performance regression.


This patch is Ok for mainline if bootstrapping and regression testing
on two more platforms is Ok.  Once committed, if the automated SPEC
testers don't show any serious regressions, it can stay.  I think its
unreasonable to ask all contributors to run SPEC, but if you have
access to it, those numbers would provide confidence before the commit.
Reasonable?

Roger
--



More information about the Gcc-patches mailing list