Re: [patch PR41843] Change from 'included' to 'equal' in types comparison

[Richard Guenther <rguenther at suse dot de>]

On Mon, 7 Dec 2009, Olga Golovanevsky wrote:

> 
> gcc-patches-owner@gcc.gnu.org wrote on 01/12/2009 18:03:08:
> 
> >
> > You might want to resort to a non-quadratic way of identifying
> > identical types like gimple_types_compatible_p does, simply
> > rely on same field ordering.
> >
> >         /* For aggregate types, all the fields must be the same.  */
> >         for (f1 = TYPE_FIELDS (t1), f2 = TYPE_FIELDS (t2);
> >              f1 && f2;
> >              f1 = TREE_CHAIN (f1), f2 = TREE_CHAIN (f2))
> >           {
> >             /* The fields must have the same name, offset and type.  */
> >             if (DECL_NAME (f1) != DECL_NAME (f2)
> >                 || DECL_NONADDRESSABLE_P (f1) != DECL_NONADDRESSABLE_P
> (f2)
> >                 || !compare_field_offset (f1, f2)
> >                 || !gimple_types_compatible_p (TREE_TYPE (f1),
> >                                                TREE_TYPE (f2)))
> >               goto different_types;
> >           }
> >
> >         /* If one aggregate has more fields than the other, they
> >            are not the same.  */
> >         if (f1 || f2)
> >           goto different_types;
> >
> 
> Right. It is not only quadratic, but the order of the fields *must* be the
> same.
> Took your suggestion and refined the patch.
> 
> Is it ok now?

Ok.

Thanks,
Richard.

