On 05/09/11 10:24, Aldy Hernandez wrote:
> Seeing that the current C++ draft has been approved, I'd like to submit
> this for mainline, and get the proper review everyone's being quietly
> avoiding :).
>
> To refresh everyone's memory, here is the problem:
>
> struct
> {
> unsigned int a : 4;
> unsigned char b;
> unsigned int c: 6;
> } var;
>
>
> void seta(){
> var.a = 12;
> }
>
>
> In the new C++ standard, stores into <a> cannot touch <b>, so we can't
> store with anything wider (e.g. a 32 bit store) that will touch <b>.
> This problem can be seen on strictly aligned targets such as ARM, where
> we store the above sequence with a 32-bit store. Or on x86-64 with <a>
> being volatile (PR48124).
>
> This patch fixes both problems, but only for the C++ memory model. This
> is NOT a generic fix PR48124, only a fix when using "--param
> allow-store-data-races=0". I will gladly change the parameter name, if
> another is preferred.
>
> The gist of this patch is in max_field_size(), where we calculate the
> maximum number of bits we can store into. In doing this calculation I
> assume we can store into the padding without causing any races. So,
> padding between fields and at the end of the structure are included.
Well, the kernel guys would like to be able to be able to preserve the
padding bits too. It's a long long sad story that I won't repeat...
And I don't think we should further complicate this stuff with the
desire to not clobber padding bits :-) Though be aware the request
might come one day....
>
> Tested on x86-64 both with and without "--param
> allow-store-data-races=0", and visually inspecting the assembly on
> arm-linux and ia64-linux.
Any way to add a test to the testsuite?
General approach seems OK; I didn't dive deeply into the implementation.
I'll leave that for rth & jason :-)
jeff
