| Message ID | 4F1D72CA.1060908@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
> and I think applying strict-volatile-bitfields to enums is probably > meaningless. MCU vendors would disagree. If a location is volatile, it's most likely hardware, and must be accessed in the user-specified way. Randomly accessing adjacent locations could cause system failure.
On 01/23/2012 08:51 PM, DJ Delorie wrote: > >> and I think applying strict-volatile-bitfields to enums is probably >> meaningless. > > MCU vendors would disagree. If a location is volatile, it's most > likely hardware, and must be accessed in the user-specified way. > Randomly accessing adjacent locations could cause system failure. This is not an issue here. The optimization in question in stor-layout tries to find out if a bitfield (volatile or not) is laid out in a way that's equivalent to a natural machine mode. For example, a 2-byte aligned 16 bit value can be HImode on most targets. The earlier patch restricts this optimization for non-volatile bitfields as well, on the grounds that an object may later be declared as volatile even if the bitfield isn't declared volatile in the struct definition. Strict-volatile-bitfields is concerned with the opposite case: ensuring that an "int x:8;" is accessed only with int-sized accesses. My thought was that this is probably pointless with enums, which I think aren't usually thought of as a specific size unlike char/short/int. So, if we see "enum something x:8;" we don't want to prevent the compiler from treating this as a plain QImode non-bitfield member. Bernd
Hi! How do we move this issue forward? On Mon, 23 Jan 2012 15:46:34 +0100, Bernd Schmidt <bernds@codesourcery.com> wrote: > On 11/29/2011 05:35 PM, Mitchell, Mark wrote: > >>> So, I still think this patch is the best way to go forward, and it > >> does > >>> fix incorrect code generation. Would appreciate an OK. > >> > >> Ping. > > > > If you don't hear any objections within a week, please proceed. > > That was committed a while ago. The part in stor-layout.c that stops us > from promoting bitfields to normal fields apparently caused some > testsuite regressions in sh tests, where some optimization dump scans > show that we can't perform the optimizations if there are BIT_FIELD_REFs > rather than a normal member access. > > The testcases use things like > enum something field:8; > and I think applying strict-volatile-bitfields to enums is probably > meaningless. Should we adjust semantics so that the compiler is free to > optimize enum bitfields? The patch would look something like the below. > Thomas has tested this on arm and sh with our internal tree. > > > Bernd > > > Index: gcc/stor-layout.c > =================================================================== > --- gcc/stor-layout.c (revision 355696) > +++ gcc/stor-layout.c (working copy) > @@ -665,8 +665,7 @@ > may make a volatile object later. */ > if (TYPE_SIZE (type) != 0 > && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST > - && GET_MODE_CLASS (TYPE_MODE (type)) == MODE_INT > - && flag_strict_volatile_bitfields <= 0) > + && GET_MODE_CLASS (TYPE_MODE (type)) == MODE_INT) > { > enum machine_mode xmode > = mode_for_size_tree (DECL_SIZE (decl), MODE_INT, 1); > @@ -674,7 +673,12 @@ > > if (xmode != BLKmode > && !(xalign > BITS_PER_UNIT && DECL_PACKED (decl)) > - && (known_align == 0 || known_align >= xalign)) > + && (known_align == 0 || known_align >= xalign) > + && (flag_strict_volatile_bitfields <= 0 > + /* Same size makes strict volatile bitfields > meaningless. */ > + || GET_MODE_SIZE (xmode) == GET_MODE_SIZE > (TYPE_MODE (type)) > + /* Strict volatile bitfields shouldn't apply to > enums. */ > + || TREE_CODE (type) == ENUMERAL_TYPE)) > { > DECL_ALIGN (decl) = MAX (xalign, DECL_ALIGN (decl)); > DECL_MODE (decl) = xmode; > Grüße, Thomas
On Fri, Feb 17, 2012 at 9:51 PM, Thomas Schwinge <thomas@codesourcery.com> wrote: > Hi! > > How do we move this issue forward? > > On Mon, 23 Jan 2012 15:46:34 +0100, Bernd Schmidt <bernds@codesourcery.com> wrote: >> On 11/29/2011 05:35 PM, Mitchell, Mark wrote: >> >>> So, I still think this patch is the best way to go forward, and it >> >> does >> >>> fix incorrect code generation. Would appreciate an OK. >> >> >> >> Ping. >> > >> > If you don't hear any objections within a week, please proceed. >> >> That was committed a while ago. The part in stor-layout.c that stops us >> from promoting bitfields to normal fields apparently caused some >> testsuite regressions in sh tests, where some optimization dump scans >> show that we can't perform the optimizations if there are BIT_FIELD_REFs >> rather than a normal member access. >> >> The testcases use things like >> enum something field:8; >> and I think applying strict-volatile-bitfields to enums is probably >> meaningless. Should we adjust semantics so that the compiler is free to >> optimize enum bitfields? The patch would look something like the below. >> Thomas has tested this on arm and sh with our internal tree. >> >> >> Bernd >> >> >> Index: gcc/stor-layout.c >> =================================================================== >> --- gcc/stor-layout.c (revision 355696) >> +++ gcc/stor-layout.c (working copy) >> @@ -665,8 +665,7 @@ >> may make a volatile object later. */ >> if (TYPE_SIZE (type) != 0 >> && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST >> - && GET_MODE_CLASS (TYPE_MODE (type)) == MODE_INT >> - && flag_strict_volatile_bitfields <= 0) >> + && GET_MODE_CLASS (TYPE_MODE (type)) == MODE_INT) >> { >> enum machine_mode xmode >> = mode_for_size_tree (DECL_SIZE (decl), MODE_INT, 1); >> @@ -674,7 +673,12 @@ >> >> if (xmode != BLKmode >> && !(xalign > BITS_PER_UNIT && DECL_PACKED (decl)) >> - && (known_align == 0 || known_align >= xalign)) >> + && (known_align == 0 || known_align >= xalign) >> + && (flag_strict_volatile_bitfields <= 0 >> + /* Same size makes strict volatile bitfields >> meaningless. */ >> + || GET_MODE_SIZE (xmode) == GET_MODE_SIZE >> (TYPE_MODE (type)) >> + /* Strict volatile bitfields shouldn't apply to >> enums. */ >> + || TREE_CODE (type) == ENUMERAL_TYPE)) What about BOOLEAN_TYPE bitfields? Thus, why not explicitely spell out && TREE_CODE (type) == INTEGER_TYPE? >> { >> DECL_ALIGN (decl) = MAX (xalign, DECL_ALIGN (decl)); >> DECL_MODE (decl) = xmode; >> > > > Grüße, > Thomas
On 02/20/2012 12:14 PM, Richard Guenther wrote: > On Fri, Feb 17, 2012 at 9:51 PM, Thomas Schwinge > <thomas@codesourcery.com> wrote: >> Hi! >> >> How do we move this issue forward? >> >> On Mon, 23 Jan 2012 15:46:34 +0100, Bernd Schmidt <bernds@codesourcery.com> wrote: >>> That was committed a while ago. The part in stor-layout.c that stops us >>> from promoting bitfields to normal fields apparently caused some >>> testsuite regressions in sh tests, where some optimization dump scans >>> show that we can't perform the optimizations if there are BIT_FIELD_REFs >>> rather than a normal member access. >>> >>> The testcases use things like >>> enum something field:8; >>> and I think applying strict-volatile-bitfields to enums is probably >>> meaningless. Should we adjust semantics so that the compiler is free to >>> optimize enum bitfields? The patch would look something like the below. > > What about BOOLEAN_TYPE bitfields? Thus, why not explicitely > spell out && TREE_CODE (type) == INTEGER_TYPE? That would work for me, if we can all agree that -fstrict-volatile-bitfields should be restricted to INTEGER_TYPE. Bernd
On 20/02/12 17:28, Bernd Schmidt wrote: > On 02/20/2012 12:14 PM, Richard Guenther wrote: >> On Fri, Feb 17, 2012 at 9:51 PM, Thomas Schwinge >> <thomas@codesourcery.com> wrote: >>> Hi! >>> >>> How do we move this issue forward? >>> >>> On Mon, 23 Jan 2012 15:46:34 +0100, Bernd Schmidt <bernds@codesourcery.com> wrote: >>>> That was committed a while ago. The part in stor-layout.c that stops us >>>> from promoting bitfields to normal fields apparently caused some >>>> testsuite regressions in sh tests, where some optimization dump scans >>>> show that we can't perform the optimizations if there are BIT_FIELD_REFs >>>> rather than a normal member access. >>>> >>>> The testcases use things like >>>> enum something field:8; >>>> and I think applying strict-volatile-bitfields to enums is probably >>>> meaningless. Should we adjust semantics so that the compiler is free to >>>> optimize enum bitfields? The patch would look something like the below. > >> >> What about BOOLEAN_TYPE bitfields? Thus, why not explicitely >> spell out && TREE_CODE (type) == INTEGER_TYPE? > > That would work for me, if we can all agree that > -fstrict-volatile-bitfields should be restricted to INTEGER_TYPE. > > > Bernd > I'm not sure why it should be. Can't a user write #ifdef __cplusplus #define BOOL bool #else #define bool _Bool #endif struct x { volatile BOOL a : 1; volatile BOOL b : 1; volatile unsigned char c : 6; volatile BOOL d : 1; } y; ? If you've got strict volatile bitfields, then the concept here is that the access uses the declared type for accessing the member. Since in the ABI bool has a defined size, then it should access the member using that size. On ARM, sizeof bool is 1, so I'd take the above to mean that accessing y.a to mean a read of a, b and c, but not d. R.
On 02/20/2012 06:39 PM, Richard Earnshaw wrote: > I'm not sure why it should be. Can't a user write > > #ifdef __cplusplus > #define BOOL bool > #else > #define bool _Bool > #endif > > struct x { > volatile BOOL a : 1; > volatile BOOL b : 1; > volatile unsigned char c : 6; > volatile BOOL d : 1; > } y; > > ? > > If you've got strict volatile bitfields, then the concept here is that > the access uses the declared type for accessing the member. Since in > the ABI bool has a defined size, then it should access the member using > that size. > > On ARM, sizeof bool is 1, so I'd take the above to mean that accessing > y.a to mean a read of a, b and c, but not d. What are your thoughts on the argument about enums? Bernd
On 20/02/12 17:51, Bernd Schmidt wrote: > On 02/20/2012 06:39 PM, Richard Earnshaw wrote: >> I'm not sure why it should be. Can't a user write >> >> #ifdef __cplusplus >> #define BOOL bool >> #else >> #define bool _Bool >> #endif >> >> struct x { >> volatile BOOL a : 1; >> volatile BOOL b : 1; >> volatile unsigned char c : 6; >> volatile BOOL d : 1; >> } y; >> >> ? >> >> If you've got strict volatile bitfields, then the concept here is that >> the access uses the declared type for accessing the member. Since in >> the ABI bool has a defined size, then it should access the member using >> that size. >> >> On ARM, sizeof bool is 1, so I'd take the above to mean that accessing >> y.a to mean a read of a, b and c, but not d. > > What are your thoughts on the argument about enums? > > > Bernd > Similar. A particular enumeration type has a defined size, so accesses should use that size. R.
Index: gcc/stor-layout.c =================================================================== --- gcc/stor-layout.c (revision 355696) +++ gcc/stor-layout.c (working copy) @@ -665,8 +665,7 @@ may make a volatile object later. */ if (TYPE_SIZE (type) != 0 && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST - && GET_MODE_CLASS (TYPE_MODE (type)) == MODE_INT - && flag_strict_volatile_bitfields <= 0) + && GET_MODE_CLASS (TYPE_MODE (type)) == MODE_INT) { enum machine_mode xmode = mode_for_size_tree (DECL_SIZE (decl), MODE_INT, 1); @@ -674,7 +673,12 @@ if (xmode != BLKmode && !(xalign > BITS_PER_UNIT && DECL_PACKED (decl)) - && (known_align == 0 || known_align >= xalign)) + && (known_align == 0 || known_align >= xalign) + && (flag_strict_volatile_bitfields <= 0 + /* Same size makes strict volatile bitfields meaningless. */ + || GET_MODE_SIZE (xmode) == GET_MODE_SIZE (TYPE_MODE (type)) + /* Strict volatile bitfields shouldn't apply to enums. */ + || TREE_CODE (type) == ENUMERAL_TYPE))