> Olga
> 
> Index: ipa-struct-reorg.c
> ===================================================================
> --- ipa-struct-reorg.c  (revision 154989)
> +++ ipa-struct-reorg.c  (working copy)
> @@ -248,6 +248,32 @@ finalize_stmt_and_append (gimple_seq *st
>    finalize_stmt (stmt);
>  }
> 
> +/* This function returns true if two fields FIELD1 and FIELD2 are
> +   semantically equal, and false otherwise.  */
> +
> +static bool
> +compare_fields (tree field1, tree field2)
> +{
> +  if (DECL_NAME (field1) && DECL_NAME (field2))
> +    {
> +      const char *name1 = IDENTIFIER_POINTER (DECL_NAME (field1));
> +      const char *name2 = IDENTIFIER_POINTER (DECL_NAME (field2));
> +
> +      gcc_assert (name1 && name2);
> +
> +      if (strcmp (name1, name2))
> +     return false;
> +
> +    }
> +  else if (DECL_NAME (field1) || DECL_NAME (field2))
> +    return false;
> +
> +  if (!is_equal_types (TREE_TYPE (field1), TREE_TYPE (field2)))
> +    return false;
> +
> +  return true;
> +}
> +
>  /* Given structure type SRT_TYPE and field FIELD,
>     this function is looking for a field with the same name
>     and type as FIELD in STR_TYPE. It returns it if found,
> @@ -264,24 +290,12 @@ find_field_in_struct_1 (tree str_type, t
>    for (str_field = TYPE_FIELDS (str_type); str_field;
>         str_field = TREE_CHAIN (str_field))
>      {
> -      const char *str_field_name;
> -      const char *field_name;
> 
>        if (!DECL_NAME (str_field))
>       continue;
> 
> -      str_field_name = IDENTIFIER_POINTER (DECL_NAME (str_field));
> -      field_name = IDENTIFIER_POINTER (DECL_NAME (field));
> -
> -      gcc_assert (str_field_name);
> -      gcc_assert (field_name);
> -
> -      if (!strcmp (str_field_name, field_name))
> -     {
> -	  /* Check field types.  */
> -       if (is_equal_types (TREE_TYPE (str_field), TREE_TYPE (field)))
> -         return str_field;
> -	}
> +      if (compare_fields (field, str_field))
> +     return str_field;
>      }
> 
>    return NULL_TREE;
> @@ -1596,11 +1610,8 @@ is_equal_types (tree type1, tree type2)
>    name1 = get_type_name (type1);
>    name2 = get_type_name (type2);
> 
> -  if (name1 && name2 && !strcmp (name1, name2))
> -    return true;
> -
> -  if (name1 && name2 && strcmp (name1, name2))
> -    return false;
> +  if (name1 && name2)
> +    return strcmp (name1, name2) == 0;
> 
>    switch (TREE_CODE (type1))
>      {
> @@ -1616,16 +1627,20 @@ is_equal_types (tree type1, tree type2)
>      case QUAL_UNION_TYPE:
>      case ENUMERAL_TYPE:
>        {
> -     tree field1;
> +     tree field1, field2;
> +
>       /* Compare fields of structure.  */
> -     for (field1 = TYPE_FIELDS (type1); field1;
> -          field1 = TREE_CHAIN (field1))
> +     for (field1 = TYPE_FIELDS (type1), field2 = TYPE_FIELDS (type2);
> +          field1 && field2;
> +          field1 = TREE_CHAIN (field1), field2 = TREE_CHAIN (field2))
>         {
> -         tree field2 = find_field_in_struct_1 (type2, field1);
> -         if (!field2)
> +         if (!compare_fields (field1, field2))
>             return false;
>         }
> -     return true;
> +     if (field1 || field2)
> +       return false;
> +     else
> +       return true;
>        }
>        break;
> 
> Index: ChangeLog
> ===================================================================
> --- ChangeLog     (revision 155033)
> +++ ChangeLog     (working copy)
> @@ -1,3 +1,10 @@
> +2009-12-07  Olga Golovanevsky  <olga@il.ibm.com>
> +
> +     PR middle-end/41843
> +     * ipa-struct-reorg.c (compare_fields): New function.
> +     (find_field_in_struct_1): Use compare_fields function.
> +     (is_equal_types): Likewise.
> +
>  2009-12-07  Uros Bizjak  <ubizjak@gmail.com>
> 
>       * config/i386/i386.md (float<SSEMODEI24:mode><X87MODEF:mode>2):
> Index: testsuite/gcc.dg/struct/wo_prof_empty_str.c
> ===================================================================
> --- testsuite/gcc.dg/struct/wo_prof_empty_str.c (revision 0)
> +++ testsuite/gcc.dg/struct/wo_prof_empty_str.c (revision 0)
> @@ -0,0 +1,47 @@
> +/* { dg-options "-O3 -fno-inline -fipa-type-escape -fdump-ipa-all
> -fipa-struct-reorg -fwhole-program -combine" } */
> +/* { dg-do compile } */
> +/* { dg-do run } */
> +
> +#include <stdlib.h>
> +
> +struct S { int a; struct V *b; };
> +typedef struct { int c; } T;
> +typedef struct { int d; int e; } U;
> +
> +void *
> +fn (void *x)
> +{
> +  return x;
> +}
> +
> +int
> +foo (struct S *s)
> +{
> +  T x;
> +
> +  T y = *(T *)fn (&x);
> +  return y.c;
> +}
> +
> +int
> +bar (struct S *s)
> +{
> +  U x;
> +
> +  U y = *(U *)fn (&x);
> +  return y.d + s->a;
> +}
> +
> +int
> +main ()
> +{
> +  struct S s;
> +
> +  foo(&s) + bar (&s);
> +
> +  return 0;
> +}
> +
> +/*--------------------------------------------------------------------------*/
> +/* { dg-final { scan-ipa-dump "No structures to transform"
> "ipa_struct_reorg" { xfail { "avr-*-*" } } } } */
> +/* { dg-final { cleanup-ipa-dump "*" } } */
> Index: testsuite/ChangeLog
> ===================================================================
> --- testsuite/ChangeLog (revision 155033)
> +++ testsuite/ChangeLog (working copy)
> @@ -1,3 +1,9 @@
> +2009-12-04  Olga Golovanevsky  <olga@il.ibm.com>
> +         Jakub Jelinek <jakub@redhat.com>
> +
> +     PR midle-end/41843
> +     * gcc.dg/struct/wo_prof_empty_str.c: New testcase.
> +
>  2009-12-06  Andy Hutchinson  <hutchinsonandy@gcc.gnu.org>
> 
>       * gcc.c-torture/execute/vla-dealloc-1.c: Use lower loop count
> 
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex