* Still fails with strict-volatile-bitfields @ 2014-01-09 8:26 Joey Ye 2014-01-09 10:45 ` Bernd Edlinger 0 siblings, 1 reply; 13+ messages in thread From: Joey Ye @ 2014-01-09 8:26 UTC (permalink / raw) To: gcc, sandra, bernd.edlinger Sandra, Bernd, Can you take a look at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 It seems a siimple case still doesn't work as expected. Did I miss anything? Thanks, Joey ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: Still fails with strict-volatile-bitfields 2014-01-09 8:26 Still fails with strict-volatile-bitfields Joey Ye @ 2014-01-09 10:45 ` Bernd Edlinger 2014-01-09 16:30 ` Richard Earnshaw 0 siblings, 1 reply; 13+ messages in thread From: Bernd Edlinger @ 2014-01-09 10:45 UTC (permalink / raw) To: Joey Ye, gcc, sandra Hi, On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote: > > Sandra, Bernd, > > Can you take a look at > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 > > It seems a siimple case still doesn't work as expected. Did I miss anything? > > Thanks, > Joey Yes, this is a major case where the C++ memory model is in conflict with AAPCS. You can get the expected code, by changing the struct like this: struct str { volatile unsigned f1: 8; unsigned dummy:24; }; If it is written this way the C++ memory model allows QImode, HImode, SImode. And -fstrict-volatile-bitfields demands SImode, so the conflict is resolved. This dummy member makes only a difference on the C level, and is completely invisible in the generated code. If -fstrict-volatile-bitfields is now given, we use SImode, if -fno-strict-volatile-bitfields is given, we give GCC the freedom to choose the access mode, maybe QImode if that is faster. In the _very_ difficult process to find an solution that seems to be acceptable to all maintainers, we came to the solution, that we need to adhere to the C++ memory model by default. And we may not change the default setting of -fstruct-volatile-bitfields at the same time! As a future extension we discussed the possibility to add a new option -fstrict-volatile-bitfields=aapcs that explicitly allows us to break the C++ memory model. But I did not yet try to implement this, as I feel that would certainly not be accepted as we are in Phase3 now. As another future extension there was the discussion about the -Wportable-volatility warning, which I see now as a warning that analyzes the structure layout and warns about any structures that are not "well-formed", in the sense, that a bit-field fails to define all bits of the container. Those people that do use bit-fields to access device-registers do always define all bits, and of course in the same mode. It would be good to have a warning, when some bits are missing. They currently have to use great care to check their structures manually. I had a proposal for that warning but that concentrated only on the volatile attribute, but I will have to re-write that completely so that it can be done in stor-layout.c: It should warn independent of optimization levels or actual bitfield member references, thus, be implemented entirely at the time we lay out the structure. The well-formed-ness of a bit-field makes that possible. But that will come too late for Phase3 as well. Regards Bernd. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Still fails with strict-volatile-bitfields 2014-01-09 10:45 ` Bernd Edlinger @ 2014-01-09 16:30 ` Richard Earnshaw 2014-01-10 8:56 ` Bernd Edlinger 0 siblings, 1 reply; 13+ messages in thread From: Richard Earnshaw @ 2014-01-09 16:30 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Joey Ye, gcc, sandra On 09/01/14 08:26, Bernd Edlinger wrote: > Hi, > > On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote: >> >> Sandra, Bernd, >> >> Can you take a look at >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 >> >> It seems a siimple case still doesn't work as expected. Did I miss anything? >> >> Thanks, >> Joey > > Yes, > > this is a major case where the C++ memory model is > in conflict with AAPCS. > Does the compiler warn about this? And if so, is the warning on by default? I think it's essential that we don't have quiet changes in behaviour without such warnings. R. > You can get the expected code, by changing the struct > like this: > > struct str { > volatile unsigned f1: 8; > unsigned dummy:24; > }; > > If it is written this way the C++ memory model allows > QImode, HImode, SImode. And -fstrict-volatile-bitfields > demands SImode, so the conflict is resolved. This dummy > member makes only a difference on the C level, and is > completely invisible in the generated code. > > If -fstrict-volatile-bitfields is now given, we use SImode, > if -fno-strict-volatile-bitfields is given, we give GCC the > freedom to choose the access mode, maybe QImode if that is > faster. > > In the _very_ difficult process to find an solution > that seems to be acceptable to all maintainers, we came to > the solution, that we need to adhere to the C++ memory > model by default. And we may not change the default > setting of -fstruct-volatile-bitfields at the same time! > > As a future extension we discussed the possibility > to add a new option -fstrict-volatile-bitfields=aapcs > that explicitly allows us to break the C++ memory model. > > But I did not yet try to implement this, as I feel that > would certainly not be accepted as we are in Phase3 now. > > As another future extension there was the discussion > about the -Wportable-volatility warning, which I see now > as a warning that analyzes the structure layout and > warns about any structures that are not "well-formed", > in the sense, that a bit-field fails to define all > bits of the container. > > Those people that do use bit-fields to access device-registers > do always define all bits, and of course in the same mode. > > It would be good to have a warning, when some bits are missing. > They currently have to use great care to check their structures > manually. > > I had a proposal for that warning but that concentrated > only on the volatile attribute, but I will have to re-write > that completely so that it can be done in stor-layout.c: > > It should warn independent of optimization levels or actual > bitfield member references, thus, be implemented entirely at > the time we lay out the structure. The well-formed-ness of > a bit-field makes that possible. > > But that will come too late for Phase3 as well. > > > Regards > Bernd. > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: Still fails with strict-volatile-bitfields 2014-01-09 16:30 ` Richard Earnshaw @ 2014-01-10 8:56 ` Bernd Edlinger 2014-01-10 9:02 ` Eric Botcazou ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Bernd Edlinger @ 2014-01-10 8:56 UTC (permalink / raw) To: Richard Earnshaw; +Cc: Joey Ye, gcc, Sandra Loosemore On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote: > > On 09/01/14 08:26, Bernd Edlinger wrote: >> Hi, >> >> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote: >>> >>> Sandra, Bernd, >>> >>> Can you take a look at >>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 >>> >>> It seems a siimple case still doesn't work as expected. Did I miss anything? >>> >>> Thanks, >>> Joey >> >> Yes, >> >> this is a major case where the C++ memory model is >> in conflict with AAPCS. >> > > Does the compiler warn about this? And if so, is the warning on by > default? I think it's essential that we don't have quiet changes in > behaviour without such warnings. > > R. No. This example was working in 4.6 and broken in 4.7 and 4.8. Well, 4.7 should have warned about that. 4.9 does simply not change that, because it would violate the C++ memory model. Maybe that should go into release notes. At the same time we had all kinds of invalid code generation, starting at 4.6, especially with packed structures in C and Ada(!), (writing not all bits, and using unaligned accesses) and that is fixed in 4.9. Bernd. > >> You can get the expected code, by changing the struct >> like this: >> >> struct str { >> volatile unsigned f1: 8; >> unsigned dummy:24; >> }; >> >> If it is written this way the C++ memory model allows >> QImode, HImode, SImode. And -fstrict-volatile-bitfields >> demands SImode, so the conflict is resolved. This dummy >> member makes only a difference on the C level, and is >> completely invisible in the generated code. >> >> If -fstrict-volatile-bitfields is now given, we use SImode, >> if -fno-strict-volatile-bitfields is given, we give GCC the >> freedom to choose the access mode, maybe QImode if that is >> faster. >> >> In the _very_ difficult process to find an solution >> that seems to be acceptable to all maintainers, we came to >> the solution, that we need to adhere to the C++ memory >> model by default. And we may not change the default >> setting of -fstruct-volatile-bitfields at the same time! >> >> As a future extension we discussed the possibility >> to add a new option -fstrict-volatile-bitfields=aapcs >> that explicitly allows us to break the C++ memory model. >> >> But I did not yet try to implement this, as I feel that >> would certainly not be accepted as we are in Phase3 now. >> >> As another future extension there was the discussion >> about the -Wportable-volatility warning, which I see now >> as a warning that analyzes the structure layout and >> warns about any structures that are not "well-formed", >> in the sense, that a bit-field fails to define all >> bits of the container. >> >> Those people that do use bit-fields to access device-registers >> do always define all bits, and of course in the same mode. >> >> It would be good to have a warning, when some bits are missing. >> They currently have to use great care to check their structures >> manually. >> >> I had a proposal for that warning but that concentrated >> only on the volatile attribute, but I will have to re-write >> that completely so that it can be done in stor-layout.c: >> >> It should warn independent of optimization levels or actual >> bitfield member references, thus, be implemented entirely at >> the time we lay out the structure. The well-formed-ness of >> a bit-field makes that possible. >> >> But that will come too late for Phase3 as well. >> >> >> Regards >> Bernd. >> >> > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Still fails with strict-volatile-bitfields 2014-01-10 8:56 ` Bernd Edlinger @ 2014-01-10 9:02 ` Eric Botcazou 2014-01-10 9:11 ` Eric Botcazou 2014-01-10 11:52 ` Richard Earnshaw 2 siblings, 0 replies; 13+ messages in thread From: Eric Botcazou @ 2014-01-10 9:02 UTC (permalink / raw) To: Bernd Edlinger; +Cc: gcc, Richard Earnshaw, Joey Ye, Sandra Loosemore > No. This example was working in 4.6 and broken in 4.7 and 4.8. > Well, 4.7 should have warned about that. The 4.7 branch is not closed so it's not too late to add the warning there. -- Eric Botcazou ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Still fails with strict-volatile-bitfields 2014-01-10 8:56 ` Bernd Edlinger 2014-01-10 9:02 ` Eric Botcazou @ 2014-01-10 9:11 ` Eric Botcazou 2014-01-10 11:52 ` Richard Earnshaw 2 siblings, 0 replies; 13+ messages in thread From: Eric Botcazou @ 2014-01-10 9:11 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Joey Ye, gcc, Richard Earnshaw, Sandra Loosemore > No. This example was working in 4.6 and broken in 4.7 and 4.8. Note that it probably broke in 4.7.1 because the DECL_BIT_FIELD_REPRESENTATIVE stuff was backported after the initial 4.7.0 release. -- Eric Botcazou ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Still fails with strict-volatile-bitfields 2014-01-10 8:56 ` Bernd Edlinger 2014-01-10 9:02 ` Eric Botcazou 2014-01-10 9:11 ` Eric Botcazou @ 2014-01-10 11:52 ` Richard Earnshaw 2014-01-10 13:45 ` Bernd Edlinger 2 siblings, 1 reply; 13+ messages in thread From: Richard Earnshaw @ 2014-01-10 11:52 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Joey Ye, gcc, Sandra Loosemore On 10/01/14 08:49, Bernd Edlinger wrote: > On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote: >> >> On 09/01/14 08:26, Bernd Edlinger wrote: >>> Hi, >>> >>> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote: >>>> >>>> Sandra, Bernd, >>>> >>>> Can you take a look at >>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 >>>> >>>> It seems a siimple case still doesn't work as expected. Did I miss anything? >>>> >>>> Thanks, >>>> Joey >>> >>> Yes, >>> >>> this is a major case where the C++ memory model is >>> in conflict with AAPCS. >>> >> >> Does the compiler warn about this? And if so, is the warning on by >> default? I think it's essential that we don't have quiet changes in >> behaviour without such warnings. >> >> R. > > No. This example was working in 4.6 and broken in 4.7 and 4.8. > Well, 4.7 should have warned about that. > > 4.9 does simply not change that, because it would violate the C++ > memory model. Maybe that should go into release notes. That's no excuse for not generating a warning at compile time when the situation is encountered. Users of this feature are experiencing a quiet change of behaviour and that's unacceptable, even if the compiler is doing what it was intended to do; that doesn't necessarily match the user's expectations. There should always be a way to rewrite the C11 intended semantics in a way that doesn't violate the strict volatile bitfields semantics. > > At the same time we had all kinds of invalid code generation, > starting at 4.6, especially with packed structures in C and Ada(!), > (writing not all bits, and using unaligned accesses) > and that is fixed in 4.9. > I'm not worried about packed structures. You can't sensibly implement the strict volatile bitfields rules when things aren't aligned. R. > > Bernd. > >> >>> You can get the expected code, by changing the struct >>> like this: >>> >>> struct str { >>> volatile unsigned f1: 8; >>> unsigned dummy:24; >>> }; >>> >>> If it is written this way the C++ memory model allows >>> QImode, HImode, SImode. And -fstrict-volatile-bitfields >>> demands SImode, so the conflict is resolved. This dummy >>> member makes only a difference on the C level, and is >>> completely invisible in the generated code. >>> >>> If -fstrict-volatile-bitfields is now given, we use SImode, >>> if -fno-strict-volatile-bitfields is given, we give GCC the >>> freedom to choose the access mode, maybe QImode if that is >>> faster. >>> >>> In the _very_ difficult process to find an solution >>> that seems to be acceptable to all maintainers, we came to >>> the solution, that we need to adhere to the C++ memory >>> model by default. And we may not change the default >>> setting of -fstruct-volatile-bitfields at the same time! >>> >>> As a future extension we discussed the possibility >>> to add a new option -fstrict-volatile-bitfields=aapcs >>> that explicitly allows us to break the C++ memory model. >>> >>> But I did not yet try to implement this, as I feel that >>> would certainly not be accepted as we are in Phase3 now. >>> >>> As another future extension there was the discussion >>> about the -Wportable-volatility warning, which I see now >>> as a warning that analyzes the structure layout and >>> warns about any structures that are not "well-formed", >>> in the sense, that a bit-field fails to define all >>> bits of the container. >>> >>> Those people that do use bit-fields to access device-registers >>> do always define all bits, and of course in the same mode. >>> >>> It would be good to have a warning, when some bits are missing. >>> They currently have to use great care to check their structures >>> manually. >>> >>> I had a proposal for that warning but that concentrated >>> only on the volatile attribute, but I will have to re-write >>> that completely so that it can be done in stor-layout.c: >>> >>> It should warn independent of optimization levels or actual >>> bitfield member references, thus, be implemented entirely at >>> the time we lay out the structure. The well-formed-ness of >>> a bit-field makes that possible. >>> >>> But that will come too late for Phase3 as well. >>> >>> >>> Regards >>> Bernd. >>> >>> >> >> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: Still fails with strict-volatile-bitfields 2014-01-10 11:52 ` Richard Earnshaw @ 2014-01-10 13:45 ` Bernd Edlinger 2014-01-10 14:12 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Bernd Edlinger @ 2014-01-10 13:45 UTC (permalink / raw) To: Richard Earnshaw; +Cc: Joey Ye, gcc, Sandra Loosemore On, Fri, 10 Jan 2014 09:41:06, Richard Earnshaw wrote: > > On 10/01/14 08:49, Bernd Edlinger wrote: >> On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote: >>> >>> On 09/01/14 08:26, Bernd Edlinger wrote: >>>> Hi, >>>> >>>> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote: >>>>> >>>>> Sandra, Bernd, >>>>> >>>>> Can you take a look at >>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 >>>>> >>>>> It seems a siimple case still doesn't work as expected. Did I miss anything? >>>>> >>>>> Thanks, >>>>> Joey >>>> >>>> Yes, >>>> >>>> this is a major case where the C++ memory model is >>>> in conflict with AAPCS. >>>> >>> >>> Does the compiler warn about this? And if so, is the warning on by >>> default? I think it's essential that we don't have quiet changes in >>> behaviour without such warnings. >>> >>> R. >> >> No. This example was working in 4.6 and broken in 4.7 and 4.8. >> Well, 4.7 should have warned about that. >> >> 4.9 does simply not change that, because it would violate the C++ >> memory model. Maybe that should go into release notes. > > That's no excuse for not generating a warning at compile time when the > situation is encountered. Users of this feature are experiencing a > quiet change of behaviour and that's unacceptable, even if the compiler > is doing what it was intended to do; that doesn't necessarily match the > user's expectations. There should always be a way to rewrite the C11 > intended semantics in a way that doesn't violate the strict volatile > bitfields semantics. > Hmm... You mean we should have a (ugly) warning enabled by default in 4.9 (at expmed.c) if a bit-field access would be perfectly aligned and so, but is in conflict with the C++ memory model, and -fstrict-volatile-bitfields is in effect. Maybe only once per compilation? >> >> At the same time we had all kinds of invalid code generation, >> starting at 4.6, especially with packed structures in C and Ada(!), >> (writing not all bits, and using unaligned accesses) >> and that is fixed in 4.9. >> > > I'm not worried about packed structures. You can't sensibly implement > the strict volatile bitfields rules when things aren't aligned. > > R. > >> >> Bernd. >> >>> >>>> You can get the expected code, by changing the struct >>>> like this: >>>> >>>> struct str { >>>> volatile unsigned f1: 8; >>>> unsigned dummy:24; >>>> }; >>>> >>>> If it is written this way the C++ memory model allows >>>> QImode, HImode, SImode. And -fstrict-volatile-bitfields >>>> demands SImode, so the conflict is resolved. This dummy >>>> member makes only a difference on the C level, and is >>>> completely invisible in the generated code. >>>> >>>> If -fstrict-volatile-bitfields is now given, we use SImode, >>>> if -fno-strict-volatile-bitfields is given, we give GCC the >>>> freedom to choose the access mode, maybe QImode if that is >>>> faster. >>>> >>>> In the _very_ difficult process to find an solution >>>> that seems to be acceptable to all maintainers, we came to >>>> the solution, that we need to adhere to the C++ memory >>>> model by default. And we may not change the default >>>> setting of -fstruct-volatile-bitfields at the same time! >>>> >>>> As a future extension we discussed the possibility >>>> to add a new option -fstrict-volatile-bitfields=aapcs >>>> that explicitly allows us to break the C++ memory model. >>>> >>>> But I did not yet try to implement this, as I feel that >>>> would certainly not be accepted as we are in Phase3 now. >>>> >>>> As another future extension there was the discussion >>>> about the -Wportable-volatility warning, which I see now >>>> as a warning that analyzes the structure layout and >>>> warns about any structures that are not "well-formed", >>>> in the sense, that a bit-field fails to define all >>>> bits of the container. >>>> >>>> Those people that do use bit-fields to access device-registers >>>> do always define all bits, and of course in the same mode. >>>> >>>> It would be good to have a warning, when some bits are missing. >>>> They currently have to use great care to check their structures >>>> manually. >>>> >>>> I had a proposal for that warning but that concentrated >>>> only on the volatile attribute, but I will have to re-write >>>> that completely so that it can be done in stor-layout.c: >>>> >>>> It should warn independent of optimization levels or actual >>>> bitfield member references, thus, be implemented entirely at >>>> the time we lay out the structure. The well-formed-ness of >>>> a bit-field makes that possible. >>>> >>>> But that will come too late for Phase3 as well. >>>> >>>> >>>> Regards >>>> Bernd. >>>> >>>> >>> >>> >> > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Still fails with strict-volatile-bitfields 2014-01-10 13:45 ` Bernd Edlinger @ 2014-01-10 14:12 ` Richard Biener 2014-01-10 14:20 ` Richard Earnshaw 2014-01-10 17:04 ` Bernd Edlinger 0 siblings, 2 replies; 13+ messages in thread From: Richard Biener @ 2014-01-10 14:12 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Richard Earnshaw, Joey Ye, gcc, Sandra Loosemore On Fri, Jan 10, 2014 at 2:30 PM, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > On, Fri, 10 Jan 2014 09:41:06, Richard Earnshaw wrote: >> >> On 10/01/14 08:49, Bernd Edlinger wrote: >>> On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote: >>>> >>>> On 09/01/14 08:26, Bernd Edlinger wrote: >>>>> Hi, >>>>> >>>>> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote: >>>>>> >>>>>> Sandra, Bernd, >>>>>> >>>>>> Can you take a look at >>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 >>>>>> >>>>>> It seems a siimple case still doesn't work as expected. Did I miss anything? >>>>>> >>>>>> Thanks, >>>>>> Joey >>>>> >>>>> Yes, >>>>> >>>>> this is a major case where the C++ memory model is >>>>> in conflict with AAPCS. >>>>> >>>> >>>> Does the compiler warn about this? And if so, is the warning on by >>>> default? I think it's essential that we don't have quiet changes in >>>> behaviour without such warnings. >>>> >>>> R. >>> >>> No. This example was working in 4.6 and broken in 4.7 and 4.8. >>> Well, 4.7 should have warned about that. >>> >>> 4.9 does simply not change that, because it would violate the C++ >>> memory model. Maybe that should go into release notes. >> >> That's no excuse for not generating a warning at compile time when the >> situation is encountered. Users of this feature are experiencing a >> quiet change of behaviour and that's unacceptable, even if the compiler >> is doing what it was intended to do; that doesn't necessarily match the >> user's expectations. There should always be a way to rewrite the C11 >> intended semantics in a way that doesn't violate the strict volatile >> bitfields semantics. >> > > Hmm... > You mean we should have a (ugly) warning enabled by default in 4.9 (at expmed.c) > if a bit-field access would be perfectly aligned and so, but is in conflict with the > C++ memory model, and -fstrict-volatile-bitfields is in effect. > Maybe only once per compilation? I'd say you want a warning for the structure declaration instead "Accesses to XYZ will not follow AACPS due to C++ memory model constraints". Richard. > >>> >>> At the same time we had all kinds of invalid code generation, >>> starting at 4.6, especially with packed structures in C and Ada(!), >>> (writing not all bits, and using unaligned accesses) >>> and that is fixed in 4.9. >>> >> >> I'm not worried about packed structures. You can't sensibly implement >> the strict volatile bitfields rules when things aren't aligned. >> >> R. >> >>> >>> Bernd. >>> >>>> >>>>> You can get the expected code, by changing the struct >>>>> like this: >>>>> >>>>> struct str { >>>>> volatile unsigned f1: 8; >>>>> unsigned dummy:24; >>>>> }; >>>>> >>>>> If it is written this way the C++ memory model allows >>>>> QImode, HImode, SImode. And -fstrict-volatile-bitfields >>>>> demands SImode, so the conflict is resolved. This dummy >>>>> member makes only a difference on the C level, and is >>>>> completely invisible in the generated code. >>>>> >>>>> If -fstrict-volatile-bitfields is now given, we use SImode, >>>>> if -fno-strict-volatile-bitfields is given, we give GCC the >>>>> freedom to choose the access mode, maybe QImode if that is >>>>> faster. >>>>> >>>>> In the _very_ difficult process to find an solution >>>>> that seems to be acceptable to all maintainers, we came to >>>>> the solution, that we need to adhere to the C++ memory >>>>> model by default. And we may not change the default >>>>> setting of -fstruct-volatile-bitfields at the same time! >>>>> >>>>> As a future extension we discussed the possibility >>>>> to add a new option -fstrict-volatile-bitfields=aapcs >>>>> that explicitly allows us to break the C++ memory model. >>>>> >>>>> But I did not yet try to implement this, as I feel that >>>>> would certainly not be accepted as we are in Phase3 now. >>>>> >>>>> As another future extension there was the discussion >>>>> about the -Wportable-volatility warning, which I see now >>>>> as a warning that analyzes the structure layout and >>>>> warns about any structures that are not "well-formed", >>>>> in the sense, that a bit-field fails to define all >>>>> bits of the container. >>>>> >>>>> Those people that do use bit-fields to access device-registers >>>>> do always define all bits, and of course in the same mode. >>>>> >>>>> It would be good to have a warning, when some bits are missing. >>>>> They currently have to use great care to check their structures >>>>> manually. >>>>> >>>>> I had a proposal for that warning but that concentrated >>>>> only on the volatile attribute, but I will have to re-write >>>>> that completely so that it can be done in stor-layout.c: >>>>> >>>>> It should warn independent of optimization levels or actual >>>>> bitfield member references, thus, be implemented entirely at >>>>> the time we lay out the structure. The well-formed-ness of >>>>> a bit-field makes that possible. >>>>> >>>>> But that will come too late for Phase3 as well. >>>>> >>>>> >>>>> Regards >>>>> Bernd. >>>>> >>>>> >>>> >>>> >>> >> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Still fails with strict-volatile-bitfields 2014-01-10 14:12 ` Richard Biener @ 2014-01-10 14:20 ` Richard Earnshaw 2014-01-10 17:04 ` Bernd Edlinger 1 sibling, 0 replies; 13+ messages in thread From: Richard Earnshaw @ 2014-01-10 14:20 UTC (permalink / raw) To: Richard Biener; +Cc: Bernd Edlinger, Joey Ye, gcc, Sandra Loosemore On 10/01/14 13:45, Richard Biener wrote: > On Fri, Jan 10, 2014 at 2:30 PM, Bernd Edlinger > <bernd.edlinger@hotmail.de> wrote: >> On, Fri, 10 Jan 2014 09:41:06, Richard Earnshaw wrote: >>> >>> On 10/01/14 08:49, Bernd Edlinger wrote: >>>> On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote: >>>>> >>>>> On 09/01/14 08:26, Bernd Edlinger wrote: >>>>>> Hi, >>>>>> >>>>>> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote: >>>>>>> >>>>>>> Sandra, Bernd, >>>>>>> >>>>>>> Can you take a look at >>>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 >>>>>>> >>>>>>> It seems a siimple case still doesn't work as expected. Did I miss anything? >>>>>>> >>>>>>> Thanks, >>>>>>> Joey >>>>>> >>>>>> Yes, >>>>>> >>>>>> this is a major case where the C++ memory model is >>>>>> in conflict with AAPCS. >>>>>> >>>>> >>>>> Does the compiler warn about this? And if so, is the warning on by >>>>> default? I think it's essential that we don't have quiet changes in >>>>> behaviour without such warnings. >>>>> >>>>> R. >>>> >>>> No. This example was working in 4.6 and broken in 4.7 and 4.8. >>>> Well, 4.7 should have warned about that. >>>> >>>> 4.9 does simply not change that, because it would violate the C++ >>>> memory model. Maybe that should go into release notes. >>> >>> That's no excuse for not generating a warning at compile time when the >>> situation is encountered. Users of this feature are experiencing a >>> quiet change of behaviour and that's unacceptable, even if the compiler >>> is doing what it was intended to do; that doesn't necessarily match the >>> user's expectations. There should always be a way to rewrite the C11 >>> intended semantics in a way that doesn't violate the strict volatile >>> bitfields semantics. >>> >> >> Hmm... >> You mean we should have a (ugly) warning enabled by default in 4.9 (at expmed.c) >> if a bit-field access would be perfectly aligned and so, but is in conflict with the >> C++ memory model, and -fstrict-volatile-bitfields is in effect. >> Maybe only once per compilation? > > I'd say you want a warning for the structure declaration instead > "Accesses to XYZ will not follow AACPS due to C++ memory model > constraints". > Well AAPCS (note ordering of letters ;-) is target-specific. But 'strict volatile bitfields' would be fine. Watch out, though, for struct a { // bitfield decls ... }; volatile struct a *p; // Need the warning here R. > Richard. > >> >>>> >>>> At the same time we had all kinds of invalid code generation, >>>> starting at 4.6, especially with packed structures in C and Ada(!), >>>> (writing not all bits, and using unaligned accesses) >>>> and that is fixed in 4.9. >>>> >>> >>> I'm not worried about packed structures. You can't sensibly implement >>> the strict volatile bitfields rules when things aren't aligned. >>> >>> R. >>> >>>> >>>> Bernd. >>>> >>>>> >>>>>> You can get the expected code, by changing the struct >>>>>> like this: >>>>>> >>>>>> struct str { >>>>>> volatile unsigned f1: 8; >>>>>> unsigned dummy:24; >>>>>> }; >>>>>> >>>>>> If it is written this way the C++ memory model allows >>>>>> QImode, HImode, SImode. And -fstrict-volatile-bitfields >>>>>> demands SImode, so the conflict is resolved. This dummy >>>>>> member makes only a difference on the C level, and is >>>>>> completely invisible in the generated code. >>>>>> >>>>>> If -fstrict-volatile-bitfields is now given, we use SImode, >>>>>> if -fno-strict-volatile-bitfields is given, we give GCC the >>>>>> freedom to choose the access mode, maybe QImode if that is >>>>>> faster. >>>>>> >>>>>> In the _very_ difficult process to find an solution >>>>>> that seems to be acceptable to all maintainers, we came to >>>>>> the solution, that we need to adhere to the C++ memory >>>>>> model by default. And we may not change the default >>>>>> setting of -fstruct-volatile-bitfields at the same time! >>>>>> >>>>>> As a future extension we discussed the possibility >>>>>> to add a new option -fstrict-volatile-bitfields=aapcs >>>>>> that explicitly allows us to break the C++ memory model. >>>>>> >>>>>> But I did not yet try to implement this, as I feel that >>>>>> would certainly not be accepted as we are in Phase3 now. >>>>>> >>>>>> As another future extension there was the discussion >>>>>> about the -Wportable-volatility warning, which I see now >>>>>> as a warning that analyzes the structure layout and >>>>>> warns about any structures that are not "well-formed", >>>>>> in the sense, that a bit-field fails to define all >>>>>> bits of the container. >>>>>> >>>>>> Those people that do use bit-fields to access device-registers >>>>>> do always define all bits, and of course in the same mode. >>>>>> >>>>>> It would be good to have a warning, when some bits are missing. >>>>>> They currently have to use great care to check their structures >>>>>> manually. >>>>>> >>>>>> I had a proposal for that warning but that concentrated >>>>>> only on the volatile attribute, but I will have to re-write >>>>>> that completely so that it can be done in stor-layout.c: >>>>>> >>>>>> It should warn independent of optimization levels or actual >>>>>> bitfield member references, thus, be implemented entirely at >>>>>> the time we lay out the structure. The well-formed-ness of >>>>>> a bit-field makes that possible. >>>>>> >>>>>> But that will come too late for Phase3 as well. >>>>>> >>>>>> >>>>>> Regards >>>>>> Bernd. >>>>>> >>>>>> >>>>> >>>>> >>>> >>> >>> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: Still fails with strict-volatile-bitfields 2014-01-10 14:12 ` Richard Biener 2014-01-10 14:20 ` Richard Earnshaw @ 2014-01-10 17:04 ` Bernd Edlinger 2014-01-13 6:40 ` Joey Ye 1 sibling, 1 reply; 13+ messages in thread From: Bernd Edlinger @ 2014-01-10 17:04 UTC (permalink / raw) To: Richard Biener; +Cc: Richard Earnshaw, Joey Ye, gcc, Sandra Loosemore On Fri, 10 Jan 2014 14:45:12, Richard Biener wrote: > > On Fri, Jan 10, 2014 at 2:30 PM, Bernd Edlinger > <bernd.edlinger@hotmail.de> wrote: >> On, Fri, 10 Jan 2014 09:41:06, Richard Earnshaw wrote: >>> >>> On 10/01/14 08:49, Bernd Edlinger wrote: >>>> On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote: >>>>> >>>>> On 09/01/14 08:26, Bernd Edlinger wrote: >>>>>> Hi, >>>>>> >>>>>> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote: >>>>>>> >>>>>>> Sandra, Bernd, >>>>>>> >>>>>>> Can you take a look at >>>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 >>>>>>> >>>>>>> It seems a siimple case still doesn't work as expected. Did I miss anything? >>>>>>> >>>>>>> Thanks, >>>>>>> Joey >>>>>> >>>>>> Yes, >>>>>> >>>>>> this is a major case where the C++ memory model is >>>>>> in conflict with AAPCS. >>>>>> >>>>> >>>>> Does the compiler warn about this? And if so, is the warning on by >>>>> default? I think it's essential that we don't have quiet changes in >>>>> behaviour without such warnings. >>>>> >>>>> R. >>>> >>>> No. This example was working in 4.6 and broken in 4.7 and 4.8. >>>> Well, 4.7 should have warned about that. >>>> >>>> 4.9 does simply not change that, because it would violate the C++ >>>> memory model. Maybe that should go into release notes. >>> >>> That's no excuse for not generating a warning at compile time when the >>> situation is encountered. Users of this feature are experiencing a >>> quiet change of behaviour and that's unacceptable, even if the compiler >>> is doing what it was intended to do; that doesn't necessarily match the >>> user's expectations. There should always be a way to rewrite the C11 >>> intended semantics in a way that doesn't violate the strict volatile >>> bitfields semantics. >>> >> >> Hmm... >> You mean we should have a (ugly) warning enabled by default in 4.9 (at expmed.c) >> if a bit-field access would be perfectly aligned and so, but is in conflict with the >> C++ memory model, and -fstrict-volatile-bitfields is in effect. >> Maybe only once per compilation? > > I'd say you want a warning for the structure declaration instead > "Accesses to XYZ will not follow AACPS due to C++ memory model > constraints". > > Richard. > Yes, that would be the way how we wanted to implement the -Wportable-volatility warning, except that it would be on by default if -fstrict-volatile-bitfields is in effect. And it would not only warn if the member is declared volatile, because the structure can be declared volatile later. Except that I did not even start to implement it that way, that would be quite late for 4.9 now? Bernd. >> >>>> >>>> At the same time we had all kinds of invalid code generation, >>>> starting at 4.6, especially with packed structures in C and Ada(!), >>>> (writing not all bits, and using unaligned accesses) >>>> and that is fixed in 4.9. >>>> >>> >>> I'm not worried about packed structures. You can't sensibly implement >>> the strict volatile bitfields rules when things aren't aligned. >>> >>> R. >>> >>>> >>>> Bernd. >>>> >>>>> >>>>>> You can get the expected code, by changing the struct >>>>>> like this: >>>>>> >>>>>> struct str { >>>>>> volatile unsigned f1: 8; >>>>>> unsigned dummy:24; >>>>>> }; >>>>>> >>>>>> If it is written this way the C++ memory model allows >>>>>> QImode, HImode, SImode. And -fstrict-volatile-bitfields >>>>>> demands SImode, so the conflict is resolved. This dummy >>>>>> member makes only a difference on the C level, and is >>>>>> completely invisible in the generated code. >>>>>> >>>>>> If -fstrict-volatile-bitfields is now given, we use SImode, >>>>>> if -fno-strict-volatile-bitfields is given, we give GCC the >>>>>> freedom to choose the access mode, maybe QImode if that is >>>>>> faster. >>>>>> >>>>>> In the _very_ difficult process to find an solution >>>>>> that seems to be acceptable to all maintainers, we came to >>>>>> the solution, that we need to adhere to the C++ memory >>>>>> model by default. And we may not change the default >>>>>> setting of -fstruct-volatile-bitfields at the same time! >>>>>> >>>>>> As a future extension we discussed the possibility >>>>>> to add a new option -fstrict-volatile-bitfields=aapcs >>>>>> that explicitly allows us to break the C++ memory model. >>>>>> >>>>>> But I did not yet try to implement this, as I feel that >>>>>> would certainly not be accepted as we are in Phase3 now. >>>>>> >>>>>> As another future extension there was the discussion >>>>>> about the -Wportable-volatility warning, which I see now >>>>>> as a warning that analyzes the structure layout and >>>>>> warns about any structures that are not "well-formed", >>>>>> in the sense, that a bit-field fails to define all >>>>>> bits of the container. >>>>>> >>>>>> Those people that do use bit-fields to access device-registers >>>>>> do always define all bits, and of course in the same mode. >>>>>> >>>>>> It would be good to have a warning, when some bits are missing. >>>>>> They currently have to use great care to check their structures >>>>>> manually. >>>>>> >>>>>> I had a proposal for that warning but that concentrated >>>>>> only on the volatile attribute, but I will have to re-write >>>>>> that completely so that it can be done in stor-layout.c: >>>>>> >>>>>> It should warn independent of optimization levels or actual >>>>>> bitfield member references, thus, be implemented entirely at >>>>>> the time we lay out the structure. The well-formed-ness of >>>>>> a bit-field makes that possible. >>>>>> >>>>>> But that will come too late for Phase3 as well. >>>>>> >>>>>> >>>>>> Regards >>>>>> Bernd. >>>>>> >>>>>> >>>>> >>>>> >>>> >>> >>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Still fails with strict-volatile-bitfields 2014-01-10 17:04 ` Bernd Edlinger @ 2014-01-13 6:40 ` Joey Ye 2014-02-03 21:08 ` Bernd Edlinger 0 siblings, 1 reply; 13+ messages in thread From: Joey Ye @ 2014-01-13 6:40 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Richard Biener, Richard Earnshaw, gcc, Sandra Loosemore Bernd, If that's the case, can you please firstly fix invoke.texi where the behavior of strict-volatile-bitfields is described? At least my interpretation of current doc doesn't explain the behavior of the case we are discussing. Also it should be a generic definition rather than target specific one. A related issue is that how would we expect users to build their original code with volatile bitfields usage? From the approach I understand they have to explicitly add -fstrict-volatile-bitfields for 4.9 (by default it is disabled now), and probably -fstrict-volatile-bitfields=aapcs (to allow C++ memory model conflict) later. Neither of them are obvious to users. How about a configure option to set default? Thanks, Joey On Fri, Jan 10, 2014 at 10:20 PM, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > On Fri, 10 Jan 2014 14:45:12, Richard Biener wrote: >> >> On Fri, Jan 10, 2014 at 2:30 PM, Bernd Edlinger >> <bernd.edlinger@hotmail.de> wrote: >>> On, Fri, 10 Jan 2014 09:41:06, Richard Earnshaw wrote: >>>> >>>> On 10/01/14 08:49, Bernd Edlinger wrote: >>>>> On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote: >>>>>> >>>>>> On 09/01/14 08:26, Bernd Edlinger wrote: >>>>>>> Hi, >>>>>>> >>>>>>> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote: >>>>>>>> >>>>>>>> Sandra, Bernd, >>>>>>>> >>>>>>>> Can you take a look at >>>>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 >>>>>>>> >>>>>>>> It seems a siimple case still doesn't work as expected. Did I miss anything? >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Joey >>>>>>> >>>>>>> Yes, >>>>>>> >>>>>>> this is a major case where the C++ memory model is >>>>>>> in conflict with AAPCS. >>>>>>> >>>>>> >>>>>> Does the compiler warn about this? And if so, is the warning on by >>>>>> default? I think it's essential that we don't have quiet changes in >>>>>> behaviour without such warnings. >>>>>> >>>>>> R. >>>>> >>>>> No. This example was working in 4.6 and broken in 4.7 and 4.8. >>>>> Well, 4.7 should have warned about that. >>>>> >>>>> 4.9 does simply not change that, because it would violate the C++ >>>>> memory model. Maybe that should go into release notes. >>>> >>>> That's no excuse for not generating a warning at compile time when the >>>> situation is encountered. Users of this feature are experiencing a >>>> quiet change of behaviour and that's unacceptable, even if the compiler >>>> is doing what it was intended to do; that doesn't necessarily match the >>>> user's expectations. There should always be a way to rewrite the C11 >>>> intended semantics in a way that doesn't violate the strict volatile >>>> bitfields semantics. >>>> >>> >>> Hmm... >>> You mean we should have a (ugly) warning enabled by default in 4.9 (at expmed.c) >>> if a bit-field access would be perfectly aligned and so, but is in conflict with the >>> C++ memory model, and -fstrict-volatile-bitfields is in effect. >>> Maybe only once per compilation? >> >> I'd say you want a warning for the structure declaration instead >> "Accesses to XYZ will not follow AACPS due to C++ memory model >> constraints". >> >> Richard. >> > > Yes, that would be the way how we wanted to implement the > -Wportable-volatility warning, except that it would be on by default > if -fstrict-volatile-bitfields is in effect. > And it would not only warn if the member is declared volatile, > because the structure can be declared volatile later. > > Except that I did not even start to implement it that way, that > would be quite late for 4.9 now? > > Bernd. >>> >>>>> >>>>> At the same time we had all kinds of invalid code generation, >>>>> starting at 4.6, especially with packed structures in C and Ada(!), >>>>> (writing not all bits, and using unaligned accesses) >>>>> and that is fixed in 4.9. >>>>> >>>> >>>> I'm not worried about packed structures. You can't sensibly implement >>>> the strict volatile bitfields rules when things aren't aligned. >>>> >>>> R. >>>> >>>>> >>>>> Bernd. >>>>> >>>>>> >>>>>>> You can get the expected code, by changing the struct >>>>>>> like this: >>>>>>> >>>>>>> struct str { >>>>>>> volatile unsigned f1: 8; >>>>>>> unsigned dummy:24; >>>>>>> }; >>>>>>> >>>>>>> If it is written this way the C++ memory model allows >>>>>>> QImode, HImode, SImode. And -fstrict-volatile-bitfields >>>>>>> demands SImode, so the conflict is resolved. This dummy >>>>>>> member makes only a difference on the C level, and is >>>>>>> completely invisible in the generated code. >>>>>>> >>>>>>> If -fstrict-volatile-bitfields is now given, we use SImode, >>>>>>> if -fno-strict-volatile-bitfields is given, we give GCC the >>>>>>> freedom to choose the access mode, maybe QImode if that is >>>>>>> faster. >>>>>>> >>>>>>> In the _very_ difficult process to find an solution >>>>>>> that seems to be acceptable to all maintainers, we came to >>>>>>> the solution, that we need to adhere to the C++ memory >>>>>>> model by default. And we may not change the default >>>>>>> setting of -fstruct-volatile-bitfields at the same time! >>>>>>> >>>>>>> As a future extension we discussed the possibility >>>>>>> to add a new option -fstrict-volatile-bitfields=aapcs >>>>>>> that explicitly allows us to break the C++ memory model. >>>>>>> >>>>>>> But I did not yet try to implement this, as I feel that >>>>>>> would certainly not be accepted as we are in Phase3 now. >>>>>>> >>>>>>> As another future extension there was the discussion >>>>>>> about the -Wportable-volatility warning, which I see now >>>>>>> as a warning that analyzes the structure layout and >>>>>>> warns about any structures that are not "well-formed", >>>>>>> in the sense, that a bit-field fails to define all >>>>>>> bits of the container. >>>>>>> >>>>>>> Those people that do use bit-fields to access device-registers >>>>>>> do always define all bits, and of course in the same mode. >>>>>>> >>>>>>> It would be good to have a warning, when some bits are missing. >>>>>>> They currently have to use great care to check their structures >>>>>>> manually. >>>>>>> >>>>>>> I had a proposal for that warning but that concentrated >>>>>>> only on the volatile attribute, but I will have to re-write >>>>>>> that completely so that it can be done in stor-layout.c: >>>>>>> >>>>>>> It should warn independent of optimization levels or actual >>>>>>> bitfield member references, thus, be implemented entirely at >>>>>>> the time we lay out the structure. The well-formed-ness of >>>>>>> a bit-field makes that possible. >>>>>>> >>>>>>> But that will come too late for Phase3 as well. >>>>>>> >>>>>>> >>>>>>> Regards >>>>>>> Bernd. >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: Still fails with strict-volatile-bitfields 2014-01-13 6:40 ` Joey Ye @ 2014-02-03 21:08 ` Bernd Edlinger 0 siblings, 0 replies; 13+ messages in thread From: Bernd Edlinger @ 2014-02-03 21:08 UTC (permalink / raw) To: Joey Ye Cc: Richard Biener, Richard Earnshaw, gcc, Sandra Loosemore, gcc-patches Hi, On Mon, 13 Jan 2014 10:26:03, Joey Ye wrote: > > Bernd, > > If that's the case, can you please firstly fix invoke.texi where the > behavior of strict-volatile-bitfields is described? At least my > interpretation of current doc doesn't explain the behavior of the case > we are discussing. Also it should be a generic definition rather than > target specific one. > Yes, if nobody objects I'd add a note like this to the documentation regarding the -fstrict-volatile-bitfields: Index: invoke.texi =================================================================== --- invoke.texi (revision 207223) +++ invoke.texi (working copy) @@ -22456,6 +22456,10 @@ case GCC falls back to generating multiple accesses rather than code that will fault or truncate the result at run time. +Note: Due to restrictions of the C/C++11 memory model, write accesses are +not allowed to touch non bit-field members. It is therefore recommended +to define all bits of the field's type as bit-field members. + The default value of this option is determined by the application binary interface for the target processor. Thanks Bernd. > A related issue is that how would we expect users to build their > original code with volatile bitfields usage? From the approach I > understand they have to explicitly add -fstrict-volatile-bitfields for > 4.9 (by default it is disabled now), and probably > -fstrict-volatile-bitfields=aapcs (to allow C++ memory model conflict) > later. Neither of them are obvious to users. How about a configure > option to set default? > > Thanks, > Joey > > On Fri, Jan 10, 2014 at 10:20 PM, Bernd Edlinger > <bernd.edlinger@hotmail.de> wrote: >> On Fri, 10 Jan 2014 14:45:12, Richard Biener wrote: >>> >>> On Fri, Jan 10, 2014 at 2:30 PM, Bernd Edlinger >>> <bernd.edlinger@hotmail.de> wrote: >>>> On, Fri, 10 Jan 2014 09:41:06, Richard Earnshaw wrote: >>>>> >>>>> On 10/01/14 08:49, Bernd Edlinger wrote: >>>>>> On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote: >>>>>>> >>>>>>> On 09/01/14 08:26, Bernd Edlinger wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote: >>>>>>>>> >>>>>>>>> Sandra, Bernd, >>>>>>>>> >>>>>>>>> Can you take a look at >>>>>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 >>>>>>>>> >>>>>>>>> It seems a siimple case still doesn't work as expected. Did I miss anything? >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Joey >>>>>>>> >>>>>>>> Yes, >>>>>>>> >>>>>>>> this is a major case where the C++ memory model is >>>>>>>> in conflict with AAPCS. >>>>>>>> >>>>>>> >>>>>>> Does the compiler warn about this? And if so, is the warning on by >>>>>>> default? I think it's essential that we don't have quiet changes in >>>>>>> behaviour without such warnings. >>>>>>> >>>>>>> R. >>>>>> >>>>>> No. This example was working in 4.6 and broken in 4.7 and 4.8. >>>>>> Well, 4.7 should have warned about that. >>>>>> >>>>>> 4.9 does simply not change that, because it would violate the C++ >>>>>> memory model. Maybe that should go into release notes. >>>>> >>>>> That's no excuse for not generating a warning at compile time when the >>>>> situation is encountered. Users of this feature are experiencing a >>>>> quiet change of behaviour and that's unacceptable, even if the compiler >>>>> is doing what it was intended to do; that doesn't necessarily match the >>>>> user's expectations. There should always be a way to rewrite the C11 >>>>> intended semantics in a way that doesn't violate the strict volatile >>>>> bitfields semantics. >>>>> >>>> >>>> Hmm... >>>> You mean we should have a (ugly) warning enabled by default in 4.9 (at expmed.c) >>>> if a bit-field access would be perfectly aligned and so, but is in conflict with the >>>> C++ memory model, and -fstrict-volatile-bitfields is in effect. >>>> Maybe only once per compilation? >>> >>> I'd say you want a warning for the structure declaration instead >>> "Accesses to XYZ will not follow AACPS due to C++ memory model >>> constraints". >>> >>> Richard. >>> >> >> Yes, that would be the way how we wanted to implement the >> -Wportable-volatility warning, except that it would be on by default >> if -fstrict-volatile-bitfields is in effect. >> And it would not only warn if the member is declared volatile, >> because the structure can be declared volatile later. >> >> Except that I did not even start to implement it that way, that >> would be quite late for 4.9 now? >> >> Bernd. >>>> >>>>>> >>>>>> At the same time we had all kinds of invalid code generation, >>>>>> starting at 4.6, especially with packed structures in C and Ada(!), >>>>>> (writing not all bits, and using unaligned accesses) >>>>>> and that is fixed in 4.9. >>>>>> >>>>> >>>>> I'm not worried about packed structures. You can't sensibly implement >>>>> the strict volatile bitfields rules when things aren't aligned. >>>>> >>>>> R. >>>>> >>>>>> >>>>>> Bernd. >>>>>> >>>>>>> >>>>>>>> You can get the expected code, by changing the struct >>>>>>>> like this: >>>>>>>> >>>>>>>> struct str { >>>>>>>> volatile unsigned f1: 8; >>>>>>>> unsigned dummy:24; >>>>>>>> }; >>>>>>>> >>>>>>>> If it is written this way the C++ memory model allows >>>>>>>> QImode, HImode, SImode. And -fstrict-volatile-bitfields >>>>>>>> demands SImode, so the conflict is resolved. This dummy >>>>>>>> member makes only a difference on the C level, and is >>>>>>>> completely invisible in the generated code. >>>>>>>> >>>>>>>> If -fstrict-volatile-bitfields is now given, we use SImode, >>>>>>>> if -fno-strict-volatile-bitfields is given, we give GCC the >>>>>>>> freedom to choose the access mode, maybe QImode if that is >>>>>>>> faster. >>>>>>>> >>>>>>>> In the _very_ difficult process to find an solution >>>>>>>> that seems to be acceptable to all maintainers, we came to >>>>>>>> the solution, that we need to adhere to the C++ memory >>>>>>>> model by default. And we may not change the default >>>>>>>> setting of -fstruct-volatile-bitfields at the same time! >>>>>>>> >>>>>>>> As a future extension we discussed the possibility >>>>>>>> to add a new option -fstrict-volatile-bitfields=aapcs >>>>>>>> that explicitly allows us to break the C++ memory model. >>>>>>>> >>>>>>>> But I did not yet try to implement this, as I feel that >>>>>>>> would certainly not be accepted as we are in Phase3 now. >>>>>>>> >>>>>>>> As another future extension there was the discussion >>>>>>>> about the -Wportable-volatility warning, which I see now >>>>>>>> as a warning that analyzes the structure layout and >>>>>>>> warns about any structures that are not "well-formed", >>>>>>>> in the sense, that a bit-field fails to define all >>>>>>>> bits of the container. >>>>>>>> >>>>>>>> Those people that do use bit-fields to access device-registers >>>>>>>> do always define all bits, and of course in the same mode. >>>>>>>> >>>>>>>> It would be good to have a warning, when some bits are missing. >>>>>>>> They currently have to use great care to check their structures >>>>>>>> manually. >>>>>>>> >>>>>>>> I had a proposal for that warning but that concentrated >>>>>>>> only on the volatile attribute, but I will have to re-write >>>>>>>> that completely so that it can be done in stor-layout.c: >>>>>>>> >>>>>>>> It should warn independent of optimization levels or actual >>>>>>>> bitfield member references, thus, be implemented entirely at >>>>>>>> the time we lay out the structure. The well-formed-ness of >>>>>>>> a bit-field makes that possible. >>>>>>>> >>>>>>>> But that will come too late for Phase3 as well. >>>>>>>> >>>>>>>> >>>>>>>> Regards >>>>>>>> Bernd. >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-02-03 21:08 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-01-09 8:26 Still fails with strict-volatile-bitfields Joey Ye 2014-01-09 10:45 ` Bernd Edlinger 2014-01-09 16:30 ` Richard Earnshaw 2014-01-10 8:56 ` Bernd Edlinger 2014-01-10 9:02 ` Eric Botcazou 2014-01-10 9:11 ` Eric Botcazou 2014-01-10 11:52 ` Richard Earnshaw 2014-01-10 13:45 ` Bernd Edlinger 2014-01-10 14:12 ` Richard Biener 2014-01-10 14:20 ` Richard Earnshaw 2014-01-10 17:04 ` Bernd Edlinger 2014-01-13 6:40 ` Joey Ye 2014-02-03 21:08 ` Bernd Edlinger
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).