[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 --- Comment #16 from Christophe Lyon --- Author: clyon Date: Fri Apr 5 15:10:12 2019 New Revision: 270168 URL: https://gcc.gnu.org/viewcvs?rev=270168=gcc=rev Log: [testsuite] PR71598: Fix testcases again 2019-04-05 Christophe Lyon PR c/71598 * gcc.dg/torture/pr71598-1.c: dg-prune arm linker messages about size of enums. * gcc.dg/torture/pr71598-2.c: Likewise. Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/torture/pr71598-1.c trunk/gcc/testsuite/gcc.dg/torture/pr71598-2.c
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 --- Comment #15 from Christophe Lyon --- Author: clyon Date: Wed Apr 3 13:17:04 2019 New Revision: 270126 URL: https://gcc.gnu.org/viewcvs?rev=270126=gcc=rev Log: [testsuite] PR71598: Fix testcases 2019-04-13 Christophe Lyon PR c/71598 * gcc.dg/torture/pr71598-1.c: Skip if short_enums target. * gcc.dg/torture/pr71598-2.c: Skip if not short_enums target. Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/torture/pr71598-1.c trunk/gcc/testsuite/gcc.dg/torture/pr71598-2.c
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 --- Comment #13 from Richard Biener --- Author: rguenth Date: Mon Apr 1 07:16:38 2019 New Revision: 270052 URL: https://gcc.gnu.org/viewcvs?rev=270052=gcc=rev Log: 2019-04-01 Richard Biener PR c/71598 * gimple.c: Include langhooks.h. (gimple_get_alias_set): Treat enumeral types as the underlying integer type. c/ * c-tree.h (c_get_alias_set): Declare. * c-objc-common.h (LANG_HOOKS_GET_ALIAS_SET): Use c_get_alias_set. * c-objc-common.c (c_get_alias_set): Treat enumeral types as the underlying integer type. * gcc.dg/torture/pr71598-1.c: New testcase. * gcc.dg/torture/pr71598-2.c: Likewise. * gcc.dg/torture/pr71598-3.c: Likewise. Added: trunk/gcc/testsuite/gcc.dg/torture/pr71598-1.c trunk/gcc/testsuite/gcc.dg/torture/pr71598-2.c trunk/gcc/testsuite/gcc.dg/torture/pr71598-3.c Modified: trunk/gcc/ChangeLog trunk/gcc/c/ChangeLog trunk/gcc/c/c-objc-common.c trunk/gcc/c/c-objc-common.h trunk/gcc/c/c-tree.h trunk/gcc/gimple.c trunk/gcc/testsuite/ChangeLog
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 Richard Biener changed: What|Removed |Added Status|ASSIGNED|RESOLVED Known to work||9.0 Resolution|--- |FIXED Known to fail||7.4.0, 8.3.0 --- Comment #14 from Richard Biener --- Fixed for GCC 9+.
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598
--- Comment #12 from Richard Biener ---
Created attachment 45973
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45973=edit
patch I am testing
I am testing the following. I needed to adjust the testcase a bit to make
the C++ FE happy, in particular casting the arguments to the function to
pointer-to-enum type and storing an enum member rather than an integer
(the conversion on the return stmts seems to work):
enum e1 { c1 };
__attribute__((noinline,noclone))
int f(enum e1 *p, unsigned *q)
{
*p = c1;
*q = 2;
return *p;
}
int main()
{
unsigned x;
if (f((enum e1 *), ) != 2)
__builtin_abort();
return 0;
}
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 --- Comment #11 from Eric Botcazou --- > I see. Do you prefer a langhook solution that would "fix" this only > for C/C++ and LTO then? That sounds like the best approach to me, but I'm no expert here. > OK, I see. VRP still expects it to exist though (just not for > pointer types). Anyhow, I'm probably leaning towards looking at > TYPE_SIZE. Hopefully "incomplete" enums do not exist ;) Yes, sorry, we do call set_min_and_max_values_for_integral_type to set the bounds on them so you're probably running into the dreaded circularity.
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 --- Comment #10 from rguenther at suse dot de --- On Fri, 15 Mar 2019, ebotcazou at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 > > --- Comment #9 from Eric Botcazou --- > > Btw, I tried to use TREE_TYPE (TYPE_MIN_VALUE ()) of the ENUMERAL_TYPE but > > that breaks with Ada (bah, no libbacktrace support there...): > > Probably because of: > > /* Note that the bounds are updated at the end of this function > to avoid an infinite recursion since they refer to the type. */ > goto discrete_type; > > > That is, input from language frontend maintainers is still needed as to > > how to get at the integer type an enum type has to be compatible with > > TBAA-wise and how to query whether there is any. For the above Ada issue > > the code could be amended to simply not do anything special for > > ENUMERAL_TYPE without a TYPE_MIN_VALUE. > > In Ada, enumeration and integer types are totally unrelated to each other. I see. Do you prefer a langhook solution that would "fix" this only for C/C++ and LTO then? > > I didn't yet try to debug what the Ada issue above is and what weird > > kind of ENUMERAL_TYPEs Ada has ... > > We probably don't set TYPE_MIN_VALUE at all to avoid the circularity. OK, I see. VRP still expects it to exist though (just not for pointer types). Anyhow, I'm probably leaning towards looking at TYPE_SIZE. Hopefully "incomplete" enums do not exist ;)
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 --- Comment #9 from Eric Botcazou --- > Btw, I tried to use TREE_TYPE (TYPE_MIN_VALUE ()) of the ENUMERAL_TYPE but > that breaks with Ada (bah, no libbacktrace support there...): Probably because of: /* Note that the bounds are updated at the end of this function to avoid an infinite recursion since they refer to the type. */ goto discrete_type; > That is, input from language frontend maintainers is still needed as to > how to get at the integer type an enum type has to be compatible with > TBAA-wise and how to query whether there is any. For the above Ada issue > the code could be amended to simply not do anything special for > ENUMERAL_TYPE without a TYPE_MIN_VALUE. In Ada, enumeration and integer types are totally unrelated to each other. > I didn't yet try to debug what the Ada issue above is and what weird > kind of ENUMERAL_TYPEs Ada has ... We probably don't set TYPE_MIN_VALUE at all to avoid the circularity.
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 Richard Biener changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org --- Comment #8 from Richard Biener --- Btw, I tried to use TREE_TYPE (TYPE_MIN_VALUE ()) of the ENUMERAL_TYPE but that breaks with Ada (bah, no libbacktrace support there...): make[3]: *** [/space/rguenther/src/svn/trunk/gcc/ada/gcc-interface/Make-lang.in:1033: ada/libgnat/a-except.o] Error 1 +===GNAT BUG DETECTED==+ | 9.0.1 20190314 (experimental) (x86_64-pc-linux-gnu) Storage_Error stack overflow or erroneous memory access| | Error detected at lib-xref-spark_specific.adb:39:37 | | Please submit a bug report; see https://gcc.gnu.org/bugs/ . | | Use a subject line meaningful to you and us to track the bug.| | Include the entire contents of this bug box in the report. | | Include the exact command that you entered. | | Also include sources listed below. | +==+ Please include these source files with error report btw, fixing this in get_alias_set would make this transitive (but exact behavior is still implementation defined!). That is, input from language frontend maintainers is still needed as to how to get at the integer type an enum type has to be compatible with TBAA-wise and how to query whether there is any. For the above Ada issue the code could be amended to simply not do anything special for ENUMERAL_TYPE without a TYPE_MIN_VALUE. For C I guess the type of TYPE_MIN_VALUE matches the type the enum is compatible with, for C++ I would hope so as well. Testcases covering also -fshort-enum should be easy to write. Note anything involving a langhook is going to not work with LTO. Note another possibility would be (given that most FEs, including LTO, make unsigned and signed type variants compatible) to look at the ENUMERAL_TYPEs mode and use the corresponding (unsigned) integer type as compatible type. That may not work for under-aligned enums though where the mode may end up as BLKmode... which means we'd need to look at TYPE_SIZE instead. I didn't yet try to debug what the Ada issue above is and what weird kind of ENUMERAL_TYPEs Ada has ...
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 --- Comment #7 from joseph at codesourcery dot com --- The relation definitely is not transitive (so you can't declare the same function to return two different enum types, for example).
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598
--- Comment #6 from Alexander Monakov ---
... and even considering that the standard never actually says that "compatible
type" relation is transitive, and so two enums technically need not be
compatible with each other, the following should follow the standard to the
letter, but is still optimized to 'return 1':
enum e1 { c1=2 };
int f(unsigned *p, enum e1 *q)
{
*p = 1;
*q = c1;
return *p;
}
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 --- Comment #5 from Alexander Monakov --- C11 6.7.2.2 p4 says, Each enumerated type shall be compatible with char, a signed integer type, or an unsigned integer type [...] and 6.5 p7 says, An object shall have its stored value accessed only by an lvalue expression that has one of the following types: * a type compatible with the effective type of the object (and the third bullet answers my implicit question in comment #2 about signed/unsigned aliases).
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 --- Comment #4 from Richard Biener --- Note I can't find any bit in the C standard that would make the testcase well-defined and support reporters view. My clang version doesn't "miscompile" it though. With LTO we'd treat both enum declarations as compatible wrt TBAA but they wouldn't be compatible with unsigned int (so in GCC speak both enums get the same TYPE_CANONICAL but that isn't TYPE_CANONICAL of unsigned int).
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 Richard Biener changed: What|Removed |Added CC||ebotcazou at gcc dot gnu.org, ||jason at gcc dot gnu.org, ||jsm28 at gcc dot gnu.org --- Comment #3 from Richard Biener --- get_alias_set does nothing special for ENUMERAL_TYPE which means distinct enums get distinct alias sets. Iff the C standard says that from a TBAA perspective an enumeral type is to be treated as one of the standard integer types then we need a way to represent such type in the middle-end or the frontends need to not expose the distinct ENUMERAL_TYPE (at least in accesses) but whatever underlying type is to be used. Note since this targets the middle-end behavior the required language semantics of other languages play a role as well. A conservative middle-end only patch could look like the following - possibly also a good idea for LTO. Eventually we could settle on the ENUMERAL_TYPEs TYPE_VALUES first entry type to specify that underlying type, but that would need agreement. It looks like TREE_TYPE of an ENUMERAL_TYPE is also not "taken" (but all FEs needing the get_alias_set treatment would need adjustment, at least it could be conditional on TREE_TYPE set). The TYPE_VALUES idea would leave us with no way to handle empty enums. Another thing to look at would be TREE_TYPE (TYPE_MIN/MAX_VALUE), for the testcase and the C FE it is unsigned int. diff --git a/gcc/alias.c b/gcc/alias.c index b64e3ea264d..8fbf34bf4dc 100644 --- a/gcc/alias.c +++ b/gcc/alias.c @@ -931,6 +931,11 @@ get_alias_set (tree t) if (set != -1) return set; + /* Enums inter-operate with the corresponding integer type. */ + else if (TREE_CODE (t) == ENUMERAL_TYPE) +return get_alias_set (build_nonstandard_integer_type + (TYPE_PRECISION (t), TYPE_UNSIGNED (t))); + /* There are no objects of FUNCTION_TYPE, so there's no point in using up an alias set for them. (There are, of course, pointers and references to functions, but that's different.) */
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598
Alexander Monakov changed:
What|Removed |Added
Keywords||wrong-code
Last reconfirmed|2017-07-29 00:00:00 |2019-3-14
CC||amonakov at gcc dot gnu.org,
||rguenth at gcc dot gnu.org
--- Comment #2 from Alexander Monakov ---
Optimized to 'return 1' already in fre1.
At the same time, accesses via 'int *' vs. 'unsigned *' potentially conflict in
int f(int *p, unsigned *q)
{
*p = 1;
*q = 2;
return *p;
}
[Bug c/71598] Wrong optimization with aliasing enums
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71598 Eric Gallager changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2017-07-29 CC||egallager at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #1 from Eric Gallager --- Confirmed (that the testcase aborts when run)
