[PATCH] D16586: Make clang AAPCS compliant w.r.t volatile bitfield accesses

[Asiri Rathnayake via cfe-commits]

rmaprath added inline comments.

================
Comment at: test/CodeGen/aapcs-bitfield.c:312-317
@@ +311,8 @@
+
+  // BE: %[[PTR3:.*]] = bitcast %struct.st5a* %[[PTR2]] to i32*
+  // BE-NEXT: %[[LD:.*]] = load volatile i32, i32* %[[PTR3]], align 4
+  // BE-NEXT: %[[CLR:.*]] = and i32 %[[LD]], -16252929
+  // BE-NEXT: %[[SET:.*]] = or i32 %[[CLR]], 1572864
+  // BE-NEXT: store volatile i32 %[[SET]], i32* %[[PTR3]], align 4
+  m->y.b = 3;
+}
----------------
rsmith wrote:
> This violates the C and C++ object models by creating a data race on `m->y.a` that was not present in the source code. A store to a bit-field cannot write to bytes that are not part of the same sequence of bit-field members. If this ABI really requires that (and supports multi-threaded systems), it is not a correct ABI for C11 nor C++11. (This leaves open the question of which standard we should follow...)
Hi Richard,

Thank you for this, I didn't know about this restriction in the C11/C++11 standards. The AAPCS is indeed at odds with the standards in this case, for a simpler example, consider:
  struct foo {
    char a;
    volatile short b : 8;
  };

  void foo(struct foo *p) {
    p->b = 0xFF;
  }
This store will cause 'a' to be written as well according to the AAPCS. The conflicting sections of the standards are:
- AAPCS: 7.1.7.4 Combining bit-field and non-bit-field members (+ 7.1.7.5 - volatile bitfield access)
- C++11: 1.7 The C++ memory model
- C11: 3.14 memory location

I will take this up with the AAPCS authors.


http://reviews.llvm.org/D16586