> On Wed, 7 Oct 2015, Jan Hubicka wrote:
> 
> > > 
> > > Did you audit all callers of mem_attrs_eq_p to see if they really
> > > only care about that?  After all MEM_EXPR, via access paths, encode
> > > type-based alias info and thus replacing one with the other (cse.c use
> > > or ifcvt.c use) is only valid if that doesn't break dependences.
> > 
> > Hmm, expr is used by ao_ref_from_mem and nonoverlapping_memrefs_p.
> > The alias set of the access is not taken from expr, but from alias set info
> > stored in the memory attribute itself (and it is checked by those to match)
> 
> But the alias-set is not everything and yes, via ao_ref_from_mem MEM_EXPR
> "leaks" to the tree oracle which happily calls 
> nonoverlapping_component_refs_of_decl_p or nonoverlapping_component_refs_p
> on it.
> 
> > I still think it is an address of the expression that matters, not the 
> > value.
> > I think operand_equal_p may, for example, consider two different VAR_DECL 
> > equivalent
> > if their constructors are, because the value is (it doesn't do that), but 
> > their
> > addresses differ.
> 
> It's structural equality of the MEM_EXPR that matters.  That neither
> operand_equal_p (..., 0) nor operand_equal_p (..., OEP_ADDRESS_OF) is
> an exact implementation for this (well, I think with '0' flags it was
> designed to be this, at least for memory references) is of course
> suspicious.  But that doesn't make using OEP_ADDRESS_OF the correct thing
> to do.

Hmm, I see.  I wonder how complex the expressions are and if we can't simply
compare AO properties of MEM_REF at toplevel and then dispatch to
operand_equal_p (..., OEP_ADDRESS_OF)
which would make more sense to me.

I would basically expect decls and mem_refs here.  Reason why I started to look
into that is that I added sanity check that operand_equal_p (...,0) is not 
called
on things that do not have value (function types and incomplete types) and this
is one of places that fires.

> 
> > I will look more into nonoverlapping_memrefs_p and ao_ref_from_mem. The 
> > first
> > one may need some update to tree-alias infrastructure....
> 
> I'd rather remove it completely (at least that was my plan eventually).
> rtx_refs_may_alias_p is supposed to handle everything it handles.

Yep, that was my feeling from looking into that yesterday....

Honza

Reply via email to