On Thu, 16 Nov 2023, Martin Uecker wrote:

> Tell the backend which types are equivalent by setting
> TYPE_CANONICAL to one struct in the set of equivalent
> structs. Structs are considered equivalent by ignoring
> all sizes of arrays nested in types below field level.

Is TYPE_CANONICAL *only* used for alias analysis?  It's not obvious to me 
that setting TYPE_CANONICAL to a type that's definitely not equivalent for 
other purposes is necessarily safe.

I also think more rationale is needed for ignoring sizes like this.  Is it 
intended for e.g. making structs with flexible array members 
alias-compatible with similar structs with a fixed-size array?

> @@ -1250,6 +1266,9 @@ comptypes_internal (const_tree type1, const_tree type2,
>  
>       if ((d1 == NULL_TREE) != (d2 == NULL_TREE))
>         data->different_types_p = true;
> +     /* ignore size mismatches */
> +     if (data->equiv)
> +       return 1;

Should start comment with capital letter, end with '.'.

> diff --git a/gcc/testsuite/gcc.dg/c23-tag-2.c 
> b/gcc/testsuite/gcc.dg/c23-tag-2.c
> index 5dd4a21e9df..e28c2b5eea2 100644
> --- a/gcc/testsuite/gcc.dg/c23-tag-2.c
> +++ b/gcc/testsuite/gcc.dg/c23-tag-2.c
> @@ -1,5 +1,5 @@
> -/* { dg-do compile { target { ! "*-*-*" } } }
> - * { dg-options "-std=c23" }
> +/* { dg-do compile }
> + * { dg-options "-std=c2x" }
>   */
>  
>  // compatibility of structs in assignment
> diff --git a/gcc/testsuite/gcc.dg/c23-tag-5.c 
> b/gcc/testsuite/gcc.dg/c23-tag-5.c
> index ff40d07aef1..95a04bf9b0e 100644
> --- a/gcc/testsuite/gcc.dg/c23-tag-5.c
> +++ b/gcc/testsuite/gcc.dg/c23-tag-5.c
> @@ -1,5 +1,6 @@
> -/* { dg-do run { target { ! "*-*-*" } } }
> - * { dg-options "-std=c23" }
> +/*
> + * { dg-do run }
> + * { dg-options "-std=c2x" }

These tests should not be changed to use -std=c2x.

> diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-2.c 
> b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c
> new file mode 100644
> index 00000000000..555c30a8501
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c
> @@ -0,0 +1,73 @@
> +/*
> + * { dg-do run }
> + * { dg-options "-std=c23 -O2" }
> + */
> +
> +
> +struct foo { int x; };
> +
> +int test_foo1(struct foo* a, void* b)
> +{
> +     a->x = 1;
> +
> +     struct foo { int x; int y; }* p = b;
> +     p->x = 2;
> +
> +     return a->x;
> +}

> +int main()
> +{
> +     struct foo y;
> +
> +     if (1 != test_foo1(&y, &y))
> +             __builtin_abort();

This test appears to be testing various invalid cases - testing that the 
compiler does not consider aliasing to occur in those cases (even though 
in fact there is aliasing).

If that's the intent of this test, it definitely needs commenting.  The 
test would also need to (be a gnu23-* test and) use appropriate attributes 
to disable interprocedural analysis, since it would be entirely valid for 
the compiler in this test to inline test_foo1, see that p->x in fact 
points to the same location as a->x despite the incompatible types, and 
have the function return 2.

The same applies to c23-tag-alias-4.c and c23-tag-alias-5.c.

> diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-5.c 
> b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c
> new file mode 100644
> index 00000000000..4e956720143
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c
> @@ -0,0 +1,30 @@
> +/* { dg-do run }
> + * { dg-options "-std=c23 -O2" }
> + */
> +
> +// not sure this is wise, but this was already like thi sbefore

"this before"

-- 
Joseph S. Myers
jos...@codesourcery.com

Reply via email to