Re: [PATCH] Tree SRA and atomicity/volatility

["Daniel Berlin" <dberlin at dberlin dot org>]

On 1/23/07, Mark Mitchell <mark@codesourcery.com> wrote:
> Eric Botcazou wrote:
>
> > 2007-01-06  Eric Botcazou  <ebotcazou@adacore.com>
> >
> >       * tree-sra.c (sra_walk_fns) <ldst>: Document new restriction.
> >       (sra_walk_modify_expr) <rhs_elt>: Treat the reference as a use
> >       if the lhs has side-effects.
> >       <lhs_elt>: Treat the reference as a use if the rhs has side-effects.
> >
> > :ADDPATCH Tree-SRA:
>
> I've read through this thread, and I think Eric's patch makes sense.

> think Richard G. eventually came to the same conclusion, so I'm not
> claiming this is some brilliant insight on my part.)  Even in C, keeping
> volatile accesses "more atomic" seems like a good thing.  And,
> certainly, any possible pessimization by not doing individual accesses
> to move data out of volatile structures is going to be insignificant, to
> overall program runtime.
I agree the principle of his patch makes sense, but i'm not sure of
the implementation.
If it's really checking for volatility, we have
stmt->has_volatile_operands, and this should be checked, rather than
every tree single operand.
We simply don't touch statements with volatile operands.

That appears not to be the case here (he's checking for some property
we don't expose in the middle end), in which case I guess testing
TREE_SIDE_EFFECTS make sense, but i'm very worried about the idea that
we are going to need to test every tree operand before making a
transformation, which is pretty expensive.
That is one of the reasons we have stmt_ann->has_volatile_ops in the
first place - so optimizers could make quick decisions about whether
they should be touching something or not.

If the only place this will ever be tested is SRA, i'm okay with it.
I have a nagging feeling it's not, particularly as we get into more
loop opts (predictive commoning, etc) and i don't want to see us say
"well, everyone was fine with us testing every tree operand then".