r198907 - [ms-abi] Handle __declspec(align) on bitfields "properly"
Warren Hunt
whunt at google.com
Thu Jan 9 17:28:05 PST 2014
Author: whunt
Date: Thu Jan 9 19:28:05 2014
New Revision: 198907
URL: http://llvm.org/viewvc/llvm-project?rev=198907&view=rev
Log:
[ms-abi] Handle __declspec(align) on bitfields "properly"
__declspec(align), when applied to bitfields affects their perferred
alignment instead of their required alignment. We don't know why.
Also, #pragma pack(n) turns packing *off* if n is greater than the
pointer size. This is now observable because of the impact of
declspec(align) on bitfields.
Modified:
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=198907&r1=198906&r2=198907&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Thu Jan 9 19:28:05 2014
@@ -2028,7 +2028,9 @@ static bool isMsLayout(const RecordDecl*
// modes).
// * There is no concept of non-virtual alignment or any distinction between
// data size and non-virtual size.
-
+// * __declspec(align) on bitfields has the effect of changing the bitfield's
+// alignment instead of its required alignment. This has implications on how
+// it interacts with pragam pack.
namespace {
struct MicrosoftRecordLayoutBuilder {
@@ -2165,6 +2167,9 @@ MicrosoftRecordLayoutBuilder::ElementInf
MicrosoftRecordLayoutBuilder::getAdjustedElementInfo(
const FieldDecl *FD) {
ElementInfo Info;
+ // Respect align attributes.
+ CharUnits FieldRequiredAlignment =
+ Context.toCharUnitsFromBits(FD->getMaxAlignment());
// Respect attributes applied to subobjects of the field.
if (const RecordType *RT =
FD->getType()->getBaseElementTypeUnsafe()->getAs<RecordType>()) {
@@ -2183,6 +2188,8 @@ MicrosoftRecordLayoutBuilder::getAdjuste
Context.getTypeInfoInChars(FD->getType());
Info.Size = FieldInfo.first;
Info.Alignment = FieldInfo.second;
+ if (FD->isBitField() && FD->getMaxAlignment() != 0)
+ Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment);
// Respect pragma pack.
if (!MaxFieldAlignment.isZero())
Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment);
@@ -2190,13 +2197,13 @@ MicrosoftRecordLayoutBuilder::getAdjuste
// Respect packed field attribute.
if (FD->hasAttr<PackedAttr>())
Info.Alignment = CharUnits::One();
- // Respect align attributes.
- CharUnits FieldRequiredAlignment =
- Context.toCharUnitsFromBits(FD->getMaxAlignment());
- // Take required alignment into account.
- Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment);
- // Capture required alignment as a side-effect.
- RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment);
+ // Take required alignment into account. __declspec(align) on bitfields
+ // impacts the alignment rather than the required alignment.
+ if (!FD->isBitField()) {
+ Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment);
+ // Capture required alignment as a side-effect.
+ RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment);
+ }
return Info;
}
@@ -2233,10 +2240,14 @@ void MicrosoftRecordLayoutBuilder::initi
MaxFieldAlignment = CharUnits::Zero();
// Honor the default struct packing maximum alignment flag.
if (unsigned DefaultMaxFieldAlignment = Context.getLangOpts().PackStruct)
- MaxFieldAlignment = CharUnits::fromQuantity(DefaultMaxFieldAlignment);
- // Honor the packing attribute.
- if (const MaxFieldAlignmentAttr *MFAA = RD->getAttr<MaxFieldAlignmentAttr>())
- MaxFieldAlignment = Context.toCharUnitsFromBits(MFAA->getAlignment());
+ MaxFieldAlignment = CharUnits::fromQuantity(DefaultMaxFieldAlignment);
+ // Honor the packing attribute. The MS-ABI ignores pragma pack if its larger
+ // than the pointer size.
+ if (const MaxFieldAlignmentAttr *MFAA = RD->getAttr<MaxFieldAlignmentAttr>()){
+ unsigned PackedAlignment = MFAA->getAlignment();
+ if (PackedAlignment <= Context.getTargetInfo().getPointerWidth(0))
+ MaxFieldAlignment = Context.toCharUnitsFromBits(PackedAlignment);
+ }
// Packed attribute forces max field alignment to be 1.
if (RD->hasAttr<PackedAttr>())
MaxFieldAlignment = CharUnits::One();
@@ -2335,18 +2346,18 @@ void MicrosoftRecordLayoutBuilder::layou
const CXXRecordDecl *BaseDecl,
const ASTRecordLayout &BaseLayout,
const ASTRecordLayout *&PreviousBaseLayout) {
- // Insert padding between two bases if the left first one is zero sized or
- // contains a zero sized subobject and the right is zero sized or one leads
- // with a zero sized base.
- if (PreviousBaseLayout && PreviousBaseLayout->hasZeroSizedSubObject() &&
- BaseLayout.leadsWithZeroSizedBase())
- Size++;
- ElementInfo Info = getAdjustedElementInfo(BaseLayout);
- CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment);
- Bases.insert(std::make_pair(BaseDecl, BaseOffset));
- Size = BaseOffset + BaseLayout.getDataSize();
- updateAlignment(Info.Alignment);
- PreviousBaseLayout = &BaseLayout;
+ // Insert padding between two bases if the left first one is zero sized or
+ // contains a zero sized subobject and the right is zero sized or one leads
+ // with a zero sized base.
+ if (PreviousBaseLayout && PreviousBaseLayout->hasZeroSizedSubObject() &&
+ BaseLayout.leadsWithZeroSizedBase())
+ Size++;
+ ElementInfo Info = getAdjustedElementInfo(BaseLayout);
+ CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment);
+ Bases.insert(std::make_pair(BaseDecl, BaseOffset));
+ Size = BaseOffset + BaseLayout.getDataSize();
+ updateAlignment(Info.Alignment);
+ PreviousBaseLayout = &BaseLayout;
VBPtrOffset = Size;
}
@@ -2598,7 +2609,7 @@ void MicrosoftRecordLayoutBuilder::layou
Size = Size.RoundUpToAlignment(VtorDispAlignment) + VtorDispSize;
// Insert the virtual base.
ElementInfo Info = getAdjustedElementInfo(BaseLayout);
- CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment);
+ CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment);
VBases.insert(std::make_pair(BaseDecl,
ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp)));
Size = BaseOffset + BaseLayout.getDataSize();
Modified: cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp?rev=198907&r1=198906&r2=198907&view=diff
==============================================================================
--- cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp (original)
+++ cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp Thu Jan 9 19:28:05 2014
@@ -183,9 +183,124 @@ struct CA2 : public CA1, public CA0 {
// CHECK-C64-NEXT: | [sizeof=17, align=1
// CHECK-C64-NEXT: | nvsize=17, nvalign=1]
+#pragma pack(16)
+struct YA {
+ __declspec(align(32)) char : 1;
+};
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct YA (empty)
+// CHECK-NEXT: 0 | char
+// CHECK-NEXT: | [sizeof=32, align=32
+// CHECK-NEXT: | nvsize=32, nvalign=32]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT: 0 | struct YA (empty)
+// CHECK-X64-NEXT: 0 | char
+// CHECK-X64-NEXT: | [sizeof=32, align=32
+// CHECK-X64-NEXT: | nvsize=32, nvalign=32]
+
+#pragma pack(1)
+struct YB {
+ char a;
+ YA b;
+};
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct YB
+// CHECK-NEXT: 0 | char a
+// CHECK-NEXT: 1 | struct YA b (empty)
+// CHECK-NEXT: 1 | char
+// CHECK: | [sizeof=33, align=1
+// CHECK-NEXT: | nvsize=33, nvalign=1]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT: 0 | struct YB
+// CHECK-X64-NEXT: 0 | char a
+// CHECK-X64-NEXT: 1 | struct YA b (empty)
+// CHECK-X64-NEXT: 1 | char
+// CHECK-X64: | [sizeof=33, align=1
+// CHECK-X64-NEXT: | nvsize=33, nvalign=1]
+
+#pragma pack(8)
+struct YC {
+ __declspec(align(32)) char : 1;
+};
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct YC (empty)
+// CHECK-NEXT: 0 | char
+// CHECK-NEXT: | [sizeof=32, align=32
+// CHECK-NEXT: | nvsize=32, nvalign=32]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT: 0 | struct YC (empty)
+// CHECK-X64-NEXT: 0 | char
+// CHECK-X64-NEXT: | [sizeof=8, align=8
+// CHECK-X64-NEXT: | nvsize=8, nvalign=8]
+
+#pragma pack(1)
+struct YD {
+ char a;
+ YC b;
+};
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct YD
+// CHECK-NEXT: 0 | char a
+// CHECK-NEXT: 1 | struct YC b (empty)
+// CHECK-NEXT: 1 | char
+// CHECK: | [sizeof=33, align=1
+// CHECK-NEXT: | nvsize=33, nvalign=1]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT: 0 | struct YD
+// CHECK-X64-NEXT: 0 | char a
+// CHECK-X64-NEXT: 1 | struct YC b (empty)
+// CHECK-X64-NEXT: 1 | char
+// CHECK-X64: | [sizeof=9, align=1
+// CHECK-X64-NEXT: | nvsize=9, nvalign=1]
+
+#pragma pack(4)
+struct YE {
+ __declspec(align(32)) char : 1;
+};
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct YE (empty)
+// CHECK-NEXT: 0 | char
+// CHECK-NEXT: | [sizeof=4, align=4
+// CHECK-NEXT: | nvsize=4, nvalign=4]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT: 0 | struct YE (empty)
+// CHECK-X64-NEXT: 0 | char
+// CHECK-X64-NEXT: | [sizeof=4, align=4
+// CHECK-X64-NEXT: | nvsize=4, nvalign=4]
+
+#pragma pack(1)
+struct YF {
+ char a;
+ YE b;
+};
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct YF
+// CHECK-NEXT: 0 | char a
+// CHECK-NEXT: 1 | struct YE b (empty)
+// CHECK-NEXT: 1 | char
+// CHECK: | [sizeof=5, align=1
+// CHECK-NEXT: | nvsize=5, nvalign=1]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT: 0 | struct YF
+// CHECK-X64-NEXT: 0 | char a
+// CHECK-X64-NEXT: 1 | struct YE b (empty)
+// CHECK-X64-NEXT: 1 | char
+// CHECK-X64: | [sizeof=5, align=1
+// CHECK-X64-NEXT: | nvsize=5, nvalign=1]
+
int a[
sizeof(X)+
sizeof(Y)+
sizeof(Z)+
sizeof(C1)+
-sizeof(CA2)];
+sizeof(CA2)+
+sizeof(YA)+
+sizeof(YB)+
+sizeof(YC)+
+sizeof(YD)+
+sizeof(YE)+
+sizeof(YF)+
+0];
More information about the cfe-commits
mailing list