> I think the patch is correct.  Consider the decl being accessed
> via memcpy which will result in base2_alias_set == 0.  The
> GIMPLE memory model says decls are just random storage and thus
> their dynamic type can change.  This is also why there's the
> "strange" (and also incomplete...) give-ups on "this looks like
> a view-conversion" are there.

In decl variant of oracle we test:

  /* If the alias set for a pointer access is zero all bets are off.  */
  if (base1_alias_set == -1)
    base1_alias_set = get_deref_alias_set (ptrtype1);
  if (base1_alias_set == 0)
    return true;
  if (base2_alias_set == -1)
    base2_alias_set = get_alias_set (base2);

While for two indirect refs we test:

 /* If the alias set for a pointer access is zero all bets are off.  */
  if (base1_alias_set == -1)
    base1_alias_set = get_deref_alias_set (ptrtype1);
  if (base1_alias_set == 0)
    return true;
  if (base2_alias_set == -1)
    base2_alias_set = get_deref_alias_set (ptrtype2);
  if (base2_alias_set == 0)
    return true;

Now assume that I have decl of some type, use it for a while and then
memcpy completely different data type to it (my understand is that you
want to support this in GIMPLE memory model)

Now VN sees the original type of decl in the original access. It knows
it does not want to use TBAA and to disable it sets base2_alias_set to 0.
(It sets base1_alias_set but things are swapped earlier in
refs_may_alias_p_1.

Now since base1!=0, alias oracle continues. TBAA intersection checks
fails, but eventually it succeeds with disambiguation based on
access path.

I do not think it makes sense to trust access path from the original
access to decl when we expect dynamic type to change.  For me this has
shown as unexpected sucesses of the access path disambiguation which is
only possible if alias set check was disabled.
> 
> Note it's easiest to build GIMPLE testcases for simple disambiguations
> these days ;)  It's also best to quote what we disambiguate by
> dumping the stmts with TDF_GIMPLE since that shows everything
> semantically important.
> 
> But does the patch really make a difference?
> 
>   if (base1_alias_set != base2_alias_set
>       && !alias_sets_conflict_p (base1_alias_set, base2_alias_set))
>     return false;
> 
> Should catch this since everything conflicts with zero?  So I
> wonder how your statistcs can be correct?  The base1_alias_set == 0
> check is redundant as well apart from the case where both
> are zero which is when we end up skipping the above test.

Everything conflicts with 0 and thus the check fails and
oracle continues eventually to aliasing_component_refs which returns false :)

Honza
> 
> Richard.
> 
> 
> > Honza
> > 
> >     * tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Also give up
> >     TBAA path when base2_alias_set is 0.
> > Index: tree-ssa-alias.c
> > ===================================================================
> > --- tree-ssa-alias.c        (revision 271813)
> > +++ tree-ssa-alias.c        (working copy)
> > @@ -1295,7 +1296,7 @@ indirect_ref_may_alias_decl_p (tree ref1
> >    ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1));
> >  
> >    /* If the alias set for a pointer access is zero all bets are off.  */
> > -  if (base1_alias_set == 0)
> > +  if (base1_alias_set == 0 || base2_alias_set == 0)
> >      return true;
> >  
> >    /* When we are trying to disambiguate an access with a pointer 
> > dereference
> > 
> 
> -- 
> Richard Biener <rguent...@suse.de>
> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

Reply via email to