On Fri, 14 Jun 2019, Jan Hubicka wrote:

> > 
> > I think this is still error-prone since we look past
> > VIEW_CONVERT_EXPRs in the path so we happily disambiguate,
> > say,
> > 
> > struct X { int e; int d; };
> > struct Y { int d; int e; };
> > 
> >   VIEW_CONVERT<X>(p->b).d
> >   VIEW_CONVERT<Y>(q->b).e
> > 
> > where in reality only the access paths from the base up to
> > the first view-conversion are relevant for path-based
> > analysis.
> > 
> > So upon seeing a VIEW_CONVERT (or BIT_FIELD_REF which
> > has the same issue) simply truncate the vector.
> > 
> > As said, it's a pre-existing issue but you are extending
> > things to possibly handle more stuff...
> > 
> > Otherwise looks good.
> OK, i added the check and also managed to construct testcase that we do
> not optimize otherwise.
> 
> I think same view convert bug ought to exist also in
> aliasing_component_refs_p and IMO ao_ref_alias set should also look
> for VCEs otherwise I can't make sense of
> 
>   /* First defer to TBAA if possible.  */
>   if (tbaa_p
>       && flag_strict_aliasing
>       && !alias_sets_conflict_p (ao_ref_alias_set (ref1),
>                                  ao_ref_alias_set (ref2)))
>     return false;

get_alias_set already handles VCEs properly.  Btw I've said
BIT_FIELD_REF has the same issue but you didn't include that
case below.

Richard.

