r198988 - [ms-abi] Change the way alignment is tracked
Warren Hunt
whunt at google.com
Fri Jan 10 17:16:40 PST 2014
Author: whunt
Date: Fri Jan 10 19:16:40 2014
New Revision: 198988
URL: http://llvm.org/viewvc/llvm-project?rev=198988&view=rev
Log:
[ms-abi] Change the way alignment is tracked
This patch more cleanly seperates the concepts of Preferred Alignment
and Required Alignment. Most notable that changes to Required Alignment
do *not* impact preferred alignment until late in struct layout. This
is observable when using pragma pack and non-virtual bases and the use
of tail padding when laying them out.
Test cases included.
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=198988&r1=198987&r2=198988&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Fri Jan 10 19:16:40 2014
@@ -2077,10 +2077,6 @@ public:
/// __declspec(align) into account. It also updates RequiredAlignment as a
/// side effect because it is most convenient to do so here.
ElementInfo getAdjustedElementInfo(const FieldDecl *FD);
- /// \brief Updates the alignment of the record.
- void updateAlignment(CharUnits MemberAlignment) {
- Alignment = std::max(Alignment, MemberAlignment);
- }
/// \brief Places a field at an offset in CharUnits.
void placeFieldAtOffset(CharUnits FieldOffset) {
FieldOffsets.push_back(Context.toBits(FieldOffset));
@@ -2157,7 +2153,9 @@ MicrosoftRecordLayoutBuilder::getAdjuste
if (Layout.hasZeroSizedSubObject())
HasZeroSizedSubObject = true;
// Respect required alignment, this is necessary because we may have adjusted
- // the alignment in the case of pragam pack.
+ // the alignment in the case of pragam pack. Note that the required alignment
+ // doesn't actually apply to the struct alignment at this point.
+ Alignment = std::max(Alignment, Info.Alignment);
Info.Alignment = std::max(Info.Alignment, Layout.getRequiredAlignment());
Info.Size = Layout.getDataSize();
return Info;
@@ -2204,6 +2202,9 @@ MicrosoftRecordLayoutBuilder::getAdjuste
// Capture required alignment as a side-effect.
RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment);
}
+ // TODO: Add a Sema warning that MS ignores bitfield alignment in unions.
+ if (!(FD->isBitField() && IsUnion))
+ Alignment = std::max(Alignment, Info.Alignment);
return Info;
}
@@ -2356,7 +2357,6 @@ void MicrosoftRecordLayoutBuilder::layou
CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment);
Bases.insert(std::make_pair(BaseDecl, BaseOffset));
Size = BaseOffset + BaseLayout.getDataSize();
- updateAlignment(Info.Alignment);
PreviousBaseLayout = &BaseLayout;
VBPtrOffset = Size;
}
@@ -2384,7 +2384,6 @@ void MicrosoftRecordLayoutBuilder::layou
placeFieldAtOffset(FieldOffset);
Size = FieldOffset + Info.Size;
}
- updateAlignment(Info.Alignment);
}
void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) {
@@ -2412,13 +2411,11 @@ void MicrosoftRecordLayoutBuilder::layou
if (IsUnion) {
placeFieldAtOffset(CharUnits::Zero());
Size = std::max(Size, Info.Size);
- // TODO: Add a Sema warning that MS ignores bitfield alignment in unions.
} else {
// Allocate a new block of memory and place the bitfield in it.
CharUnits FieldOffset = Size.RoundUpToAlignment(Info.Alignment);
placeFieldAtOffset(FieldOffset);
Size = FieldOffset + Info.Size;
- updateAlignment(Info.Alignment);
RemainingBitsInField = Context.toBits(Info.Size) - Width;
}
}
@@ -2443,7 +2440,6 @@ MicrosoftRecordLayoutBuilder::layoutZero
CharUnits FieldOffset = Size.RoundUpToAlignment(Info.Alignment);
placeFieldAtOffset(FieldOffset);
Size = FieldOffset;
- updateAlignment(Info.Alignment);
}
}
@@ -2475,8 +2471,6 @@ void MicrosoftRecordLayoutBuilder::injec
i != e; ++i)
if (i->second >= InjectionSite)
i->second += Offset;
- // Update the object alignment.
- updateAlignment(PointerInfo.Alignment);
// The presence of a vbptr suppresses zero sized objects that are not in
// virtual bases.
HasZeroSizedSubObject = false;
@@ -2500,7 +2494,6 @@ void MicrosoftRecordLayoutBuilder::injec
for (BaseOffsetsMapTy::iterator i = Bases.begin(), e = Bases.end();
i != e; ++i)
i->second += Offset;
- updateAlignment(PointerInfo.Alignment);
}
void MicrosoftRecordLayoutBuilder::injectVPtrs(const CXXRecordDecl *RD) {
@@ -2512,6 +2505,7 @@ void MicrosoftRecordLayoutBuilder::injec
// the record.
injectVBPtr(RD);
injectVFPtr(RD);
+ Alignment = std::max(Alignment, PointerInfo.Alignment);
return;
}
// In 64-bit mode, structs with RequiredAlignment greater than 8 get special
@@ -2521,7 +2515,7 @@ void MicrosoftRecordLayoutBuilder::injec
FieldOffsets.clear();
Bases.clear();
Size = CharUnits::Zero();
- updateAlignment(PointerInfo.Alignment);
+ Alignment = std::max(Alignment, PointerInfo.Alignment);
if (HasOwnVFPtr)
Size = PointerInfo.Size;
layoutNonVirtualBases(RD);
@@ -2619,7 +2613,6 @@ void MicrosoftRecordLayoutBuilder::layou
VBases.insert(std::make_pair(BaseDecl,
ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp)));
Size = BaseOffset + BaseLayout.getDataSize();
- updateAlignment(Info.Alignment);
PreviousBaseLayout = &BaseLayout;
}
}
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=198988&r1=198987&r2=198988&view=diff
==============================================================================
--- cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp (original)
+++ cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp Fri Jan 10 19:16:40 2014
@@ -74,7 +74,7 @@ struct Y : A, B {
// CHECK-NEXT: 12 | char a
// CHECK-NEXT: 14 | int b
// CHECK-NEXT: | [sizeof=20, align=4
-// CHECK-NEXT: | nvsize=20, nvalign=4]
+// CHECK-NEXT: | nvsize=18, nvalign=4]
// CHECK-X64: *** Dumping AST Record Layout
// CHECK-X64: *** Dumping AST Record Layout
// CHECK-X64-NEXT: 0 | struct Y
@@ -85,7 +85,7 @@ struct Y : A, B {
// CHECK-X64-NEXT: 12 | char a
// CHECK-X64-NEXT: 14 | int b
// CHECK-X64-NEXT: | [sizeof=20, align=4
-// CHECK-X64-NEXT: | nvsize=20, nvalign=4]
+// CHECK-X64-NEXT: | nvsize=18, nvalign=4]
struct Z : virtual B {
char a;
@@ -291,6 +291,36 @@ struct YF {
// CHECK-X64: | [sizeof=5, align=1
// CHECK-X64-NEXT: | nvsize=5, nvalign=1]
+#pragma pack(16)
+struct __declspec(align(16)) D0 { char a; };
+#pragma pack(1)
+struct D1 : public D0 { char a; };
+#pragma pack(16)
+struct D2 : D1 { char a; };
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK: *** Dumping AST Record Layout
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct D2
+// CHECK-NEXT: 0 | struct D1 (base)
+// CHECK-NEXT: 0 | struct D0 (base)
+// CHECK-NEXT: 0 | char a
+// CHECK-NEXT: 1 | char a
+// CHECK-NEXT: 2 | char a
+// CHECK-NEXT: | [sizeof=16, align=16
+// CHECK-NEXT: | nvsize=16, nvalign=16]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT: 0 | struct D2
+// CHECK-X64-NEXT: 0 | struct D1 (base)
+// CHECK-X64-NEXT: 0 | struct D0 (base)
+// CHECK-X64-NEXT: 0 | char a
+// CHECK-X64-NEXT: 1 | char a
+// CHECK-X64-NEXT: 2 | char a
+// CHECK-X64-NEXT: | [sizeof=16, align=16
+// CHECK-X64-NEXT: | nvsize=16, nvalign=16]
+
int a[
sizeof(X)+
sizeof(Y)+
@@ -303,4 +333,6 @@ sizeof(YC)+
sizeof(YD)+
sizeof(YE)+
sizeof(YF)+
+sizeof(YF)+
+sizeof(D2)+
0];
More information about the cfe-commits
mailing list