The attached patch uses the logic you suggest below, this does indeed also fix the problem I was seeing originally. Does this look OK?
(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 From: Richard Smith <[email protected]> To: Ulrich Weigand/Germany/IBM@IBMDE, Chandler Carruth <[email protected]> Cc: llvm cfe <[email protected]> Date: 20.04.2015 23:47 Subject: Re: [PATCH][RFC] Respect alignment of nested bitfields Sent by: [email protected] On Mon, Mar 30, 2015 at 11:31 AM, Ulrich Weigand <[email protected] > 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 [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
clang-align-bitfield
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
