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