[PATCH][RFC] Respect alignment of nested bitfields

[Richard Smith]

On Mon, Mar 30, 2015 at 11:31 AM, Ulrich Weigand <Ulrich.Weigand at de.ibm.com>
wrote:

> Hello,
>
> tools/clang/test/CodeGen/packed-nest-unpacked.c contains this test:
>
> struct XBitfield {
>   unsigned b1 : 10;
>   unsigned b2 : 12;
>   unsigned b3 : 10;
> };
> struct YBitfield {
>   char x;
>   struct XBitfield y;
> } __attribute((packed));
> struct YBitfield gbitfield;
>
> unsigned test7() {
>   // CHECK: @test7
>   // CHECK: load i32, i32* getelementptr inbounds (%struct.YBitfield,
> %struct.YBitfield* @gbitfield, i32 0, i32 1, i32 0), align 4
>   return gbitfield.y.b2;
> }
>
> The "align 4" is actually wrong.  Accessing all of "gbitfield.y" as a
> single
> i32 is of course possible, but that still doesn't make it 4-byte aligned as
> it remains packed at offset 1 in the surrounding gbitfield object.
>
> This alignment was changed by commit r169489, which also introduced changes
> to bitfield access code in CGExpr.cpp.  Code before that change used to
> take
> into account *both* the alignment of the field to be accessed within the
> current struct, *and* the alignment of that outer struct itself; this logic
> was removed by the above commit.
>
> This can cause incorrect code to be generated (I've seen an example on
> SystemZ
> where code would crash due to unaligned access).
>
> The patch below adds back logic to also consider alignment of the outer
> struct
> in EmitLoadOfBitfieldLValue and EmitStoreThroughBitfieldLValue.  It also
> extends the test case to cover the case of storing into the nested
> bitfield.
>

+  int64_t Align = Info.StorageAlignment;
+  if (!LV.getAlignment().isZero())
+    Align = std::min(Align, LV.getAlignment().getQuantity());

This seems pessimistic. If LV is known to be more aligned than we'd expect
for the bitfield, it seems that we should use its alignment in that case
too. That is:

  int64_t Align = LV.getAlignment().isZero() ? Info.StorageAlignment :
LV.getAlignment().getQuantity();


> (See attached file: clang-align-bitfield)
>
>
> Mit freundlichen Gruessen / Best Regards
>
> Ulrich Weigand
>
> --
>   Dr. Ulrich Weigand | Phone: +49-7031/16-3727
>   STSM, GNU/Linux compilers and toolchain
>   IBM Deutschland Research & Development GmbH
>   Vorsitzende des Aufsichtsrats: Martina Koederitz | Geschäftsführung: Dirk
> Wittkopp
>   Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht
> Stuttgart, HRB 243294
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150420/a4a77403/attachment.html>