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.edlin...@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.edlin...@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. >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>>>
