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"
>
@@ -634,6 +634,36 @@ public:
auto_vec<tree> typedefs_seen;
};
+
+/* Hash table for structs and unions. */
+struct c_struct_hasher : ggc_ptr_hash<tree_node>
+{
+ static hashval_t hash (tree t);
+ static bool equal (tree, tree);
+};
+
+/* Hash an RECORD OR UNION. */
+hashval_t
+c_struct_hasher::hash (tree type)
+{
+ inchash::hash hstate;
+
+ hstate.add_int (TREE_CODE (type));
+ hstate.add_object (TYPE_NAME (type));
+
+ return hstate.end ();
+}
+
+/* Compare two RECORD or UNION types. */
+bool
+c_struct_hasher::equal (tree t1, tree t2)
+{
+ return comptypes_equiv_p (t1, t2);
+}
+
+/* All tagged typed so that TYPE_CANONICAL can be set correctly. */
+static GTY (()) hash_table<c_struct_hasher> *c_struct_htab;
+
/* Information for the struct or union currently being parsed, or
NULL if not parsing a struct or union. */
static class c_struct_parse_info *struct_parse_info;
@@ -9646,6 +9676,24 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
C_TYPE_BEING_DEFINED (t) = 0;
+ /* Set type canonical based on equivalence class. */
+ if (flag_isoc23)
+ {
+ if (NULL == c_struct_htab)
+ c_struct_htab = hash_table<c_struct_hasher>::create_ggc (61);
+
+ hashval_t hash = c_struct_hasher::hash (t);
+
+ tree *e = c_struct_htab->find_slot_with_hash (t, hash, INSERT);
+ if (*e)
+ TYPE_CANONICAL (t) = *e;
+ else
+ {
+ TYPE_CANONICAL (t) = t;
+ *e = t;
+ }
+ }
+
tree incomplete_vars = C_TYPE_INCOMPLETE_VARS (TYPE_MAIN_VARIANT (t));
for (x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x))
{
@@ -389,6 +389,11 @@ c_get_alias_set (tree t)
if (TREE_CODE (t) == ENUMERAL_TYPE)
return get_alias_set (ENUM_UNDERLYING_TYPE (t));
+ /* Structs with variable size can alias different incompatible
+ structs. Let them alias anything. */
+ if (RECORD_OR_UNION_TYPE_P (t) && C_TYPE_VARIABLE_SIZE (t))
+ return 0;
+
return c_common_get_alias_set (t);
}
@@ -758,6 +758,7 @@ extern tree require_complete_type (location_t, tree);
extern bool same_translation_unit_p (const_tree, const_tree);
extern int comptypes (tree, tree);
extern bool comptypes_same_p (tree, tree);
+extern int comptypes_equiv_p (tree, tree);
extern int comptypes_check_different_types (tree, tree, bool *);
extern int comptypes_check_enum_int (tree, tree, bool *);
extern bool c_mark_addressable (tree, bool = false);
@@ -1063,6 +1063,7 @@ struct comptypes_data {
bool different_types_p;
bool warning_needed;
bool anon_field;
+ bool equiv;
const struct tagged_tu_seen_cache* cache;
};
@@ -1123,6 +1124,21 @@ comptypes_check_different_types (tree type1, tree type2,
return ret ? (data.warning_needed ? 2 : 1) : 0;
}
+
+
+/* Like comptypes, but if it returns nonzero for struct and union
+ types considered equivalent for aliasing purposes. */
+
+int
+comptypes_equiv_p (tree type1, tree type2)
+{
+ struct comptypes_data data = { };
+ data.equiv = true;
+ bool ret = comptypes_internal (type1, type2, &data);
+
+ return ret;
+}
+
/* Return true if TYPE1 and TYPE2 are compatible types for assignment
or various other operations. If they are compatible but a warning may
@@ -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;
/* Sizes must match unless one is missing or variable. */
if (d1 == NULL_TREE || d2 == NULL_TREE || d1 == d2)
return true;
@@ -1467,6 +1486,9 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree t2,
if (list_length (TYPE_FIELDS (t1)) != list_length (TYPE_FIELDS (t2)))
return false;
+ if (data->equiv && (C_TYPE_VARIABLE_SIZE (t1) || C_TYPE_VARIABLE_SIZE (t2)))
+ return 0;
+
for (s1 = TYPE_FIELDS (t1), s2 = TYPE_FIELDS (t2);
s1 && s2;
s1 = DECL_CHAIN (s1), s2 = DECL_CHAIN (s2))
@@ -1486,6 +1508,15 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree t2,
&& simple_cst_equal (DECL_FIELD_BIT_OFFSET (s1),
DECL_FIELD_BIT_OFFSET (s2)) != 1)
return false;
+
+ tree st1 = TYPE_SIZE (TREE_TYPE (s1));
+ tree st2 = TYPE_SIZE (TREE_TYPE (s2));
+
+ if (data->equiv
+ && st1 && TREE_CODE (st1) == INTEGER_CST
+ && st2 && TREE_CODE (st2) == INTEGER_CST
+ && !tree_int_cst_equal (st1, st2))
+ return 0;
}
return true;
@@ -1,5 +1,5 @@
-/* { dg-do compile { target { ! "*-*-*" } } }
- * { dg-options "-std=c23" }
+/* { dg-do compile }
+ * { dg-options "-std=c2x" }
*/
// compatibility of structs in assignment
@@ -1,5 +1,6 @@
-/* { dg-do run { target { ! "*-*-*" } } }
- * { dg-options "-std=c23" }
+/*
+ * { dg-do run }
+ * { dg-options "-std=c2x" }
*/
// nesting and parameters
new file mode 100644
@@ -0,0 +1,48 @@
+/*
+ * { dg-do run }
+ * { dg-options "-std=c23 -O2" }
+ */
+
+
+struct foo { int x; };
+
+int test_foo(struct foo* a, void* b)
+{
+ a->x = 1;
+
+ struct foo { int x; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+enum bar { A = 1, B = 3 };
+
+int test_bar(enum bar* a, void* b)
+{
+ *a = A;
+
+ enum bar { A = 1, B = 3 }* p = b;
+ *p = B;
+
+ return *a;
+}
+
+
+int main()
+{
+ struct foo y;
+
+ if (2 != test_foo(&y, &y))
+ __builtin_abort();
+
+ enum bar z;
+
+ if (A == test_bar(&z, &z))
+ __builtin_abort();
+
+ return 0;
+}
+
+
new file mode 100644
@@ -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 test_foo2(struct foo* a, void* b)
+{
+ a->x = 1;
+
+ struct fox { int x; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+enum bar { A = 1, B = 3, C = 5, D = 9 };
+
+int test_bar1(enum bar* a, void* b)
+{
+ *a = A;
+
+ enum bar { A = 1, B = 3, C = 6, D = 9 }* p = b;
+ *p = B;
+
+ return *a;
+}
+
+int test_bar2(enum bar* a, void* b)
+{
+ *a = A;
+
+ enum baX { A = 1, B = 3, C = 5, D = 9 }* p = b;
+ *p = B;
+
+ return *a;
+}
+
+
+int main()
+{
+ struct foo y;
+
+ if (1 != test_foo1(&y, &y))
+ __builtin_abort();
+
+ if (1 != test_foo2(&y, &y))
+ __builtin_abort();
+
+ enum bar z;
+
+ if (A == test_bar1(&z, &z))
+ __builtin_abort();
+
+ if (A == test_bar2(&z, &z))
+ __builtin_abort();
+
+ return 0;
+}
+
+
new file mode 100644
@@ -0,0 +1,48 @@
+/*
+ * { dg-do run }
+ * { dg-options "-std=c23 -flto -O2" }
+ */
+
+
+struct foo { int x; };
+
+int test_foo(struct foo* a, void* b)
+{
+ a->x = 1;
+
+ struct foo { int x; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+enum bar { A = 1, B = 3 };
+
+int test_bar(enum bar* a, void* b)
+{
+ *a = A;
+
+ enum bar { A = 1, B = 3 }* p = b;
+ *p = B;
+
+ return *a;
+}
+
+
+int main()
+{
+ struct foo y;
+
+ if (2 != test_foo(&y, &y))
+ __builtin_abort();
+
+ enum bar z;
+
+ if (A == test_bar(&z, &z))
+ __builtin_abort();
+
+ return 0;
+}
+
+
new file mode 100644
@@ -0,0 +1,73 @@
+/*
+ * { dg-do run }
+ * { dg-options "-std=c23 -flto -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 test_foo2(struct foo* a, void* b)
+{
+ a->x = 1;
+
+ struct fox { int x; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+enum bar { A = 1, B = 3, C = 5, D = 9 };
+
+int test_bar1(enum bar* a, void* b)
+{
+ *a = A;
+
+ enum bar { A = 1, B = 3, C = 6, D = 9 }* p = b;
+ *p = B;
+
+ return *a;
+}
+
+int test_bar2(enum bar* a, void* b)
+{
+ *a = A;
+
+ enum baX { A = 1, B = 3, C = 5, D = 9 }* p = b;
+ *p = B;
+
+ return *a;
+}
+
+
+int main()
+{
+ struct foo y;
+
+ if (1 != test_foo1(&y, &y))
+ __builtin_abort();
+
+ if (1 != test_foo2(&y, &y))
+ __builtin_abort();
+
+ enum bar z;
+
+ if (A == test_bar1(&z, &z))
+ __builtin_abort();
+
+ if (A == test_bar2(&z, &z))
+ __builtin_abort();
+
+ return 0;
+}
+
+
new file mode 100644
@@ -0,0 +1,30 @@
+/* { dg-do run }
+ * { dg-options "-std=c23 -O2" }
+ */
+
+// not sure this is wise, but this was already like thi sbefore
+
+typedef struct { int x; } foo_t;
+
+int test_foo(foo_t* a, void* b)
+{
+ a->x = 1;
+
+ struct { int x; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+int main()
+{
+ foo_t y;
+
+ if (1 != test_foo(&y, &y))
+ __builtin_abort();
+
+ return 0;
+}
+
+
new file mode 100644
@@ -0,0 +1,77 @@
+/*
+ * { dg-do run }
+ * { dg-options "-std=c23 -O2" }
+ */
+
+
+/* We check that we tolerate differences for
+ * optimization.
+ */
+
+
+struct bar0 { int x; int f[3]; int y; };
+
+int test_bar0(struct bar0* a, void* b)
+{
+ a->x = 1;
+
+ struct bar0 { int x; int f[4]; int y; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+
+struct bar1 { int x; int (*f)[3]; };
+
+int test_bar1(struct bar1* a, void* b)
+{
+ a->x = 1;
+
+ struct bar1 { int x; int (*f)[3]; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+struct bar2 { int x; int (*f)[3]; };
+
+int test_bar2(struct bar2* a, void* b)
+{
+ a->x = 1;
+
+ struct bar2 { int x; int (*f)[4]; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+
+int main()
+{
+ // control
+
+ struct bar0 z0;
+
+ if (1 != test_bar0(&z0, &z0))
+ __builtin_abort();
+
+ // this could be different
+ struct bar1 z1;
+
+ if (2 != test_bar1(&z1, &z1))
+ __builtin_abort();
+
+ struct bar2 z2;
+
+ if (2 != test_bar2(&z2, &z2))
+ __builtin_abort();
+
+
+ return 0;
+}
+
+
new file mode 100644
@@ -0,0 +1,86 @@
+/*
+ * { dg-do run }
+ * { dg-options "-std=c23 -O2" }
+ */
+
+
+
+struct bar { int x; int f[]; };
+
+int test_bar1(struct bar* a, void* b)
+{
+ a->x = 1;
+
+ struct bar { int x; int f[]; }* p = b;
+ struct bar* q = a;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+int test_bar3(struct bar* a, void* b)
+{
+ a->x = 1;
+
+ struct bar { int x; int f[1]; }* p = b;
+ struct bar* q = a; /* { dg-warning "incompatible" } */
+ p->x = 2;
+
+ return a->x;
+}
+
+
+int test_bar4(struct bar* a, void* b)
+{
+ a->x = 1;
+
+ int n = 3;
+ struct bar { int x; int f[n]; }* p = b;
+ struct bar* q = a;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+struct foo { int x; int f[3]; };
+
+
+int test_foo1(struct foo* a, void* b)
+{
+ a->x = 1;
+
+ int n = 3;
+ struct foo { int x; int f[n]; }* p = b;
+ struct foo* q = a;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+
+int main()
+{
+ struct bar z;
+
+ if (2 != test_bar1(&z, &z))
+ __builtin_abort();
+
+#if 0
+ if (1 != test_bar3(&z, &z))
+ __builtin_abort();
+#endif
+ if (2 != test_bar4(&z, &z))
+ __builtin_abort();
+
+ struct foo y;
+
+ if (2 != test_foo1(&y, &y))
+ __builtin_abort();
+
+ return 0;
+}
+
+
new file mode 100644
@@ -0,0 +1,90 @@
+/*
+ * { dg-do run }
+ * { dg-options "-std=c23 -O2" }
+ */
+
+
+/* We check that we tolerate differences for
+ * optimization.
+ */
+
+struct bar1 { int x; enum A1 { X1 = 1 } f; };
+
+int test_bar1(struct bar1* a, void* b)
+{
+ a->x = 1;
+
+ struct bar1 { int x; enum A1 { X1 = 2 } f; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+struct bar2 { int x; enum A2 { X2 = 1 } f; };
+
+int test_bar2(struct bar2* a, void* b)
+{
+ a->x = 1;
+
+ struct bar2 { int x; enum B2 { X2 = 1 } f; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+
+struct bar3 { int x; enum A3 { X3 = 1 } f; };
+
+int test_bar3(struct bar3* a, void* b)
+{
+ a->x = 1;
+
+ struct bar3 { int x; enum A3 { Y3 = 1 } f; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+struct bar4 { int x; enum { Z4 = 1 } f; };
+
+int test_bar4(struct bar4* a, void* b)
+{
+ a->x = 1;
+
+ struct bar4 { int x; enum { Z4 = 1 } f; }* p = b;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+
+int main()
+{
+ struct bar1 z1;
+
+ if (1 != test_bar1(&z1, &z1))
+ __builtin_abort();
+
+ struct bar2 z2;
+
+ if (1 != test_bar2(&z2, &z2))
+ __builtin_abort();
+
+ struct bar3 z3;
+
+ if (1 != test_bar3(&z3, &z3))
+ __builtin_abort();
+
+ struct bar4 z4;
+
+ if (1 != test_bar4(&z4, &z4))
+ __builtin_abort();
+
+ return 0;
+}
+
+
new file mode 100644
@@ -0,0 +1,33 @@
+/*
+ * { dg-do run }
+ * { dg-options "-std=gnu23 -O2" }
+ */
+
+
+
+struct bar { int x; int f[]; };
+
+int test_bar2(struct bar* a, void* b)
+{
+ a->x = 1;
+
+ struct bar { int x; int f[0]; }* p = b;
+ struct bar* q = a;
+ p->x = 2;
+
+ return a->x;
+}
+
+
+
+int main()
+{
+ struct bar z;
+
+ if (2 != test_bar2(&z, &z))
+ __builtin_abort();
+
+ return 0;
+}
+
+