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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
