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

[Asiri Rathnayake via cfe-commits]

rmaprath added inline comments.

================
Comment at: lib/CodeGen/CGExpr.cpp:1761
@@ +1760,3 @@
+    Ptr = Address(AdjustAAPCSBitfieldAccess(Dst, Info, false),
+                  getContext().getTypeAlignInChars(Dst.getType()));
+
----------------
rjmccall wrote:
> 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.
I'm wondering if it would be possible to do the all the adjustments (not just setting the bitfield LValue alignment) within EmitLValueForField(). It would certainly simplify the code a lot (e.g. No need to dissect the load/store GEP as I currently do in AdjustAAPCSBitfieldAccess(), I can intercept the original GEP when it is being constructed).

One problem I have is, as part of the adjustments, I have to override the CGBitFieldInfo of the bitfield LValue. But LValue holds a pointer to the CGBitFieldInfo and I'd have to allocate a new CGBitFieldInfo instance and pass it into LValue::MakeBitfield() method. Not sure if that is a good idea (who'd free it?). If you have any suggestions, please let me know.

Adjusting alignments in EmitLValueForField() and then again adjusting other bitfield access parameters in the load/store methods would be a bit messy, I imagine.

Thanks for all the feedback!


http://reviews.llvm.org/D16586