* Fix verify_type ICE with -fshort-enum @ 2015-06-11 21:49 Jan Hubicka 2015-06-12 8:53 ` Marek Polacek 0 siblings, 1 reply; 5+ messages in thread From: Jan Hubicka @ 2015-06-11 21:49 UTC (permalink / raw) To: gcc-patches Hi, gcc.c-torture/execute/930408-1.c currently ICE on -fshort-enum target(s) because TYPE_PACKED is not consistently set among type variants. Bootstrapped/regtested ppc64le-linux, OK? Honza PR middle-end/66325 * c-decl.c (start_enum): Set TYPE_PACKED consistently among type variants. Index: c-decl.c =================================================================== --- c-decl.c (revision 224250) +++ c-decl.c (working copy) @@ -7946,7 +7946,8 @@ the_enum->enum_overflow = 0; if (flag_short_enums) - TYPE_PACKED (enumtype) = 1; + for (tree v = TYPE_MAIN_VARIANT (enumtype); v ;v = TYPE_NEXT_VARIANT (v)) + TYPE_PACKED (v) = 1; /* FIXME: This will issue a warning for a use of a type defined within sizeof in a statement expr. This is not terribly serious ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Fix verify_type ICE with -fshort-enum 2015-06-11 21:49 Fix verify_type ICE with -fshort-enum Jan Hubicka @ 2015-06-12 8:53 ` Marek Polacek 2015-06-16 11:14 ` Richard Biener 0 siblings, 1 reply; 5+ messages in thread From: Marek Polacek @ 2015-06-12 8:53 UTC (permalink / raw) To: Jan Hubicka; +Cc: gcc-patches On Thu, Jun 11, 2015 at 11:47:43PM +0200, Jan Hubicka wrote: > Hi, > gcc.c-torture/execute/930408-1.c currently ICE on -fshort-enum target(s) because > TYPE_PACKED is not consistently set among type variants. I guess that's because of the forward declaration in the test. But I have no access to an ARM machine, so can't verify. > Bootstrapped/regtested ppc64le-linux, OK? > Honza > > PR middle-end/66325 > * c-decl.c (start_enum): Set TYPE_PACKED consistently among type variants. > Index: c-decl.c > =================================================================== > --- c-decl.c (revision 224250) > +++ c-decl.c (working copy) > @@ -7946,7 +7946,8 @@ > the_enum->enum_overflow = 0; > > if (flag_short_enums) > - TYPE_PACKED (enumtype) = 1; > + for (tree v = TYPE_MAIN_VARIANT (enumtype); v ;v = TYPE_NEXT_VARIANT (v)) Please fix the formatting here: no space before ;. Ok with that change. Marek ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Fix verify_type ICE with -fshort-enum 2015-06-12 8:53 ` Marek Polacek @ 2015-06-16 11:14 ` Richard Biener 2015-06-16 22:16 ` Jan Hubicka 0 siblings, 1 reply; 5+ messages in thread From: Richard Biener @ 2015-06-16 11:14 UTC (permalink / raw) To: Marek Polacek; +Cc: Jan Hubicka, GCC Patches On Fri, Jun 12, 2015 at 10:00 AM, Marek Polacek <polacek@redhat.com> wrote: > On Thu, Jun 11, 2015 at 11:47:43PM +0200, Jan Hubicka wrote: >> Hi, >> gcc.c-torture/execute/930408-1.c currently ICE on -fshort-enum target(s) because >> TYPE_PACKED is not consistently set among type variants. > > I guess that's because of the forward declaration in the test. But I have > no access to an ARM machine, so can't verify. > >> Bootstrapped/regtested ppc64le-linux, OK? >> Honza >> >> PR middle-end/66325 >> * c-decl.c (start_enum): Set TYPE_PACKED consistently among type variants. >> Index: c-decl.c >> =================================================================== >> --- c-decl.c (revision 224250) >> +++ c-decl.c (working copy) >> @@ -7946,7 +7946,8 @@ >> the_enum->enum_overflow = 0; >> >> if (flag_short_enums) >> - TYPE_PACKED (enumtype) = 1; >> + for (tree v = TYPE_MAIN_VARIANT (enumtype); v ;v = TYPE_NEXT_VARIANT (v)) Though I wonder why flag_short_enums was not true when building the (main-)variant? Looks like -fshort-enums is also 'Optimization', so the above is bogus for enum foo {x = 1 }; void __attribute__((optimize(short-enums))) foo() { const enum foo x = 1; } no? The main variant is correctly _not_ packed but now you make it packed as you reach foo ()? Richard. > Please fix the formatting here: no space before ;. > > Ok with that change. > > Marek ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Fix verify_type ICE with -fshort-enum 2015-06-16 11:14 ` Richard Biener @ 2015-06-16 22:16 ` Jan Hubicka 2015-06-17 10:51 ` Richard Biener 0 siblings, 1 reply; 5+ messages in thread From: Jan Hubicka @ 2015-06-16 22:16 UTC (permalink / raw) To: Richard Biener; +Cc: Marek Polacek, Jan Hubicka, GCC Patches > >> PR middle-end/66325 > >> * c-decl.c (start_enum): Set TYPE_PACKED consistently among type variants. > >> Index: c-decl.c > >> =================================================================== > >> --- c-decl.c (revision 224250) > >> +++ c-decl.c (working copy) > >> @@ -7946,7 +7946,8 @@ > >> the_enum->enum_overflow = 0; > >> > >> if (flag_short_enums) > >> - TYPE_PACKED (enumtype) = 1; > >> + for (tree v = TYPE_MAIN_VARIANT (enumtype); v ;v = TYPE_NEXT_VARIANT (v)) > > Though I wonder why flag_short_enums was not true when building the > (main-)variant? What I believe happens is that there is forward declaration of enum that leads to biuld incomplete enum type that has no packed flag set. Then we produce a type variant and after that we complete the main variant, but do not update the other variant. Actually looking into where the variant is updated, it happens in finish_enum that copies many flags, perhaps it would make more sense to copy TYPE_PACKED there? > Looks like -fshort-enums is also 'Optimization', so the above is bogus for > > enum foo {x = 1 }; > > void __attribute__((optimize(short-enums))) foo() > { > const enum foo x = 1; > } > > no? The main variant is correctly _not_ packed but now you make it > packed as you reach foo ()? Perhaps it is defined as Optimization but it does not bind to uses of types: enum foo {a,b,c}; __attribute__((optimize("short-enums"))) main() { const enum foo x; enum bar {a,b,c}; printf ("%i %i\n",sizeof (x), sizeof(enum bar)); } prints 4 1 it depends when the main variant is finished. I wonder if there is any practical value on support Optimization attribute for this kind of ABI breaking options. It may be easier to simply drop the Optimization flag completely from -fshort-enums and friends. -fshort-double ICEs at initialization time at least since 4.8.x $ gcc t.c -fshort-double <built-in>: internal compiler error: in layout_type, at stor-layout.c:2220 0xafe41b layout_type(tree_node*) ../../gcc/stor-layout.c:2219 so I suggest dropping that flag completely. Honza > > Richard. > > > Please fix the formatting here: no space before ;. > > > > Ok with that change. > > > > Marek ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Fix verify_type ICE with -fshort-enum 2015-06-16 22:16 ` Jan Hubicka @ 2015-06-17 10:51 ` Richard Biener 0 siblings, 0 replies; 5+ messages in thread From: Richard Biener @ 2015-06-17 10:51 UTC (permalink / raw) To: Jan Hubicka; +Cc: Marek Polacek, GCC Patches On Wed, Jun 17, 2015 at 12:10 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >> >> PR middle-end/66325 >> >> * c-decl.c (start_enum): Set TYPE_PACKED consistently among type variants. >> >> Index: c-decl.c >> >> =================================================================== >> >> --- c-decl.c (revision 224250) >> >> +++ c-decl.c (working copy) >> >> @@ -7946,7 +7946,8 @@ >> >> the_enum->enum_overflow = 0; >> >> >> >> if (flag_short_enums) >> >> - TYPE_PACKED (enumtype) = 1; >> >> + for (tree v = TYPE_MAIN_VARIANT (enumtype); v ;v = TYPE_NEXT_VARIANT (v)) >> >> Though I wonder why flag_short_enums was not true when building the >> (main-)variant? > > What I believe happens is that there is forward declaration of enum that leads > to biuld incomplete enum type that has no packed flag set. Then we produce > a type variant and after that we complete the main variant, but do not update > the other variant. > > Actually looking into where the variant is updated, it happens in finish_enum > that copies many flags, perhaps it would make more sense to copy TYPE_PACKED > there? Yes, it sounds like so. >> Looks like -fshort-enums is also 'Optimization', so the above is bogus for >> >> enum foo {x = 1 }; >> >> void __attribute__((optimize(short-enums))) foo() >> { >> const enum foo x = 1; >> } >> >> no? The main variant is correctly _not_ packed but now you make it >> packed as you reach foo ()? > > Perhaps it is defined as Optimization but it does not bind to uses of types: > > enum foo {a,b,c}; > __attribute__((optimize("short-enums"))) > main() > { > const enum foo x; > enum bar {a,b,c}; > printf ("%i %i\n",sizeof (x), sizeof(enum bar)); > } > > prints > 4 1 > > it depends when the main variant is finished. I wonder if there is any > practical value on support Optimization attribute for this kind of ABI breaking > options. It may be easier to simply drop the Optimization flag completely from > -fshort-enums and friends. > > -fshort-double ICEs at initialization time at least since 4.8.x > $ gcc t.c -fshort-double > <built-in>: internal compiler error: in layout_type, at stor-layout.c:2220 > 0xafe41b layout_type(tree_node*) > ../../gcc/stor-layout.c:2219 > > so I suggest dropping that flag completely. IIRC it "works" for -m32, but yes... Yes also to _not_ make ABI changing flags 'Optimization'. Richard. > Honza >> >> Richard. >> >> > Please fix the formatting here: no space before ;. >> > >> > Ok with that change. >> > >> > Marek ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-17 10:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-06-11 21:49 Fix verify_type ICE with -fshort-enum Jan Hubicka 2015-06-12 8:53 ` Marek Polacek 2015-06-16 11:14 ` Richard Biener 2015-06-16 22:16 ` Jan Hubicka 2015-06-17 10:51 ` Richard Biener
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).