| Message ID | 51C2157B.9090707@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 19 Jun 2013, Sandra Loosemore wrote: > On 06/17/2013 06:02 PM, Sandra Loosemore wrote: > > > > I had another thought: perhaps -fstrict-volatile-bitfields could remain > > the default on targets where it currently is, but it can be overridden > > by an appropriate -std= option. Perhaps also GCC could give an error if > > -fstrict-volatile-bitfields is given explicitly with an incompatible > > -std= option. > > Like this. This patch is intended to be applied on top of the other 5 pieces > in this series, although in theory it's independent of them. OK to commit, > and does this resolve the objection to part 3? I don't think it's right to depend on the standard version like this. The existing semantics for GNU C and C++ follow the memory model for all standard versions, and that's the sort of thing that shouldn't depend on the target architecture. In the absence of explicit -fstrict-volatile-bitfields, semantics conflicting with the memory model should only be enabled by one of the --param options to allow data races, and not by some default option relating to something in a target ABI that's incompatible with the normal language semantics.
On 06/19/2013 05:10 PM, Joseph S. Myers wrote: > > I don't think it's right to depend on the standard version like this. The > existing semantics for GNU C and C++ follow the memory model for all > standard versions, and that's the sort of thing that shouldn't depend on > the target architecture. In the absence of explicit > -fstrict-volatile-bitfields, semantics conflicting with the memory model > should only be enabled by one of the --param options to allow data races, > and not by some default option relating to something in a target ABI > that's incompatible with the normal language semantics. Hmmmm. Well, I'm willing to hack up a patch to remove -fstrict-volatile-bitfields from the defaults for all backends, if it is the consensus of the GCC community to do that and it unblocks consideration of the other wrong-code bug fix patches in the series. I'm concerned, though, that we should consider the perspective of GCC users on the affected targets as well as that of a standards committee member. E.g., suppose I am an ARM user with some code manipulating memory-mapped I/O registers that was originally developed with an older version of GCC, or with a different compiler. Maybe it is not even my own code, but something I got from a chip vendor SDK. People working with such target-specific, low-level code are far more likely to be familiar with and conforming to ARM's published guidelines for volatile bit-field access than obscure details of a language standard that is not even being selected as the dialect for compiling the code. I think there's a good argument here that by retroactively applying the C11/C++11 memory model to older standard versions, GCC has simply broken access to memory-mapped registers on ARM. After all, the AAPCS predates these newer standards and older versions of GCC at least made an effort to implement the behavior AAPCS required, and if the pr23623 testcase had been added at the time that issue was originally resolved back in 2006, the regression would have been caught immediately when the bitfield range patches were committed. I hope that by the time GCC switches to C11 and C++11 as its default dialects, ARM will have revised its ABI or clarified how this conflict should be resolved. :-) Anyway, what do other people think? -Sandra
On Wed, Jun 19, 2013 at 7:55 PM, Sandra Loosemore <sandra@codesourcery.com> wrote: > On 06/19/2013 05:10 PM, Joseph S. Myers wrote: >> >> >> I don't think it's right to depend on the standard version like this. The >> existing semantics for GNU C and C++ follow the memory model for all >> standard versions, and that's the sort of thing that shouldn't depend on >> the target architecture. In the absence of explicit >> -fstrict-volatile-bitfields, semantics conflicting with the memory model >> should only be enabled by one of the --param options to allow data races, >> and not by some default option relating to something in a target ABI >> that's incompatible with the normal language semantics. > > > Hmmmm. Well, I'm willing to hack up a patch to remove > -fstrict-volatile-bitfields from the defaults for all backends, if it is the > consensus of the GCC community to do that and it unblocks consideration of > the other wrong-code bug fix patches in the series. > > I'm concerned, though, that we should consider the perspective of GCC users > on the affected targets as well as that of a standards committee member. > E.g., suppose I am an ARM user with some code manipulating memory-mapped I/O > registers that was originally developed with an older version of GCC, or > with a different compiler. Maybe it is not even my own code, but something > I got from a chip vendor SDK. People working with such target-specific, > low-level code are far more likely to be familiar with and conforming to > ARM's published guidelines for volatile bit-field access than obscure > details of a language standard that is not even being selected as the > dialect for compiling the code. I think there's a good argument here that > by retroactively applying the C11/C++11 memory model to older standard > versions, GCC has simply broken access to memory-mapped registers on ARM. > After all, the AAPCS predates these newer standards and older versions of > GCC at least made an effort to implement the behavior AAPCS required, and if > the pr23623 testcase had been added at the time that issue was originally > resolved back in 2006, the regression would have been caught immediately > when the bitfield range patches were committed. > > I hope that by the time GCC switches to C11 and C++11 as its default > dialects, ARM will have revised its ABI or clarified how this conflict > should be resolved. :-) > > Anyway, what do other people think? I rather ARM think about the issues. This effects even AARCH64 where most programs are going to be following the C11/C++11 memory model due to the market that is being aimed. I think it is a good idea to have ARM resolves the issues before even breaking C11/C++11 memory model. I know for Cavium's AARCH64 GCC I am going to turn off -fstrict-volatile-bitfields for AARCH64 even though it violates the ABI since it violates the C/c++ standard first. The C/c++ standard in my mind is what takes precedence over the ABI. Thanks, Andrew > > -Sandra >
Index: gcc/c-family/c-opts.c =================================================================== --- gcc/c-family/c-opts.c (revision 199963) +++ gcc/c-family/c-opts.c (working copy) @@ -813,6 +813,18 @@ c_common_post_options (const char **pfil C_COMMON_OVERRIDE_OPTIONS; #endif + /* C11 and C++11 specify a memory model that is incompatible with + -fstrict-volatile-bitfields. Warn if that option is given explicitly + and prevent backends from defaulting to turning it on. */ + if (flag_isoc11 || cxx_dialect >= cxx11) + { + if (flag_strict_volatile_bitfields > 0) + warning (0, "-fstrict-volatile-bitfields conflicts with the " + "C11 and C++11 memory model"); + else + flag_strict_volatile_bitfields = 0; + } + /* Excess precision other than "fast" requires front-end support. */ if (c_dialect_cxx ()) Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 199963) +++ gcc/doc/invoke.texi (working copy) @@ -20899,4 +20899,12 @@ AAPCS, @option{-fstrict-volatile-bitfields} is the default. +Note that @option{-fstrict-volatile-bitfields} is incompatible with +the bit-field access behavior required by the ISO C11 and C++11 +standards. GCC warns if @option{-fstrict-volatile-bitfields} is given +explicitly with an incompatible @option{-std=} option. On targets +that otherwise default to @option{-fstrict-volatile-bitfields}, +providing an incompatible @option{-std=} option implicitly disables +@option{-fstrict-volatile-bitfields}. + @item -fsync-libcalls @opindex fsync-libcalls