Re: [PATCH][RFA] [PR rtl-optimization/64317] Enhance postreload-gcse.c to eliminate more redundant loads

[Jeff Law <law at redhat dot com>]

On 03/18/2015 12:19 AM, Andrew Pinski wrote:
> On Tue, Mar 17, 2015 at 11:27 AM, Jeff Law <law@redhat.com> wrote:
> > On 03/17/2015 04:35 AM, Richard Biener wrote:
> >
> > > > I'll test both.  In the common case, the cost is going to be the basic
> > > > bookkeeping so that we can compute the transparent property.  The
> > > > actual
> > > > computation of transparency and everything else is guarded on having
> > > > something in the hash tables -- and the overwhelming majority of the
> > > > time there's nothing in the hash tables.
> > > >
> > > > Regardless, I'll pin down boxes and do some testing.
> > > >
> > > >
> > > > > I'm slightly leaning towards trying it even in stage4, but if e.g.
> > > >
> > > > richi
> > > > > disagrees, we could defer it to stage1 too.
> > > >
> > > > I'd be OK either way.  I just want us to make a decision one way or the
> > >
> > >
> > > If it fixes a regression then OK for trunk, otherwise please wait for
> > > stage 1 to open.
> >
> > It fixes 64317 which is a P2 regression.
>
>
> I had a pass which I inherited from my predecessor which was basically
> doing the same as your patch:
> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/postreload-load.c;h=2652e51f8eca310d51db3a30e5f6c8847be436ce;hb=refs/heads/apinski/thunderx-cost
>
> But I like your patch much better than this one.  I saw many loads
> being removed for AARCH64 also at -Ofast -funroll-loops on SPEC 2006
> with this pass.  But it seemed to due to subregs which I had mentioned
> at https://gcc.gnu.org/ml/gcc/2015-01/msg00125.html .  When I get a
> chance I can test your patch out on AARCH64 and disable "my" pass to
> see if "my" pass catches more than your patch.
ISTM this pass from your predecessor is tackling a different issue. 
Namely it arranges to copy the result of the load (or source of a store) 
from a register which gets clobbered into a non-clobbered hard register 
so that it can be re-used later.

My patch detected cases where the the load/store register is not 
clobbered across intermediate blocks and arranges to re-use it.

Unless I'm missing something, they ought to be able to work together.

I'd recommend officially submitting that code.  Bonus points for 
integrating it into the existing pass rather than running it as a 
separate pass.

jeff