> Honza
> 
>       * gcc.dg/tree-ssa/alias-access-path-2.c: New testcase.
> 
>       * tree-ssa-alias.c (alias_stats): Add
>       nonoverlapping_component_refs_p_may_alias,
>       nonoverlapping_component_refs_p_no_alias,
>       nonoverlapping_component_refs_of_decl_p_may_alias,
>       nonoverlapping_component_refs_of_decl_p_no_alias.
>       (dump_alias_stats): Dump them.
>       (nonoverlapping_component_refs_of_decl_p): Add stats.
>       (nonoverlapping_component_refs_p): Add stats; do not stop on first
>       ARRAY_REF.
> 
> Index: testsuite/gcc.dg/tree-ssa/alias-access-path-2.c
> ===================================================================
> --- testsuite/gcc.dg/tree-ssa/alias-access-path-2.c   (nonexistent)
> +++ testsuite/gcc.dg/tree-ssa/alias-access-path-2.c   (working copy)
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-fre3" } */
> +struct a {
> +  int val;
> +};
> +struct b {
> +  struct a a[10],a2[10];
> +};
> +struct c {
> +  struct b b[10];
> +} *cptr;
> +
> +struct d {struct c c;} *dptr;
> +
> +int
> +test (int i, int j, int k, int l)
> +{
> +  cptr->b[i].a[j].val=123;
> +  dptr->c.b[k].a2[l].val=2;
> +  return cptr->b[i].a[j].val;
> +}
> +/* { dg-final { scan-tree-dump-times "return 123" 1 "fre3"} } */
> Index: tree-ssa-alias.c
> ===================================================================
> --- tree-ssa-alias.c  (revision 272283)
> +++ tree-ssa-alias.c  (working copy)
> @@ -100,6 +100,10 @@ static struct {
>    unsigned HOST_WIDE_INT call_may_clobber_ref_p_no_alias;
>    unsigned HOST_WIDE_INT aliasing_component_refs_p_may_alias;
>    unsigned HOST_WIDE_INT aliasing_component_refs_p_no_alias;
> +  unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_may_alias;
> +  unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_no_alias;
> +  unsigned HOST_WIDE_INT nonoverlapping_component_refs_of_decl_p_may_alias;
> +  unsigned HOST_WIDE_INT nonoverlapping_component_refs_of_decl_p_no_alias;
>  } alias_stats;
>  
>  void
> @@ -124,7 +128,19 @@ dump_alias_stats (FILE *s)
>          alias_stats.call_may_clobber_ref_p_no_alias,
>          alias_stats.call_may_clobber_ref_p_no_alias
>          + alias_stats.call_may_clobber_ref_p_may_alias);
> -  fprintf (s, "  aliasing_component_ref_p: "
> +  fprintf (s, "  nonoverlapping_component_refs_p: "
> +        HOST_WIDE_INT_PRINT_DEC" disambiguations, "
> +        HOST_WIDE_INT_PRINT_DEC" queries\n",
> +        alias_stats.nonoverlapping_component_refs_p_no_alias,
> +        alias_stats.nonoverlapping_component_refs_p_no_alias
> +        + alias_stats.nonoverlapping_component_refs_p_may_alias);
> +  fprintf (s, "  nonoverlapping_component_refs_of_decl_p: "
> +        HOST_WIDE_INT_PRINT_DEC" disambiguations, "
> +        HOST_WIDE_INT_PRINT_DEC" queries\n",
> +        alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias,
> +        alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias
> +        + alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias);
> +  fprintf (s, "  aliasing_component_refs_p: "
>          HOST_WIDE_INT_PRINT_DEC" disambiguations, "
>          HOST_WIDE_INT_PRINT_DEC" queries\n",
>          alias_stats.aliasing_component_refs_p_no_alias,
> @@ -1047,7 +1063,10 @@ nonoverlapping_component_refs_of_decl_p
>    if (TREE_CODE (ref1) == MEM_REF)
>      {
>        if (!integer_zerop (TREE_OPERAND (ref1, 1)))
> -     return false;
> +     {
> +       ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
> +       return false;
> +     }
>        ref1 = TREE_OPERAND (TREE_OPERAND (ref1, 0), 0);
>      }
>  
> @@ -1060,7 +1079,10 @@ nonoverlapping_component_refs_of_decl_p
>    if (TREE_CODE (ref2) == MEM_REF)
>      {
>        if (!integer_zerop (TREE_OPERAND (ref2, 1)))
> -     return false;
> +     {
> +       ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
> +       return false;
> +     }
>        ref2 = TREE_OPERAND (TREE_OPERAND (ref2, 0), 0);
>      }
>  
> @@ -1080,7 +1102,10 @@ nonoverlapping_component_refs_of_decl_p
>        do
>       {
>         if (component_refs1.is_empty ())
> -         return false;
> +         {
> +           ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
> +           return false;
> +         }
>         ref1 = component_refs1.pop ();
>       }
>        while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref1, 0))));
> @@ -1088,7 +1113,10 @@ nonoverlapping_component_refs_of_decl_p
>        do
>       {
>         if (component_refs2.is_empty ())
> -          return false;
> +         {
> +           ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
> +           return false;
> +         }
>         ref2 = component_refs2.pop ();
>       }
>        while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref2, 0))));
> @@ -1096,7 +1124,10 @@ nonoverlapping_component_refs_of_decl_p
>        /* Beware of BIT_FIELD_REF.  */
>        if (TREE_CODE (ref1) != COMPONENT_REF
>         || TREE_CODE (ref2) != COMPONENT_REF)
> -     return false;
> +     {
> +       ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
> +       return false;
> +     }
>  
>        tree field1 = TREE_OPERAND (ref1, 1);
>        tree field2 = TREE_OPERAND (ref2, 1);
> @@ -1109,7 +1140,10 @@ nonoverlapping_component_refs_of_decl_p
>  
>        /* We cannot disambiguate fields in a union or qualified union.  */
>        if (type1 != type2 || TREE_CODE (type1) != RECORD_TYPE)
> -      return false;
> +     {
> +       ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
> +       return false;
> +     }
>  
>        if (field1 != field2)
>       {
> @@ -1117,15 +1151,23 @@ nonoverlapping_component_refs_of_decl_p
>            same.  */
>         if (DECL_BIT_FIELD_REPRESENTATIVE (field1) == field2
>             || DECL_BIT_FIELD_REPRESENTATIVE (field2) == field1)
> -         return false;
> +         {
> +           ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
> +           return false;
> +         }
>         /* Different fields of the same record type cannot overlap.
>            ??? Bitfields can overlap at RTL level so punt on them.  */
>         if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2))
> -         return false;
> +         {
> +           ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
> +           return false;
> +         }
> +       ++alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias;
>         return true;
>       }
>      }
>  
> +  ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
>    return false;
>  }
>  
> @@ -1154,40 +1196,67 @@ nonoverlapping_component_refs_p (const_t
>  {
>    if (!flag_strict_aliasing
>        || !x || !y
> -      || TREE_CODE (x) != COMPONENT_REF
> -      || TREE_CODE (y) != COMPONENT_REF)
> -    return false;
> +      || !handled_component_p (x)
> +      || !handled_component_p (y))
> +    {
> +      ++alias_stats.nonoverlapping_component_refs_p_may_alias;
> +      return false;
> +    }
>  
>    auto_vec<const_tree, 16> fieldsx;
> -  while (TREE_CODE (x) == COMPONENT_REF)
> +  while (handled_component_p (x))
>      {
> -      tree field = TREE_OPERAND (x, 1);
> -      tree type = DECL_FIELD_CONTEXT (field);
> -      if (TREE_CODE (type) == RECORD_TYPE)
> -     fieldsx.safe_push (field);
> +      if (TREE_CODE (x) == COMPONENT_REF)
> +     {
> +       tree field = TREE_OPERAND (x, 1);
> +       tree type = DECL_FIELD_CONTEXT (field);
> +       if (TREE_CODE (type) == RECORD_TYPE)
> +         fieldsx.safe_push (field);
> +     }
> +      else if (TREE_CODE (x) == VIEW_CONVERT_EXPR)
> +     fieldsx.truncate (0);
>        x = TREE_OPERAND (x, 0);
>      }
>    if (fieldsx.length () == 0)
>      return false;
>    auto_vec<const_tree, 16> fieldsy;
> -  while (TREE_CODE (y) == COMPONENT_REF)
> +  while (handled_component_p (y))
>      {
> -      tree field = TREE_OPERAND (y, 1);
> -      tree type = DECL_FIELD_CONTEXT (field);
> -      if (TREE_CODE (type) == RECORD_TYPE)
> -     fieldsy.safe_push (TREE_OPERAND (y, 1));
> +      if (TREE_CODE (y) == COMPONENT_REF)
> +     {
> +       tree field = TREE_OPERAND (y, 1);
> +       tree type = DECL_FIELD_CONTEXT (field);
> +       if (TREE_CODE (type) == RECORD_TYPE)
> +         fieldsy.safe_push (TREE_OPERAND (y, 1));
> +     }
> +      else if (TREE_CODE (y) == VIEW_CONVERT_EXPR)
> +     fieldsx.truncate (0);
>        y = TREE_OPERAND (y, 0);
>      }
>    if (fieldsy.length () == 0)
> -    return false;
> +    {
> +      ++alias_stats.nonoverlapping_component_refs_p_may_alias;
> +      return false;
> +    }
>  
>    /* Most common case first.  */
>    if (fieldsx.length () == 1
>        && fieldsy.length () == 1)
> -    return ((DECL_FIELD_CONTEXT (fieldsx[0])
> -          == DECL_FIELD_CONTEXT (fieldsy[0]))
> -         && fieldsx[0] != fieldsy[0]
> -         && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0])));
> +   {
> +     if ((DECL_FIELD_CONTEXT (fieldsx[0])
> +         == DECL_FIELD_CONTEXT (fieldsy[0]))
> +        && fieldsx[0] != fieldsy[0]
> +        && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0])))
> +      {
> +         ++alias_stats.nonoverlapping_component_refs_p_no_alias;
> +         return true;
> +      }
> +     else
> +      {
> +         ++alias_stats.nonoverlapping_component_refs_p_may_alias;
> +         return false;
> +      }
> +   }
>  
>    if (fieldsx.length () == 2)
>      {
> @@ -1222,11 +1291,18 @@ nonoverlapping_component_refs_p (const_t
>                same.  */
>             if (DECL_BIT_FIELD_REPRESENTATIVE (fieldx) == fieldy
>                 || DECL_BIT_FIELD_REPRESENTATIVE (fieldy) == fieldx)
> -             return false;
> +             {
> +                ++alias_stats.nonoverlapping_component_refs_p_may_alias;
> +                return false;
> +             }
>             /* Different fields of the same record type cannot overlap.
>                ??? Bitfields can overlap at RTL level so punt on them.  */
>             if (DECL_BIT_FIELD (fieldx) && DECL_BIT_FIELD (fieldy))
> -             return false;
> +             {
> +                ++alias_stats.nonoverlapping_component_refs_p_may_alias;
> +                return false;
> +             }
> +           ++alias_stats.nonoverlapping_component_refs_p_no_alias;
>             return true;
>           }
>       }
> @@ -1245,6 +1321,7 @@ nonoverlapping_component_refs_p (const_t
>      }
>    while (1);
>  
> +  ++alias_stats.nonoverlapping_component_refs_p_may_alias;
>    return false;
>  }
>  
> 

-- 
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