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

[John McCall via cfe-commits]

rjmccall added a comment.

Well, that's certainly an interesting ABI rule.

A few high-level notes:

1. AAPCS requires the bit-field to be loaded on a store, even if the store fills the entire container; that doesn't seem to be implemented in your patch.

2. Especially because of #1, let's not do this unless the l-value is actually volatile.  The ABI rule here is arguably actively wrong for non-volatile cases, e.g. struct { volatile char c; short s : 8; }.

3. Instead of using string comparisons all over the place, please make this a flag on the CG TargetInfo or something.


================
Comment at: lib/CodeGen/CGExpr.cpp:1761
@@ +1760,3 @@
+    Ptr = Address(AdjustAAPCSBitfieldAccess(Dst, Info, false),
+                  getContext().getTypeAlignInChars(Dst.getType()));
+
----------------
This alignment computation is wrong; you need to be basing this on the alignment of the base.  It would be easier to do that in the formation of the LValue in the first place in EmitLValueForField, and then you won't need to modify as many of these uses.


http://reviews.llvm.org/D16586