Fw: [patch] conditional store elimination

[Tehila Meyzels]

Michael Matz <matz@suse.de> wrote on 05/11/2007 09:29:27:

> Hi,
>
> On Thu, 1 Nov 2007, Tehila Meyzels wrote:
>
> > > > >Later passes then have the possibility to transform the if into a
> > > > >conditional store (and transforming the *p read to a copy).
> > >
> > > Just a small update...
> > > It is (indeed) working on SPU.
> >
> > Well... I looked on it more closer today.
> > It seems to me, that cselim pass doesn't do exactly what you wrote.
> > Attached are a testcase I wrote + dump before the cselim pass + dump
> > after cselim pass.
> > (See attached file: before_cselim.txt)(See attached file:
cselim_test.txt)
> > (See attached file: after_cselim.txt)
> >
> > It looks like the pass adds a conditional load (in the else block),
> > and this is not what I expected (according to your documentation,
> > the speculative store suppose to be before the condition
> > (i.e., unconditional).
>
> No.

Sorry, I meant "speculative load" instead of "speculative store".

>Per documentation and design, the store becomes unconditional, but is
> of course placed after the condition, it couldn't be otherwise.  Instead
> the value to be stored is determined conditionally between the original
> value on that place and the RHS originally stored into it.  So for your
> example:
>
>   int i, *q, *p = ai;
>   for (i = 0; i < 10; i++)
>     {
>       *q = *p;
>       if (i<5)
>         *p = 5;
>   ...
>
> this is transformed conceptually into:
>
>   int i, *q, *p = ai;
>   for (i = 0; i < 10; i++)
>     {
>       *q = *p;
>       if (!(i<5))
>         cstore = *p;
>       else
>         cstore = 5;
>       *p = cstore;
>
> Where the selection between the 5 and *p is done in a phi-node.  Look at
> the comment before tree_ssa_cs_elim, which also explains it.
>
> The dom pass immediately running after cselim will then replace the
> conditional access to *p with a temporary from the unconditional load
> before, which then leads to later passes being presented only with
> instructions working on SSA names, which then are finally transformed
into
> cmoves.
>
> But that work in dom can't happen for your testcase because you write *q
> just before the condition.  As *p and *q alias the value *p will not be
> available anymore in the conditional block.  So there would have to be
> something else to make the load unconditional too.  That's currently not
> done by cselim, but IIRC we talked about that for some future
enhancement.
>
> So, the reason why it doesn't work as intended for you is the write to *q

> after a load from *p.  Try this:
>
> extern void access (int, ...);
> void f(int *q)
> {
>   int i, *p = ai, hold;
>   for (i = 0; i < 10; i++)
>     {
>       *q = *p;
>       hold = *p;
>       *p = 42;
>       if (i<5)
>         *p = 5;
>       p++;
>       access (42, ai[i], hold);
>     }
> }
>
> Note that we meanwhile also need a _write_ to *p to apply the
> transformation.  See the threads about unconditional stores introducing
> traps.
>
> > Maybe this is another reason why conditional load hoisting should
> > be included in this pass as well.
>
> Indeed.

OK. I'll start working on that.

Thanks for the explanation,
Tehila.
>
>
> Ciao,
> Michael.