RE: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
> >> have an option for true AAPCS compliance, which will >> be allowed to break the C++11 memory model and > >> And an option that addresses your requirements, >> which will _not_ break the C++11 memory model > > So the problem isn't that what *I* need conflicts with C++11, it's > that what AAPCS needs conflicts? Yes, there are two written specifications which are in conflict AAPCS and C++11. We cannot follow both at the same time. But from this discussion I've learned, that your target's requirements can easily co-exist with the C++ memory model. Because if you only use well-formed bit-fields, the C++ memory model just allows everything, and we can choose what to do. Regards Bernd.
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
> have an option for true AAPCS compliance, which will > be allowed to break the C++11 memory model and > And an option that addresses your requirements, > which will _not_ break the C++11 memory model So the problem isn't that what *I* need conflicts with C++11, it's that what AAPCS needs conflicts?
RE: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
Bernd Edlinger wrote:
>Hi,
>
>>> What I would suggest is to have a -fgnu-strict-volatile-bit-fields
>>
>> Why a new option? The -fstrict-volatile-bitfields option is already
>> GCC-specific, and I think it can do what you want anyway.
>
>As I understand Richard's comment, he proposes to
>have an option for true AAPCS compliance, which will
>be allowed to break the C++11 memory model and
>which will _not_ be the default on any target.
>Name it -fstrict-volatile-bitfields.
>
>And an option that addresses your requirements,
>which will _not_ break the C++11 memory model
>and which will be the default on some targets,
>dependent on the respective ABI requirements.
>Name it -fgnu-strict-volatile-bit-fields.
Yes. You could also make it -fstrict-volatile-bitfields={off,gnu,aacps} if you
think that's better.
Richard.
>
>Bernd.
RE: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
Hi, >> What I would suggest is to have a -fgnu-strict-volatile-bit-fields > > Why a new option? The -fstrict-volatile-bitfields option is already > GCC-specific, and I think it can do what you want anyway. As I understand Richard's comment, he proposes to have an option for true AAPCS compliance, which will be allowed to break the C++11 memory model and which will _not_ be the default on any target. Name it -fstrict-volatile-bitfields. And an option that addresses your requirements, which will _not_ break the C++11 memory model and which will be the default on some targets, dependent on the respective ABI requirements. Name it -fgnu-strict-volatile-bit-fields. Bernd.
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
> What I would suggest is to have a -fgnu-strict-volatile-bit-fields Why a new option? The -fstrict-volatile-bitfields option is already GCC-specific, and I think it can do what you want anyway.
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
> We use the container mode where possible.
> It is always possible for well-formed bit-fields.
This is the only part I really need.
> If necessary the user has to add anonymous bit field members, or
> convert normal members to bit-fields.
IIRC I added code to support normal members as well, the "bitfields"
part of the option name is not entirely accurate.
> But when the bit field is conflict with the either the hardware's alignment
> requirements or with the C++11 memory model, we follow the latter.
Fine with me.
> 2. we allow unaligned packed data, but we may use multiple accesses
> for a single read/write op. Also in this case: no data store races
> outside the member.
We should note that volatile != atomic in these cases. Perhaps a
warning would be in order?
> Example:
>
> struct {
> volatile int a : 24;
> volatile char b;
> };
>
> This is not well-formed, and -fstrict-volatile-bitfields will have
> no effect on it.
I agree this isn't well-formed, but -fs-v-b could still make a best
effort since none of the fields *individually* span a natural
boundary.
> The user has to re-write this structure to:
>
> struct {
> volatile int a : 24;
> volatile char b : 8;
> };
More accurate would be these:
struct {
volatile int a : 24;
volatile int b : 8;/* overlap, should be the same type */
};
struct {
volatile int a : 24;
volatile int : 0;
volatile char b; /* no overlap, make the padding explicit */
};
RE: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
Hi,
On Thu, 17 Oct 2013 16:41:13, DJ Delorie wrote:
> I'm starting from an MCU that doesn't work right if GCC doesn't do
> what the user tells GCC to do. I added -fstrict-volatile-bitfields to
> tell gcc that it needs to be more strict than the standard allows for
> bitfield access, because without that flag, there's no way to force
> gcc to use a specific access width on bitfields. When I added that
> flag, some ARM folks chose to enable it in their target, because they
> felt they needed it. If different ARM folks feel otherwise, that's a
> target problem.
>
> If the user tells gcc that a particular 32-bit memory location should
> be treated as both a char and a long, then gcc has been given
> inconsistent information. The spec says it can do what it wants to
> access that memory, and it does.
>
> If the user tells gcc that a particular 16-bit memory location
> consists of 16-bits worth of "unsigned short", and the user has told
> gcc that it needs to be strict about accessing that field in the type
> specified, and gcc uses a 32-bit access anyway, gcc is wrong.
>
> I will agree with you 100% that gcc can do whatever the spec allows if
> the user does NOT specify -fstrict-volatile-bitfields, but the flag is
> there to tell gcc that the user needs stricter control than the spec
> demands. If the user uses that flag, *and* gives gcc information that
> is inconsistent with the use of that flag, then the user is wrong.
>
> I note that you assume GNU/Linux is involved, perhaps that's part of
> the problem. Maybe the Linux kernel needs gcc to ignore its bitfield
> types, but other ARM firmware may have other requirements. If you and
> the other ARM maintainers want to argue over whether
> -fstrict-volatile-bitfields is enabled by default for the ARM target,
> go for it. Just leave my targets alone.
>
>> where those semantics contradict the semantics of ISO C.
>
> Where in the spec does it say that a compiler MUST access an "unsigned
> short" bitfield with a 32-bit access? I've seen places where it says
> the compiler MAY or MIGHT do it, but not MUST.
Well, I'm starting to think that we can never follow the ARM AAPCS by the
letter.
But maybe we could implement -fstrict-volatile-bitfields in this way:
We use the container mode where possible.
It is always possible for well-formed bit-fields.
If necessary the user has to add anonymous bit field members, or
convert normal members to bit-fields.
But when the bit field is conflict with the either the hardware's alignment
requirements or with the C++11 memory model, we follow the latter.
This means that, even for volatile data:
1. we never write anything outside the BIT_FIELD_REPRESENTATIVE.
2. we allow unaligned packed data, but we may use multiple accesses for a
single read/write op. Also in this case: no data store races outside the
member.
Example:
struct {
volatile int a : 24;
volatile char b;
};
This is not well-formed, and -fstrict-volatile-bitfields will have no effect on
it.
The user has to re-write this structure to:
struct {
volatile int a : 24;
volatile char b : 8;
};
Maybe a warning for "not well-formed" bit-fields could be helpful...
What do you think?
Bernd.
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
On Wed, Oct 9, 2013 at 3:09 AM, Bernd Edlinger
wrote:
> Hi,
>
> On Mon, 30 Sep 2013 16:18:30, DJ Delorie wrote:
>>
>> As per my previous comments on this patch, I will not approve the
>> changes to the m32c backend, as they will cause real bugs in real
>> hardware, and violate the hardware's ABI. The user may use
>> -fno-strict-volatile-bitfields if they do not desire this behavior and
>> understand the consequences.
>>
>> I am not a maintainer for the rx and h8300 ports, but they are in the
>> same situation.
>>
>> To reiterate my core position: if the user defines a proper "volatile
>> int" bitfield, and the compiler does anything other than an int-sized
>> access, the compiler is WRONG. Any optimization that changes volatile
>> accesses to something other than what the user specified is a bug that
>> needs to be fixed before this option can be non-default.
>
> hmm, I just tried to use the latest 4.9 trunk to compile the example from
> the AAPCS document:
>
> struct s
> {
> volatile int a:8;
> volatile char b:2;
> };
>
> struct s ss;
>
> int
> main ()
> {
> ss.a=1;
> ss.b=1;
> return 0;
> }
>
> and the resulting code is completely against the written AAPCS specification:
>
> main:
> @ Function supports interworking.
> @ args = 0, pretend = 0, frame = 0
> @ frame_needed = 0, uses_anonymous_args = 0
> @ link register save eliminated.
> ldr r3, .L2
> ldrhr2, [r3]
> bic r2, r2, #254
> orr r2, r2, #1
> strhr2, [r3]@ movhi
> ldrhr2, [r3]
> bic r2, r2, #512
> orr r2, r2, #256
> strhr2, [r3]@ movhi
> mov r0, #0
> bx lr
>
> two half-word accesses, to my total surprise!
>
> As it looks like, the -fstrict-volatile-bitfields are already totally broken,
> apparently in favour of the C++ memory model, at least at the write-side.
Note that the C++ memory model restricts the _maximum_ memory region
we may touch - it does not constrain the minimum as AAPCS does.
What I would suggest is to have a -fgnu-strict-volatile-bit-fields
(or two levels of it) enabled by default on AAPCS targets which will
follow the AAPCS if it doesn't violate the maximum memory region
constraints of the C++ memory model.
I never claimed that the C++ memory model is good enough for AAPCS
but there was consensus that the default on AAPCS should not violate
the C++ memory model by default.
-fgnu-strict-volatile-bit-fields should be fully implementable in
get_best_mode if you pass down the desired AAPCS mode.
Richard.
> These are aligned accesses, not the packed structures, that was the
> only case where it used to work once.
>
> This must be fixed. I do not understand why we cannot agree, that
> at least the bug-fix part of Sandra's patch needs to be applied.
>
> Regards
> Bernd.
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
I'm starting from an MCU that doesn't work right if GCC doesn't do what the user tells GCC to do. I added -fstrict-volatile-bitfields to tell gcc that it needs to be more strict than the standard allows for bitfield access, because without that flag, there's no way to force gcc to use a specific access width on bitfields. When I added that flag, some ARM folks chose to enable it in their target, because they felt they needed it. If different ARM folks feel otherwise, that's a target problem. If the user tells gcc that a particular 32-bit memory location should be treated as both a char and a long, then gcc has been given inconsistent information. The spec says it can do what it wants to access that memory, and it does. If the user tells gcc that a particular 16-bit memory location consists of 16-bits worth of "unsigned short", and the user has told gcc that it needs to be strict about accessing that field in the type specified, and gcc uses a 32-bit access anyway, gcc is wrong. I will agree with you 100% that gcc can do whatever the spec allows if the user does NOT specify -fstrict-volatile-bitfields, but the flag is there to tell gcc that the user needs stricter control than the spec demands. If the user uses that flag, *and* gives gcc information that is inconsistent with the use of that flag, then the user is wrong. I note that you assume GNU/Linux is involved, perhaps that's part of the problem. Maybe the Linux kernel needs gcc to ignore its bitfield types, but other ARM firmware may have other requirements. If you and the other ARM maintainers want to argue over whether -fstrict-volatile-bitfields is enabled by default for the ARM target, go for it. Just leave my targets alone. > where those semantics contradict the semantics of ISO C. Where in the spec does it say that a compiler MUST access an "unsigned short" bitfield with a 32-bit access? I've seen places where it says the compiler MAY or MIGHT do it, but not MUST.
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
On Thu, 17 Oct 2013, DJ Delorie wrote: > > At least on ARM, you can e.g. have a non-bit-field "char" that occupies > > part of the same 4-byte unit as an "int" bit-field. > > Ok, "when the user doesn't give the compiler conflicting requirements." > (which is why I contemplated making those conflicts errors at first) I don't consider them conflicting. My default starting point is that the user has a GNU C program, using target-independent semantics of GNU C, and expects that program to be compiled and executed according to those semantics. (In this case, it can be an ISO C program; no GNU extensions are required.) Those semantics include the memory model. The fact that the user happens to be compiling for an ARM processor shouldn't change those semantics. If, say, a GNU/Linux distribution contains code expecting the memory model, we shouldn't make the distributor use special options for building that code on ARM; building for a new architecture should, as far as possible, just work (and new architecture porters are hardly likely to know if one of thousands of packages might have such a requirement). You seem to be starting from a user having a program that instead of expecting the normal target-independent semantics expects some other semantics specified by an ABI document, where those semantics contradict the semantics of ISO C. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
> At least on ARM, you can e.g. have a non-bit-field "char" that occupies > part of the same 4-byte unit as an "int" bit-field. Ok, "when the user doesn't give the compiler conflicting requirements." (which is why I contemplated making those conflicts errors at first) I'm a big fan of blaming the user when they mix things together and expect the compiler to read their mind.
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
On Thu, 17 Oct 2013, DJ Delorie wrote: > > It is as Sandra said, at least on ARM -fstrict-volatile-bitfields > > does not function at all. And the C++11 memory model wins all the time. > > Are we talking about N2429? I read through the changes and it didn't > preclude honoring the user's types. At least on ARM, you can e.g. have a non-bit-field "char" that occupies part of the same 4-byte unit as an "int" bit-field. And the C11/C++11 memory model prohibits stores to that bit-field from doing read/modify/write on the whole 4-byte unit, as that's a store data race. If the user wants to allow that data race, they can rewrite their code by changing the "char" into a bit-field. I believe that in all cases where strict-volatile-bitfields ABIs are incompatible with the default --param allow-store-data-races=0, the structure can be similarly rewritten by the user so the adjacent fields become bit-fields, if they want the strict-volatile-bitfields requirements for their code to be compatible with the memory model. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
> where in the C standard did you read the requirement that every bit
> field must be complete? (This is a serious question).
The spec doesn't say each field must be complete, but neither does it
say that the structure must be as big as the type used. If you
specify "int foo:1" then the compile is allowed to make the struct
smaller than "int".
> extern struct
> {
> volatile unsigned int b : 1;
> } bf3;
>
> on my compiler this structure occupies 4 bytes.
> and it is aligned at 4 bytes.
> That is OK for me and AAPCS.
On my target, the structure occupies 1 byte. I suspect gcc is
rounding up to WORD_MODE, which is 4 bytes for you and 1 for me. If
so, you're relying on a coincidence, not a standard.
> But the access "bf3.b=1" is SI mode with Sandra's patch (part 1, which just
> obeys the AAPCS and does nothing else)
> and QI mode without this patch, which is just a BUG.
> I am quite surprised how your target manages to avoid it?
It doesn't, but I wouldn't expect it to, because IMHO that test is
invalid - you have not given a precise enough test to expect
consistent results.
> It is as Sandra said, at least on ARM -fstrict-volatile-bitfields
> does not function at all. And the C++11 memory model wins all the time.
Are we talking about N2429? I read through the changes and it didn't
preclude honoring the user's types.
> > Looking through the tests, most of them combine "packed" with
> > mismatched types. IMHO, those tests are invalid.
>
> I dont think so. They are simply packed. And volatile just says that
> the value may be changed by a different thread. It has a great
> impact on loop optimizations.
Nothing you've said so far precludes honoring the user's types when
the user gives you a consistent structure. Consider:
typedef struct { unsigned char b0:1, b1:1, b2:1, b3:1, b4:1, b5:1, b6:1, b7:1;
} Bits;
extern volatile struct {
Bits reg1; /* read status without resetting them */
Bits reg2; /* read status and atomically reset them */
} interrupt_handler;
Without -fstrict-volatile-bitfields, gcc is allowed to use a 16-bit
access to read reg1, but this causes an unexpected read to volatile
reg2, which may have unintended consequences. The spec doesn't say
the compiler *must* read reg2, it just *doesn't* say that it *can't*.
The -fstrict-volatile-bitfields flag tells the compiler that it
*can't*. IMHO this doesn't violate the spec, it just limits what the
compiler can do within the spec.
If ARM wants fast word-sized volatile bitfields, use "int" and
structure your bitfields so that no "int" field overlaps a natural
"int" boundary.
When I added the option, I considered making it an error() to define a
strict volatile bitfield that spanned the allowed boundary of the type
specified, but I figured that would be too much.
> > I've not objected to fixing -fstrict-volatile-bitfields, or making the
> > -fno-strict-volatile-bitfields case match the standard. I've only
> > objected to breaking my targets by making that flag not the default.
>
> Fine. Why cant we just get this fixed?
Dunno. I'm only opposing the patch that disabled it on my targets.
RE: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
On Wed, 16 Oct 2013 22:50:20, DJ Delorie wrote:
> Not all of them can work, because they describe something that can't
> be done in hardware. For example, the first test has an incomplete
> bitfield - the fields do not completely describe an "int" so the
> structure is smaller (one byte, according to sizeof()) than the access
> type (2-8 bytes).
>
where in the C standard did you read the requirement that every bit
field must be complete? (This is a serious question).
extern struct
{
volatile unsigned int b : 1;
} bf3;
on my compiler this structure occupies 4 bytes.
and it is aligned at 4 bytes.
That is OK for me and AAPCS.
But the access "bf3.b=1" is SI mode with Sandra's patch (part 1, which just
obeys the AAPCS and does nothing else)
and QI mode without this patch, which is just a BUG.
I am quite surprised how your target manages to avoid it?
It is as Sandra said, at least on ARM -fstrict-volatile-bitfields
does not function at all. And the C++11 memory model wins all the time.
> Looking through the tests, most of them combine "packed" with
> mismatched types. IMHO, those tests are invalid.
I dont think so. They are simply packed. And volatile just says that
the value may be changed by a different thread. It has a great
impact on loop optimizations.
>>> Either way, if -fstrict-volatile-bitfields does not do what it's
>>> supposed to do, the correct action is to fix it - not to disable it on
>>> targets that rely on it for correct operation.
Agreed. That is the number one priority here.
>...
> I've not objected to fixing -fstrict-volatile-bitfields, or making the
> -fno-strict-volatile-bitfields case match the standard. I've only
> objected to breaking my targets by making that flag not the default.
Fine. Why cant we just get this fixed?
Bernd.
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
> I'm curious; do all the test cases included in > http://gcc.gnu.org/ml/gcc-patches/2013-09/msg02058.html > work for you on those targets as well (without applying the rest of the > patch)? Not all of them can work, because they describe something that can't be done in hardware. For example, the first test has an incomplete bitfield - the fields do not completely describe an "int" so the structure is smaller (one byte, according to sizeof()) than the access type (2-8 bytes). Looking through the tests, most of them combine "packed" with mismatched types. IMHO, those tests are invalid. > > Either way, if -fstrict-volatile-bitfields does not do what it's > > supposed to do, the correct action is to fix it - not to disable it on > > targets that rely on it for correct operation. > > Unfortunately, "fixing" -fstrict-volatile-bitfields to conform to the > AAPCS breaks conformance with the C11/C++11 standards, The global default is -fno-strict-volatile-bitfields. The flag was added specifically to force the compiler to always use the user-specified type when accessing bitfields, at least when it was physically possible. I don't see why there's a problem. Without the flag, you follow the standard. With the flag, you do what the user told you to do. It's up to the targets to decide which way the flag defaults if the user doesn't specify. For my targets, the default must be to do what the user told you to do, regardless of the standards, because the standards lead to hardware failure. > Meanwhile, while we're bickering about what the default should be, > ARM users (in particular) are stuck in a situation where GCC's > default behavior is to generate unaligned accesses that fault at > runtime or wrong code that silently truncates bitfields, Note that if the user defines a structure that intentionally mixes types in a hardware word, the compiler can't "do what it's told". It's still up to the user to write well-defined structures in those cases. Mixing packed, incomplete bitfields, and multiple types, (i.e. pr48784-1.c in that patch) is an example of a case where gcc can't do what the user asked, because what the user asked is nonsense. > and there is *no* command-line option or even build-time option they > can provide to make GCC generate code that is correct according to > the AAPCS. At the time I added -fstrict-volatile-bitfields, the ARM maintainers seemed to agree that the new functionality was what they needed. So, -fstrict-volatile-bitfields *is* that command line option, it's just either broken for them, or they're trying to do something not physically possible. > But, looking at it practically, I think the battle over > "correctness" was already over when the C and C++ standards > committees voted to specify this behavior in a particular way, and > that in time ABIs that specify some other behavior are going to be > either revised or considered archaic or obsolete. My problem isn't some arbitrary ABI, it's a hardware peripheral register that just can't be accessed in the wrong mode, because the peripheral won't work if you do that. Most MCU peripherals work this way. Just reading a byte or word outside the intended one could have real consequences in the hardware. > About the only way forward I can see is if the GCC steering > committee steps in with a general policy statement about what to do > when language standards conflict with a target ABI. I've not objected to fixing -fstrict-volatile-bitfields, or making the -fno-strict-volatile-bitfields case match the standard. I've only objected to breaking my targets by making that flag not the default.
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
On 10/16/2013 05:46 PM, DJ Delorie wrote: As it looks like, the -fstrict-volatile-bitfields are already totally broken, I tested your example on rl78-elf, with and without -fstrict-volatile-bitfields, and it generates correct code with it and incorrect code without it. Same for m32c-elf. Same for h8300-elf. Seems to be working OK for me. I'm curious; do all the test cases included in http://gcc.gnu.org/ml/gcc-patches/2013-09/msg02058.html work for you on those targets as well (without applying the rest of the patch)? Either way, if -fstrict-volatile-bitfields does not do what it's supposed to do, the correct action is to fix it - not to disable it on targets that rely on it for correct operation. Unfortunately, "fixing" -fstrict-volatile-bitfields to conform to the AAPCS breaks conformance with the C11/C++11 standards, and we seem to be at an impasse where global reviewers refuse to consider anything that breaks language standard conformance and target maintainers refuse to consider anything that breaks ABI conformance. Meanwhile, while we're bickering about what the default should be, ARM users (in particular) are stuck in a situation where GCC's default behavior is to generate unaligned accesses that fault at runtime or wrong code that silently truncates bitfields, and there is *no* command-line option or even build-time option they can provide to make GCC generate code that is correct according to the AAPCS. FWIW, I don't care much myself in an abstract way about whether language standard conformance or ABI conformance should rule here. But, looking at it practically, I think the battle over "correctness" was already over when the C and C++ standards committees voted to specify this behavior in a particular way, and that in time ABIs that specify some other behavior are going to be either revised or considered archaic or obsolete. Anyway, at this point I've given up on thinking either side is going to convince the other in this debate, and I haven't seen much evidence that either side would be willing to compromise, either (e.g., Bernd's patch to add warnings in cases where the behavior differs, or my earlier proposal to make the defaults depend on the selected language standard as well as the ABI, don't seem to have placated either side). About the only way forward I can see is if the GCC steering committee steps in with a general policy statement about what to do when language standards conflict with a target ABI. And, of course, the patch itself still needs review -Sandra, who is feeling pretty discouraged about how hard it is to contribute to GCC :-(
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
> As it looks like, the -fstrict-volatile-bitfields are already totally broken, I tested your example on rl78-elf, with and without -fstrict-volatile-bitfields, and it generates correct code with it and incorrect code without it. Same for m32c-elf. Same for h8300-elf. Seems to be working OK for me. Either way, if -fstrict-volatile-bitfields does not do what it's supposed to do, the correct action is to fix it - not to disable it on targets that rely on it for correct operation.
RE: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
Hi,
On Mon, 30 Sep 2013 16:18:30, DJ Delorie wrote:
>
> As per my previous comments on this patch, I will not approve the
> changes to the m32c backend, as they will cause real bugs in real
> hardware, and violate the hardware's ABI. The user may use
> -fno-strict-volatile-bitfields if they do not desire this behavior and
> understand the consequences.
>
> I am not a maintainer for the rx and h8300 ports, but they are in the
> same situation.
>
> To reiterate my core position: if the user defines a proper "volatile
> int" bitfield, and the compiler does anything other than an int-sized
> access, the compiler is WRONG. Any optimization that changes volatile
> accesses to something other than what the user specified is a bug that
> needs to be fixed before this option can be non-default.
hmm, I just tried to use the latest 4.9 trunk to compile the example from
the AAPCS document:
struct s
{
volatile int a:8;
volatile char b:2;
};
struct s ss;
int
main ()
{
ss.a=1;
ss.b=1;
return 0;
}
and the resulting code is completely against the written AAPCS specification:
main:
@ Function supports interworking.
@ args = 0, pretend = 0, frame = 0
@ frame_needed = 0, uses_anonymous_args = 0
@ link register save eliminated.
ldr r3, .L2
ldrh r2, [r3]
bic r2, r2, #254
orr r2, r2, #1
strh r2, [r3] @ movhi
ldrh r2, [r3]
bic r2, r2, #512
orr r2, r2, #256
strh r2, [r3] @ movhi
mov r0, #0
bx lr
two half-word accesses, to my total surprise!
As it looks like, the -fstrict-volatile-bitfields are already totally broken,
apparently in favour of the C++ memory model, at least at the write-side.
These are aligned accesses, not the packed structures, that was the
only case where it used to work once.
This must be fixed. I do not understand why we cannot agree, that
at least the bug-fix part of Sandra's patch needs to be applied.
Regards
Bernd.
Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 2/2
As per my previous comments on this patch, I will not approve the changes to the m32c backend, as they will cause real bugs in real hardware, and violate the hardware's ABI. The user may use -fno-strict-volatile-bitfields if they do not desire this behavior and understand the consequences. I am not a maintainer for the rx and h8300 ports, but they are in the same situation. To reiterate my core position: if the user defines a proper "volatile int" bitfield, and the compiler does anything other than an int-sized access, the compiler is WRONG. Any optimization that changes volatile accesses to something other than what the user specified is a bug that needs to be fixed before this option can be non-default.
