[cfe-commits] r136991 - in /cfe/trunk: lib/AST/RecordLayoutBuilder.cpp test/CodeGen/arm-apcs-zerolength-bitfield.c
Chad Rosier
mcrosier at apple.com
Sat Aug 6 10:27:12 PDT 2011
Hi Fariborz,
Comments below.
On Aug 6, 2011, at 9:23 AM, Fariborz Jahanian wrote:
> Hi Chad,
> Please see below.
>
> On Aug 5, 2011, at 3:38 PM, Chad Rosier wrote:
>
>> Author: mcrosier
>> Date: Fri Aug 5 17:38:04 2011
>> New Revision: 136991
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=136991&view=rev
>> Log:
>> Add support for using anonymous bitfields (e.g., int : 0) to enforce alignment.
>> This fixes cases where the anonymous bitfield is followed by a bitfield member.
>> E.g.,
>> struct t4
>> {
>> char foo;
>> long : 0;
>> char bar : 1;
>> };
>>
>> rdar://9859156
>>
>>
>> Modified:
>> cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
>> cfe/trunk/test/CodeGen/arm-apcs-zerolength-bitfield.c
>>
>> Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=136991&r1=136990&r2=136991&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
>> +++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Fri Aug 5 17:38:04 2011
>> @@ -1348,12 +1348,12 @@
>> }
>> LastFD = FD;
>> }
>> - else if (Context.Target.useZeroLengthBitfieldAlignment() &&
>> - !Context.Target.useBitFieldTypeAlignment()) {
>> + else if (!Context.Target.useBitFieldTypeAlignment() &&
>> + Context.Target.useZeroLengthBitfieldAlignment()) {
>> FieldDecl *FD = (*Field);
>> - if (Context.ZeroBitfieldFollowsBitfield(FD, LastFD))
>> + if (FD->isBitField() &&
>> + FD->getBitWidth()->EvaluateAsInt(Context).getZExtValue() == 0)
>> ZeroLengthBitfield = FD;
>> - LastFD = FD;
>> }
>> LayoutField(*Field);
>> }
>> @@ -1411,8 +1411,8 @@
>> setDataSize(std::max(getDataSizeInBits(), FieldSize));
>> FieldOffset = 0;
>> } else {
>> - // The bitfield is allocated starting at the next offset aligned appropriately
>> - // for T', with length n bits.
>> + // The bitfield is allocated starting at the next offset aligned
>> + // appropriately for T', with length n bits.
>> FieldOffset = llvm::RoundUpToAlignment(getDataSizeInBits(),
>> Context.toBits(TypeAlign));
>>
>> @@ -1451,19 +1451,30 @@
>> FieldAlign = TypeSize;
>>
>> if (ZeroLengthBitfield) {
Notice this code is predicated on the ZeroLengthBitfield variable.
>> - // If a zero-length bitfield is inserted after a bitfield,
>> - // and the alignment of the zero-length bitfield is
>> - // greater than the member that follows it, `bar', `bar'
>> - // will be aligned as the type of the zero-length bitfield.
>> - if (ZeroLengthBitfield != D) {
>> - std::pair<uint64_t, unsigned> FieldInfo =
>> - Context.getTypeInfo(ZeroLengthBitfield->getType());
>> - unsigned ZeroLengthBitfieldAlignment = FieldInfo.second;
>> - // Ignore alignment of subsequent zero-length bitfields.
>> - if ((ZeroLengthBitfieldAlignment > FieldAlign) || (FieldSize == 0))
>> - FieldAlign = ZeroLengthBitfieldAlignment;
>> - if (FieldSize)
>> - ZeroLengthBitfield = 0;
>> + std::pair<uint64_t, unsigned> FieldInfo;
>> + unsigned ZeroLengthBitfieldAlignment;
>> + if (IsMsStruct) {
>
> You added this conditional. I don't recall this right now. Is this code always executed when in ms_strtuct mode, I assume?
Revisions 136858 and 136991 added support for using zero length (aka anonymous) bitfields for alignment purposes (for non-ms_struct structs). Prior to r136858, the ZeroLengthBitfield variable was only set in RecordLayoutBuilder::LayoutFields() like this:
if (IsMsStruct) {
FieldDecl *FD = (*Field);
if (Context.ZeroBitfieldFollowsBitfield(FD, LastFD))
ZeroLengthBitfield = FD;
...
All other definitions were setting ZeroLengthBitfield to NULL.
So, yes, this code was always/only executed when laying out a struct with the ms_struct attribute. In this work, I reused the ZeroLengthBitfield variable, so I needed to add the conditional you mentioned to distinguish between how zero length bitfields are handled for ms_structs vs. non-ms_structs.
Chad
>
> - Thanks, Fariborz
>
>> + // If a zero-length bitfield is inserted after a bitfield,
>> + // and the alignment of the zero-length bitfield is
>> + // greater than the member that follows it, `bar', `bar'
>> + // will be aligned as the type of the zero-length bitfield.
>> + if (ZeroLengthBitfield != D) {
>> + FieldInfo = Context.getTypeInfo(ZeroLengthBitfield->getType());
>> + ZeroLengthBitfieldAlignment = FieldInfo.second;
>> + // Ignore alignment of subsequent zero-length bitfields.
>> + if ((ZeroLengthBitfieldAlignment > FieldAlign) || (FieldSize == 0))
>> + FieldAlign = ZeroLengthBitfieldAlignment;
>> + if (FieldSize)
>> + ZeroLengthBitfield = 0;
>> + }
>> + } else {
>> + // The alignment of a zero-length bitfield affects the alignment
>> + // of the next member. The alignment is the max of the zero
>> + // length bitfield's alignment and a target specific fixed value.
>> + unsigned ZeroLengthBitfieldBoundary =
>> + Context.Target.getZeroLengthBitfieldBoundary();
>> + if (ZeroLengthBitfieldBoundary > FieldAlign)
>> + FieldAlign = ZeroLengthBitfieldBoundary;
>> }
>> }
>>
>> @@ -1476,10 +1487,11 @@
>> // was unnecessary (-Wpacked).
>> unsigned UnpackedFieldAlign = FieldAlign;
>> uint64_t UnpackedFieldOffset = FieldOffset;
>> - if (!Context.Target.useBitFieldTypeAlignment())
>> + if (!Context.Target.useBitFieldTypeAlignment() && !ZeroLengthBitfield)
>> UnpackedFieldAlign = 1;
>>
>> - if (FieldPacked || !Context.Target.useBitFieldTypeAlignment())
>> + if (FieldPacked ||
>> + (!Context.Target.useBitFieldTypeAlignment() && !ZeroLengthBitfield))
>> FieldAlign = 1;
>> FieldAlign = std::max(FieldAlign, D->getMaxAlignment());
>> UnpackedFieldAlign = std::max(UnpackedFieldAlign, D->getMaxAlignment());
>> @@ -1500,10 +1512,14 @@
>> UnpackedFieldOffset = llvm::RoundUpToAlignment(UnpackedFieldOffset,
>> UnpackedFieldAlign);
>>
>> - // Padding members don't affect overall alignment.
>> - if (!D->getIdentifier())
>> + // Padding members don't affect overall alignment, unless zero length bitfield
>> + // alignment is enabled.
>> + if (!D->getIdentifier() && !Context.Target.useZeroLengthBitfieldAlignment())
>> FieldAlign = UnpackedFieldAlign = 1;
>>
>> + if (!IsMsStruct)
>> + ZeroLengthBitfield = 0;
>> +
>> // Place this field at the current location.
>> FieldOffsets.push_back(FieldOffset);
>>
>>
>> Modified: cfe/trunk/test/CodeGen/arm-apcs-zerolength-bitfield.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm-apcs-zerolength-bitfield.c?rev=136991&r1=136990&r2=136991&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/CodeGen/arm-apcs-zerolength-bitfield.c (original)
>> +++ cfe/trunk/test/CodeGen/arm-apcs-zerolength-bitfield.c Fri Aug 5 17:38:04 2011
>> @@ -58,8 +58,8 @@
>> char bar : 1;
>> char bar2;
>> };
>> -static int arr6_offset[(offsetof(struct t6, bar2) == 1) ? 0 : -1];
>> -static int arr6_sizeof[(sizeof(struct t6) == 2) ? 0 : -1];
>> +static int arr6_offset[(offsetof(struct t6, bar2) == 5) ? 0 : -1];
>> +static int arr6_sizeof[(sizeof(struct t6) == 8) ? 0 : -1];
>>
>> struct t7
>> {
>> @@ -68,8 +68,8 @@
>> char bar1 : 1;
>> char bar2;
>> };
>> -static int arr7_offset[(offsetof(struct t7, bar2) == 1) ? 0 : -1];
>> -static int arr7_sizeof[(sizeof(struct t7) == 2) ? 0 : -1];
>> +static int arr7_offset[(offsetof(struct t7, bar2) == 5) ? 0 : -1];
>> +static int arr7_sizeof[(sizeof(struct t7) == 8) ? 0 : -1];
>>
>> struct t8
>> {
>> @@ -78,8 +78,8 @@
>> char bar1 : 1;
>> char bar2;
>> };
>> -static int arr8_offset[(offsetof(struct t8, bar2) == 1) ? 0 : -1];
>> -static int arr8_sizeof[(sizeof(struct t8) == 2) ? 0 : -1];
>> +static int arr8_offset[(offsetof(struct t8, bar2) == 5) ? 0 : -1];
>> +static int arr8_sizeof[(sizeof(struct t8) == 8) ? 0 : -1];
>>
>> struct t9
>> {
>> @@ -88,8 +88,8 @@
>> char bar1 : 1;
>> char bar2;
>> };
>> -static int arr9_offset[(offsetof(struct t9, bar2) == 1) ? 0 : -1];
>> -static int arr9_sizeof[(sizeof(struct t9) == 2) ? 0 : -1];
>> +static int arr9_offset[(offsetof(struct t9, bar2) == 5) ? 0 : -1];
>> +static int arr9_sizeof[(sizeof(struct t9) == 8) ? 0 : -1];
>>
>> struct t10
>> {
>> @@ -98,9 +98,8 @@
>> char bar1 : 1;
>> char bar2;
>> };
>> -static int arr10_offset[(offsetof(struct t10, bar2) == 1) ? 0 : -1];
>> -static int arr10_sizeof[(sizeof(struct t10) == 2) ? 0 : -1];
>> -
>> +static int arr10_offset[(offsetof(struct t10, bar2) == 5) ? 0 : -1];
>> +static int arr10_sizeof[(sizeof(struct t10) == 8) ? 0 : -1];
>>
>> struct t11
>> {
>> @@ -110,8 +109,8 @@
>> char bar1 : 1;
>> char bar2;
>> };
>> -static int arr11_offset[(offsetof(struct t11, bar2) == 1) ? 0 : -1];
>> -static int arr11_sizeof[(sizeof(struct t11) == 2) ? 0 : -1];
>> +static int arr11_offset[(offsetof(struct t11, bar2) == 5) ? 0 : -1];
>> +static int arr11_sizeof[(sizeof(struct t11) == 8) ? 0 : -1];
>>
>> struct t12
>> {
>> @@ -124,6 +123,117 @@
>> static int arr12_offset[(offsetof(struct t12, bar) == 4) ? 0 : -1];
>> static int arr12_sizeof[(sizeof(struct t12) == 8) ? 0 : -1];
>>
>> +struct t13
>> +{
>> + char foo;
>> + long : 0;
>> + char bar;
>> +};
>> +static int arr13_offset[(offsetof(struct t13, bar) == 4) ? 0 : -1];
>> +static int arr13_sizeof[(sizeof(struct t13) == 8) ? 0 : -1];
>> +
>> +struct t14
>> +{
>> + char foo1;
>> + int : 0;
>> + char foo2 : 1;
>> + short foo3 : 16;
>> + char : 0;
>> + short foo4 : 16;
>> + char bar1;
>> + int : 0;
>> + char bar2;
>> +};
>> +static int arr14_bar1_offset[(offsetof(struct t14, bar1) == 10) ? 0 : -1];
>> +static int arr14_bar2_offset[(offsetof(struct t14, bar2) == 12) ? 0 : -1];
>> +static int arr14_sizeof[(sizeof(struct t14) == 16) ? 0 : -1];
>> +
>> +struct t15
>> +{
>> + char foo;
>> + char : 0;
>> + int : 0;
>> + char bar;
>> + long : 0;
>> + char : 0;
>> +};
>> +static int arr15_offset[(offsetof(struct t15, bar) == 4) ? 0 : -1];
>> +static int arr15_sizeof[(sizeof(struct t15) == 8) ? 0 : -1];
>> +
>> +struct t16
>> +{
>> + long : 0;
>> + char bar;
>> +};
>> +static int arr16_offset[(offsetof(struct t16, bar) == 0) ? 0 : -1];
>> +static int arr16_sizeof[(sizeof(struct t16) == 4) ? 0 : -1];
>> +
>> +struct t17
>> +{
>> + char foo;
>> + long : 0;
>> + long : 0;
>> + char : 0;
>> + char bar;
>> +};
>> +static int arr17_offset[(offsetof(struct t17, bar) == 4) ? 0 : -1];
>> +static int arr17_sizeof[(sizeof(struct t17) == 8) ? 0 : -1];
>> +
>> +struct t18
>> +{
>> + long : 0;
>> + long : 0;
>> + char : 0;
>> +};
>> +static int arr18_sizeof[(sizeof(struct t18) == 0) ? 0 : -1];
>> +
>> +struct t19
>> +{
>> + char foo1;
>> + long foo2 : 1;
>> + char : 0;
>> + long foo3 : 32;
>> + char bar;
>> +};
>> +static int arr19_offset[(offsetof(struct t19, bar) == 8) ? 0 : -1];
>> +static int arr19_sizeof[(sizeof(struct t19) == 12) ? 0 : -1];
>> +
>> +struct t20
>> +{
>> + short : 0;
>> + int foo : 1;
>> + long : 0;
>> + char bar;
>> +};
>> +static int arr20_offset[(offsetof(struct t20, bar) == 4) ? 0 : -1];
>> +static int arr20_sizeof[(sizeof(struct t20) == 8) ? 0 : -1];
>> +
>> +struct t21
>> +{
>> + short : 0;
>> + int foo1 : 1;
>> + char : 0;
>> + int foo2 : 16;
>> + long : 0;
>> + char bar1;
>> + int bar2;
>> + long bar3;
>> + char foo3 : 8;
>> + char : 0;
>> + long : 0;
>> + int foo4 : 32;
>> + short foo5: 1;
>> + long bar4;
>> + short foo6: 16;
>> + short foo7: 16;
>> + short foo8: 16;
>> +};
>> +static int arr21_bar1_offset[(offsetof(struct t21, bar1) == 8) ? 0 : -1];
>> +static int arr21_bar2_offset[(offsetof(struct t21, bar2) == 12) ? 0 : -1];
>> +static int arr21_bar3_offset[(offsetof(struct t21, bar3) == 16) ? 0 : -1];
>> +static int arr21_bar4_offset[(offsetof(struct t21, bar4) == 32) ? 0 : -1];
>> +static int arr21_sizeof[(sizeof(struct t21) == 44) ? 0 : -1];
>> +
>> int main() {
>> return 0;
>> }
>>
>>
>> _______________________________________________
>> 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/20110806/4893fa9b/attachment.html>
More information about the cfe-commits
mailing list