| Message ID | 4D92103E.90100@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Mar 29, 2011 at 7:00 PM, Aldy Hernandez <aldyh@redhat.com> wrote: > [Language lawyers, please correct me if I have mis-interpreted the upcoming > standard in any way.] > > In the C++ memory model, contiguous bitfields comprise a single memory > location, so it's fair game to bit twiddle them when setting them. For > example: > > struct { > unsigned int a : 4; > unsigned int b : 4; > unsigned int c : 4; > }; > > In the above example, you can touch <b> and <c> while setting <a>. No race > there. > > However, non contiguous bitfields are a different story: > > struct { > unsigned int a : 4; > char b; > unsigned int c : 6; > }; > > Here we have 3 distinct memory locations, so you can't touch <b> or <c> > while setting <a>. No bit twiddling allowed. > > Similarly for bitfields separated by a zero-length bitfield: > > struct { > unsigned int a : 4; > int : 0; > unsigned int c : 6; > }; > > In the above example, <a> and <c> are distinct memory locations. > > Also, a structure/union boundary will also separate previously contiguous > bit sequences: > > struct { > unsigned int a : 4; > struct { unsigned int b : 4 } BBB; > unsigned int c : 4; > }; > > Here we have 3 distinct memory locations, so again, we can't clobber <b> or > <c> while setting <a>. > > The patch below adds a non-contiguous bit test (bitfields-2.C) which passes > on x86, but upon assembly inspection, fails on PPC64, s390, and Alpha. > These 3 architectures bit-twiddle their way out of the problem. The memory model is not implementable on strict-alignment targets that do not have a byte store operation. But we previously said that ;) Also consider global vars char a; char b; accessing them on strict-align targets may access adjacent globals (that's a problem anyway, also with alias analysis). Richard. > There is also a similar test already in the testsuite (bitfields.C) which is > similar except one field is a volatile. This test fails on x86-64 as well > and is the subject of PR48128 which Jakub is currently tackling. > > As soon as Jakub finishes with PR48128, I will be working on getting these > bitfield tests working in a C++ memory model fashion. > > Committing to branch. >
Index: gcc.dg/memmodel/subfields.c =================================================================== --- gcc.dg/memmodel/subfields.c (revision 170937) +++ gcc.dg/memmodel/subfields.c (working copy) @@ -20,6 +20,7 @@ struct test_struct { not affect any of the other fields in the structure. An improper implementation may load an entire word, change the 8 bits for field 'a' and write the entire word back out. */ +__attribute__((noinline)) void set_a(char x) { var.a = x; Index: g++.dg/memmodel/bitfields-2.C =================================================================== --- g++.dg/memmodel/bitfields-2.C (revision 0) +++ g++.dg/memmodel/bitfields-2.C (revision 0) @@ -0,0 +1,71 @@ +/* { dg-do link } */ +/* { dg-options "-O2 --param allow-load-data-races=0 --param allow-store-data-races=0" } */ +/* { dg-final { memmodel-gdb-test } } */ + +/* Test that setting <var.a> does not touch either <var.b> or <var.c>. + In the C++ memory model, non contiguous bitfields ("a" and "c" + here) should be considered as distinct memory locations, so we + can't use bit twiddling to set either one. */ + +#include <stdio.h> +#include "memmodel.h" + +#define CONSTA 12 + +static int global; +struct S +{ + unsigned int a : 4; + unsigned char b; + unsigned int c : 6; +} var; + +__attribute__((noinline)) +void set_a() +{ + var.a = CONSTA; +} + +void memmodel_other_threads() +{ + ++global; + var.b = global; + var.c = global; +} + +int memmodel_step_verify() +{ + int ret = 0; + if (var.b != global) + { + printf ("FAIL: Unexpected value: var.b is %d, should be %d\n", + var.b, global); + ret = 1; + } + if (var.c != global) + { + printf ("FAIL: Unexpected value: var.c is %d, should be %d\n", + var.c, global); + ret = 1; + } + return ret; +} + +int memmodel_final_verify() +{ + int ret = memmodel_step_verify(); + if (var.a != CONSTA) + { + printf ("FAIL: Unexpected value: var.a is %d, should be %d\n", + var.a, CONSTA); + ret = 1; + } + return ret; +} + +int main() +{ + set_a(); + memmodel_done(); + return 0; +} Index: g++.dg/memmodel/bitfields.C =================================================================== --- g++.dg/memmodel/bitfields.C (revision 171248) +++ g++.dg/memmodel/bitfields.C (working copy) @@ -23,6 +23,7 @@ struct S unsigned int c : 6; } var; +__attribute__((noinline)) void set_a() { var.a = CONSTA;
[Language lawyers, please correct me if I have mis-interpreted the upcoming standard in any way.] In the C++ memory model, contiguous bitfields comprise a single memory location, so it's fair game to bit twiddle them when setting them. For example: struct { unsigned int a : 4; unsigned int b : 4; unsigned int c : 4; }; In the above example, you can touch <b> and <c> while setting <a>. No race there. However, non contiguous bitfields are a different story: struct { unsigned int a : 4; char b; unsigned int c : 6; }; Here we have 3 distinct memory locations, so you can't touch <b> or <c> while setting <a>. No bit twiddling allowed. Similarly for bitfields separated by a zero-length bitfield: struct { unsigned int a : 4; int : 0; unsigned int c : 6; }; In the above example, <a> and <c> are distinct memory locations. Also, a structure/union boundary will also separate previously contiguous bit sequences: struct { unsigned int a : 4; struct { unsigned int b : 4 } BBB; unsigned int c : 4; }; Here we have 3 distinct memory locations, so again, we can't clobber <b> or <c> while setting <a>. The patch below adds a non-contiguous bit test (bitfields-2.C) which passes on x86, but upon assembly inspection, fails on PPC64, s390, and Alpha. These 3 architectures bit-twiddle their way out of the problem. There is also a similar test already in the testsuite (bitfields.C) which is similar except one field is a volatile. This test fails on x86-64 as well and is the subject of PR48128 which Jakub is currently tackling. As soon as Jakub finishes with PR48128, I will be working on getting these bitfield tests working in a C++ memory model fashion. Committing to branch. * gcc.dg/memmodel/subfields.c (set_a): Set noinline attribute. * g++.dg/memmodel/bitfields.C (set_a): Same. * g++.dg/memmodel/bitfields-2.C: New.