[PATCH] Handle non-volatile GIMPLE_ASM in get_references_in_stmt (PR tree-optimization/51987)

[Richard Guenther]

On Wed, 25 Jan 2012, Jakub Jelinek wrote:

> On Wed, Jan 25, 2012 at 10:49:27AM +0100, Richard Guenther wrote:
> > I wonder if it's worth handling asms in any fancy way here, considering
> > that data-ref analysis happily punts on them completely.  Thus, why
> > not change the above to
> > 
> >   || stmt_code == GIMPLE_ASM)
> 
> I think asm ("..." : "+r" (x)); and similar are sufficiently common that
> it is worth doing something at least for those.
> Thus, I'd be fine with say
>   (stmt_code == GIMPLE_ASM
>    && (gimple_asm_volatile_p (stmt) || gimple_vuse (stmt)))
> and don't do anything about the operands.
> That would handle the gimple_asm_clobbers_memory_p case, or the case in this
> PR, etc.
> Would you be ok with that instead?  That would be a conservative fix
> for this stage.

Yes, that's ok with me.  For the rest you can file an enhancement PR.

> > ?  That sounds more safe for this stage anyway (does a volatile
> > asm really clobber memory even if you don't explicitely say so?
> > At least the operand scanner won't add virtual operands just because of 
> > that, so the check looks bogus anyway).
> 
> Given that asm volatile should act like an optimization barrier, I think it
> is desirable to punt on them even when they don't have memory operands.

A volatile asm without explicitely clobbering memory does not act as
an optimization barrier.  If it should, then it definitely needs
virtual operands.

Sure, but just like we punt on gimple_has_volatile_ops_p stmts - passes
should explicitely check for gimple_asm_volatile_p or use
gimple_has_side_effects.

> > Btw, there are numerous callers of get_references_in_stmt that
> > do not check its return value ... (and I think it misses a
> > return value kind that tells the vector of references may be incomplete,
> > like in the ASM kind right now).
> 
> Incomplete should be signalled by returning true I'd say, that means beyond
> the listed ones it accesses some others too.
> 
> > Btw, get_references_in_stmt should probably use walk_stmt_load_store_ops,
> > so we have a single place where we put knowledge where memory references
> > can occur.
> 
> Probably, but wouldn't that be a 4.8 material?

Yes.

Thanks,
Richard.