Re: [PATCH] add optional split pass before CSE2

[Trevor_Smigiel at playstation dot sony dot com]

* Richard Guenther <richard.guenther@gmail.com> [2009-05-05 02:35]:
> On Tue, May 5, 2009 at 1:02 AM,  <Trevor_Smigiel@playstation.sony.com> wrote:
> > Hi Steven,
> >
> > Thanks for the reply, sorry I wasn't detailed enough in my previous
> > posts.
> >
> > I see that I need to reevaluate my approach. ?I have been carrying this
> > patch forward internally since 4.1. ?A lot has changed, so the
> > optimizations between expand and cse2 might not be as important as they
> > were before. ?So simply doing it at expand time might be better now.
> > Or, even if some RTL passes are still effective, I understand that it is
> > preferable to fix certain problems in GIMPLE rather than RTL.
> >
> >
> > * Steven Bosscher <stevenb.gcc@gmail.com> [2009-05-04 14:30]:
> >> This explanation doesn't help me, at least, understand why this new
> >> split pass is necessary. Questions I'm left with:
> >>
> >> (1) What is it that makes it impossible to split up everything when
> >> expanding to RTL?
> >
> > It is possible. ?Previously, it lead to worse performance.
> >
> > The problem I was originally fixing is with aggregates and the fact that
> > the SPU architecture has only aligned, 16 byte loads and stores. ?(For
> > non-aggregates, our ABI guarantees that scalars occupy an entire 16
> > bytes.)
> >
> > When loading a member of an aggregate we have to convert to 16 byte
> > loads/stores and change the alias set to account for any adjacent
> > members that are loaded incidentally as part of that 16 bytes. ?The
> > current implementation simply changes the alias set to 0.
> 
> Ugh.  The only problem I can see is that with read-modify-write
> cycles you need to avoid scheduling stores to unmodified parts
> inbetween the read and the write.  If you do not change the
> alias set to 0 for the read then a store may be sinked after
> the read.
> 
> Wouldn't it be better to make sure the read part of the
> read-modify-write cycle also implicitly clobbers the memory?
> That would tie both the read and the write together and would
> avoid stores from being sinked inbetween.
> 
> The read case should be completely unaffected by aliasing issues.

Right.  We actually only change the alias set for stores, not for loads.
That change seems to be sufficient for correctness, i.e., we have never
encountered any bugs in our internal implementation based on GCC 4.1.
I will see if it makes any difference when doing it at expand.


Trevor