Re: GCSE store motion

[Daniel Berlin <dberlin at dberlin dot org>]

On Thu, 16 May 2002, Jan Hubicka wrote:

> > On Wed, 15 May 2002, Roger Sayle wrote:
> > 
> > > 
> > > > In addition, never, in any RTL dumps of any code, ever, have I seen it
> > > > remove a single store.
> > > 
> > > I'd suggest compiling the testcase in the patch below with -O3 on CVS
> > > mainline (before store motion was disabled).  The test is ill-formed
> > > and the duplicate store should be moved, the call to abort() reveals
> > > that the optimizer did its job.  Feel free to step through with a
> > > debuggger to convince yourself that it was GCSE's store motion at
> > > work.  For example, it doesn't abort compiling with "-O3 -fno-gcse".
> > > 
> > > Seeing is believing.
> > Yes, as I mentioned, I misspoke. It now 
> > removes 2 stores through global store removal during compilation of gcc, and moves 
> > 3.
> 
> Can you, please describe to person who didn't had time to investigate it
> in detail why the optimization is not effective and what needs to be done
> to fix it? (or point me to mail mentioning it).

Sure.

It's not effective because the only stores it considers are those that 
meet these tests:
1. The destination is a symbol ref (not based on a symbol ref, just a 
symbol ref)
2. rtx_varies_p == false

If you attempt to change #1, you'll start to find that #2 lies.
It'll claim things don't vary even over two executions of the program when 
they do.

On a side rant, based on what rtx_varies_p and rtx_unstable_p 
*CLAIM* to do (by the comments in front of each function), one shouldn't need to check the 
operands of the store if it's rtx_unstable_p, since !rtx_unstable_p 
means that it wouldn't be different at a different point in the program.
But of course, this is likely just an inaccurate comment or bugs in 
setting the flags it's checking.

The stores that pass both of these tests are going to be, in 99% of 
cases stores into globals (I won't discount that it's possible to come up 
with non-global stores that meet these tests).

Theoretically, one could at least use rtx_unstable_p instead of 
rtx_varies_p.   But, as i mentioned, you'll get wrong answers sometimes.
With a bit of extra screening and whatnot elsewhere to account for this, I 
had it down to miscompiling one function in one file that was part of 
java.

The big gains in effectiveness would come from letting it move around 
basically any store as long as the destination wouldn't change 
because of where we moved it to (and i'm sure there are a few other 
conditions that i'm forgetting at the moment).
> I think it would worth comment in the code disabling it.
> 
> Honza
>