Message ID | e367dd44ee5634f2ae3653b322339f8d61aa039a.camel@tugraz.at |
---|---|
State | New |
Headers | show |
Series | [C] : allow aliasing of compatible types derived from enumeral types [PR115157] | expand |
On Tue, 21 May 2024, Martin Uecker wrote: > > C: allow aliasing of compatible types derived from enumeral types [PR115157] > > Aliasing of enumeral types with the underlying integer is now allowed > by setting the aliasing set to zero. But this does not allow aliasing > of derived types which are compatible as required by ISO C. Instead, > initially set structural equality. Then set TYPE_CANONICAL and update > pointers and main variants when the type is completed (as done for > structures and unions in C23). > > PR 115157 > > gcc/c/ > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. > * c-obj-common.cc (get_alias_set): Remove special case. > (get_aka_type): Add special case. > > gcc/ > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead > of TYPE_CANONICAL. > > gcc/testsuite/ > * gcc.dg/enum-alias-1.c: New test. > * gcc.dg/enum-alias-2.c: New test. > * gcc.dg/enum-alias-3.c: New test. OK, in the absence of objections on middle-end or Go grounds within the next week.
On Thu, May 23, 2024 at 2:00 PM Joseph Myers <josmyers@redhat.com> wrote: > > On Tue, 21 May 2024, Martin Uecker wrote: > > > > C: allow aliasing of compatible types derived from enumeral types [PR115157] > > > > Aliasing of enumeral types with the underlying integer is now allowed > > by setting the aliasing set to zero. But this does not allow aliasing > > of derived types which are compatible as required by ISO C. Instead, > > initially set structural equality. Then set TYPE_CANONICAL and update > > pointers and main variants when the type is completed (as done for > > structures and unions in C23). > > > > PR 115157 > > > > gcc/c/ > > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. > > * c-obj-common.cc (get_alias_set): Remove special case. > > (get_aka_type): Add special case. > > > > gcc/ > > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead > > of TYPE_CANONICAL. > > > > gcc/testsuite/ > > * gcc.dg/enum-alias-1.c: New test. > > * gcc.dg/enum-alias-2.c: New test. > > * gcc.dg/enum-alias-3.c: New test. > > OK, in the absence of objections on middle-end or Go grounds within the > next week. The godump.cc patch is && (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE || !container->decls_seen.contains - (TYPE_CANONICAL (TREE_TYPE (decl))))) + (TYPE_MAIN_VARIANT (TREE_TYPE (decl))))) { What is the problem you are seeing? This patch isn't right: 1) The code is saying if "X == NULL_TREE || !already_seen(X)". This patch is changing the latter X but not the former. They should be consistent. 2) At the bottom of that conditional block is code that adds a value to container->decls_seen. Today that code is adding TYPE_CANONICAL. If we change the condition to test TYPE_MAIN_VARIANT, then we need to add TYPE_MAIN_VARIANT to decls_seen. Hope that makes sense. I don't know why the patch is required, but it's fine with those changes as long as the libgo tests continue to pass. Ian
Am Donnerstag, dem 23.05.2024 um 14:30 -0700 schrieb Ian Lance Taylor: > On Thu, May 23, 2024 at 2:00 PM Joseph Myers <josmyers@redhat.com> wrote: > > > > On Tue, 21 May 2024, Martin Uecker wrote: > > > > > > C: allow aliasing of compatible types derived from enumeral types [PR115157] > > > > > > Aliasing of enumeral types with the underlying integer is now allowed > > > by setting the aliasing set to zero. But this does not allow aliasing > > > of derived types which are compatible as required by ISO C. Instead, > > > initially set structural equality. Then set TYPE_CANONICAL and update > > > pointers and main variants when the type is completed (as done for > > > structures and unions in C23). > > > > > > PR 115157 > > > > > > gcc/c/ > > > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > > > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. > > > * c-obj-common.cc (get_alias_set): Remove special case. > > > (get_aka_type): Add special case. > > > > > > gcc/ > > > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead > > > of TYPE_CANONICAL. > > > > > > gcc/testsuite/ > > > * gcc.dg/enum-alias-1.c: New test. > > > * gcc.dg/enum-alias-2.c: New test. > > > * gcc.dg/enum-alias-3.c: New test. > > > > OK, in the absence of objections on middle-end or Go grounds within the > > next week. > > The godump.cc patch is > > && (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE > || !container->decls_seen.contains > - (TYPE_CANONICAL (TREE_TYPE (decl))))) > + (TYPE_MAIN_VARIANT (TREE_TYPE (decl))))) > { > > What is the problem you are seeing? Test failures in godump-1.c > > This patch isn't right: > > 1) The code is saying if "X == NULL_TREE || !already_seen(X)". This > patch is changing the latter X but not the former. They should be > consistent. Maybe the X == NULL_TREE can be removed if we add TYPE_MAIN_VARIANTs instead? > > 2) At the bottom of that conditional block is code that adds a value > to container->decls_seen. Today that code is adding TYPE_CANONICAL. > If we change the condition to test TYPE_MAIN_VARIANT, then we need to > add TYPE_MAIN_VARIANT to decls_seen. Yes, obviously this is wrong. Thanks! Martin > > Hope that makes sense. > > I don't know why the patch is required, but it's fine with those > changes as long as the libgo tests continue to pass. > > Ian
On Thu, May 23, 2024 at 2:48 PM Martin Uecker <uecker@tugraz.at> wrote: > > Am Donnerstag, dem 23.05.2024 um 14:30 -0700 schrieb Ian Lance Taylor: > > On Thu, May 23, 2024 at 2:00 PM Joseph Myers <josmyers@redhat.com> wrote: > > > > > > On Tue, 21 May 2024, Martin Uecker wrote: > > > > > > > > C: allow aliasing of compatible types derived from enumeral types [PR115157] > > > > > > > > Aliasing of enumeral types with the underlying integer is now allowed > > > > by setting the aliasing set to zero. But this does not allow aliasing > > > > of derived types which are compatible as required by ISO C. Instead, > > > > initially set structural equality. Then set TYPE_CANONICAL and update > > > > pointers and main variants when the type is completed (as done for > > > > structures and unions in C23). > > > > > > > > PR 115157 > > > > > > > > gcc/c/ > > > > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > > > > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. > > > > * c-obj-common.cc (get_alias_set): Remove special case. > > > > (get_aka_type): Add special case. > > > > > > > > gcc/ > > > > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead > > > > of TYPE_CANONICAL. > > > > > > > > gcc/testsuite/ > > > > * gcc.dg/enum-alias-1.c: New test. > > > > * gcc.dg/enum-alias-2.c: New test. > > > > * gcc.dg/enum-alias-3.c: New test. > > > > > > OK, in the absence of objections on middle-end or Go grounds within the > > > next week. > > > > The godump.cc patch is > > > > && (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE > > || !container->decls_seen.contains > > - (TYPE_CANONICAL (TREE_TYPE (decl))))) > > + (TYPE_MAIN_VARIANT (TREE_TYPE (decl))))) > > { > > > > What is the problem you are seeing? > > Test failures in godump-1.c > > > > > This patch isn't right: > > > > 1) The code is saying if "X == NULL_TREE || !already_seen(X)". This > > patch is changing the latter X but not the former. They should be > > consistent. > > Maybe the X == NULL_TREE can be removed if we > add TYPE_MAIN_VARIANTs instead? If TYPE_MAIN_VARIANT is never NULL_TREE, then I agree that the NULL_TREE test can be removed. Ian
On Thu, 23 May 2024, Ian Lance Taylor wrote: > On Thu, May 23, 2024 at 2:48 PM Martin Uecker <uecker@tugraz.at> wrote: > > > > Am Donnerstag, dem 23.05.2024 um 14:30 -0700 schrieb Ian Lance Taylor: > > > On Thu, May 23, 2024 at 2:00 PM Joseph Myers <josmyers@redhat.com> wrote: > > > > > > > > On Tue, 21 May 2024, Martin Uecker wrote: > > > > > > > > > > C: allow aliasing of compatible types derived from enumeral types [PR115157] > > > > > > > > > > Aliasing of enumeral types with the underlying integer is now allowed > > > > > by setting the aliasing set to zero. But this does not allow aliasing > > > > > of derived types which are compatible as required by ISO C. Instead, > > > > > initially set structural equality. Then set TYPE_CANONICAL and update > > > > > pointers and main variants when the type is completed (as done for > > > > > structures and unions in C23). > > > > > > > > > > PR 115157 > > > > > > > > > > gcc/c/ > > > > > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > > > > > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. > > > > > * c-obj-common.cc (get_alias_set): Remove special case. > > > > > (get_aka_type): Add special case. > > > > > > > > > > gcc/ > > > > > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead > > > > > of TYPE_CANONICAL. > > > > > > > > > > gcc/testsuite/ > > > > > * gcc.dg/enum-alias-1.c: New test. > > > > > * gcc.dg/enum-alias-2.c: New test. > > > > > * gcc.dg/enum-alias-3.c: New test. > > > > > > > > OK, in the absence of objections on middle-end or Go grounds within the > > > > next week. > > > > > > The godump.cc patch is > > > > > > && (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE > > > || !container->decls_seen.contains > > > - (TYPE_CANONICAL (TREE_TYPE (decl))))) > > > + (TYPE_MAIN_VARIANT (TREE_TYPE (decl))))) > > > { > > > > > > What is the problem you are seeing? > > > > Test failures in godump-1.c > > > > > > > > This patch isn't right: > > > > > > 1) The code is saying if "X == NULL_TREE || !already_seen(X)". This > > > patch is changing the latter X but not the former. They should be > > > consistent. > > > > Maybe the X == NULL_TREE can be removed if we > > add TYPE_MAIN_VARIANTs instead? > > If TYPE_MAIN_VARIANT is never NULL_TREE, then I agree that the > NULL_TREE test can be removed. TYPE_MAIN_VARIANT is indeed never NULL_TREE. Richard.
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index b691b91b3db..6e6606c9570 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -5051,7 +5051,7 @@ shadow_tag_warned (const struct c_declspecs *declspecs, int warned) if (t == NULL_TREE) { t = make_node (code); - if (flag_isoc23 && code != ENUMERAL_TYPE) + if (flag_isoc23 || code == ENUMERAL_TYPE) SET_TYPE_STRUCTURAL_EQUALITY (t); pushtag (input_location, name, t); } @@ -8828,7 +8828,7 @@ parser_xref_tag (location_t loc, enum tree_code code, tree name, the forward-reference will be altered into a real type. */ ref = make_node (code); - if (flag_isoc23 && code != ENUMERAL_TYPE) + if (flag_isoc23 || code == ENUMERAL_TYPE) SET_TYPE_STRUCTURAL_EQUALITY (ref); if (code == ENUMERAL_TYPE) { @@ -9919,6 +9919,7 @@ start_enum (location_t loc, struct c_enum_contents *the_enum, tree name, { enumtype = make_node (ENUMERAL_TYPE); TYPE_SIZE (enumtype) = NULL_TREE; + SET_TYPE_STRUCTURAL_EQUALITY (enumtype); pushtag (loc, name, enumtype); if (fixed_underlying_type != NULL_TREE) { @@ -9935,6 +9936,8 @@ start_enum (location_t loc, struct c_enum_contents *the_enum, tree name, TYPE_SIZE (enumtype) = NULL_TREE; TYPE_PRECISION (enumtype) = TYPE_PRECISION (fixed_underlying_type); ENUM_UNDERLYING_TYPE (enumtype) = fixed_underlying_type; + TYPE_CANONICAL (enumtype) = TYPE_CANONICAL (fixed_underlying_type); + c_update_type_canonical (enumtype); layout_type (enumtype); } } @@ -10094,6 +10097,10 @@ finish_enum (tree enumtype, tree values, tree attributes) ENUM_UNDERLYING_TYPE (enumtype) = c_common_type_for_size (TYPE_PRECISION (tem), TYPE_UNSIGNED (tem)); + TYPE_CANONICAL (enumtype) = + TYPE_CANONICAL (ENUM_UNDERLYING_TYPE (enumtype)); + c_update_type_canonical (enumtype); + layout_type (enumtype); } diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc index b7c72d2609c..551ec6f4b65 100644 --- a/gcc/c/c-objc-common.cc +++ b/gcc/c/c-objc-common.cc @@ -130,6 +130,8 @@ get_aka_type (tree type) result = get_aka_type (orig_type); } + else if (TREE_CODE (type) == ENUMERAL_TYPE) + return type; else { tree canonical = TYPE_CANONICAL (type); @@ -418,11 +420,6 @@ c_var_mod_p (tree x, tree fn ATTRIBUTE_UNUSED) alias_set_type c_get_alias_set (tree t) { - /* Allow aliasing between enumeral types and the underlying - integer type. This is required since those are compatible types. */ - 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)) diff --git a/gcc/godump.cc b/gcc/godump.cc index 66e73ade7df..372871a59ec 100644 --- a/gcc/godump.cc +++ b/gcc/godump.cc @@ -1121,7 +1121,7 @@ go_output_typedef (class godump_container *container, tree decl) && !container->decls_seen.contains (TREE_TYPE (decl)) && (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE || !container->decls_seen.contains - (TYPE_CANONICAL (TREE_TYPE (decl))))) + (TYPE_MAIN_VARIANT (TREE_TYPE (decl))))) { tree element; diff --git a/gcc/testsuite/gcc.dg/enum-alias-1.c b/gcc/testsuite/gcc.dg/enum-alias-1.c new file mode 100644 index 00000000000..8fa30eb7897 --- /dev/null +++ b/gcc/testsuite/gcc.dg/enum-alias-1.c @@ -0,0 +1,24 @@ +/* { dg-do run } */ +/* { dg-options "-O2" } */ + +enum E { E1 = -1, E2 = 0, E3 = 1 }; + +typedef int A; +typedef enum E B; + +_Static_assert(_Generic((A){ 0 }, B: 1), ""); + +void* foo(void* a, void *b, A *c, B *d) +{ + *(A**)a = c; + *(B**)b = d; + return *(A**)a; +} + +int main() +{ + A *a, b, c; + if (&c != (A*)foo(&a, &a, &b, &c)) + __builtin_abort(); +} + diff --git a/gcc/testsuite/gcc.dg/enum-alias-2.c b/gcc/testsuite/gcc.dg/enum-alias-2.c new file mode 100644 index 00000000000..7ca3f3b2db8 --- /dev/null +++ b/gcc/testsuite/gcc.dg/enum-alias-2.c @@ -0,0 +1,25 @@ +/* { dg-do run } */ +/* { dg-options "-O2" } */ + +typedef int A; + +void* foo(void* a, void *b, void *c, void *d) +{ + *(A**)a = c; + + { + typedef enum E B; + enum E { E1 = -1, E2 = 0, E3 = 1 }; + *(B**)b = d; + } + + return *(A**)a; +} + +int main() +{ + A *a, b, c; + if (&c != (A*)foo(&a, &a, &b, &c)) + __builtin_abort(); +} + diff --git a/gcc/testsuite/gcc.dg/enum-alias-3.c b/gcc/testsuite/gcc.dg/enum-alias-3.c new file mode 100644 index 00000000000..36a4f02a455 --- /dev/null +++ b/gcc/testsuite/gcc.dg/enum-alias-3.c @@ -0,0 +1,26 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -flto" } */ + +typedef int *A; + +void* foo(void* a, void *b, void *c, void *d) +{ + *(A**)a = c; + + typedef enum E *B; + enum E { E1 = -1, E2 = 0, E3 = 1 }; + { + *(B**)b = d; + } + + return *(A**)a; +} + +int main() +{ + A *a, b, c; + if (&c != (A*)foo(&a, &a, &b, &c)) + __builtin_abort(); +} + +