Thanks Joseph, I will sent an updated series tomorrow.
Richard, maybe you could look at what I wrote below
about my use of TYPE_CANONICAL ? Does this make sense?
Am Donnerstag, dem 23.11.2023 um 23:47 +0000 schrieb Joseph Myers:
> 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.
My understand is that it is used for aliasing analysis and also
checking of conversions. TYPE_CANONICAL must be consistent with
the idea the middle-end has about type conversions. But as long
as we do not give the same TYPE_CANONICAL to types the middle-end
thinks must be incompatible using its own type checking machinery,
it should be safe even for types the C standard thinks must be
incompatible for some reason.
> 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?
The main reason are pointers to arrays:
struct foo { int (*x)[]; }
struct foo { int (*x)[2]; };
struct foo { int (*x)[1]; };
So at least when putting it in terms of equivalence classes,
one has no choice than making those types equivalent. So
all those would get the same TYPE_CANONICAL. The middle-endÂ
does not care about the different pointer types (in
useless_type_conversion_p or
gimple_canonical_types_compatible_p).
Martin
>
> > @@ -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"
>
