On 8/14/10 10:39 PM, Charles Davis wrote:
> On 8/14/10 8:03 PM, Sean Hunt wrote:
>> On 08/14/2010 07:34 PM, Charles Davis wrote:
>>> On 8/14/10 1:55 PM, John McCall wrote:
>>>> This seems fine, though yeah, I guess I'd prefer that you checked it in
>>>> with
>>>> the MS layout code.
>>> Here's a version that includes some MS-specific code. It handles the
>>> case where a class has both a virtual method and a virtual base (in such
>>> cases, VC would give it two virtual pointers).
>>
>> Does every member function need to be virtual? A lot of those seem like
>> overriding them is possibly a bad idea,
> Most C++ ABIs might be compatible with C, but how do we know that
> someone won't come up with a novel ABI that has completely different
> conceptions about everything--including class layout?
>> and in any case it's not very
>> useful to make them virtual if we aren't overriding them.
> I concede to you on that. As much as I don't want to end up breaking
> backwards compatibility later on, it might be worth removing most of the
> 'virtual's from the patch to minimize the (tiny) performance impact of
> virtual calls. The Clang C++ API is supposed to be "unstable", after all...
And here's a patch where only the methods that are overridden in
MSRecordLayoutBuilder are 'virtual'.
Chip
Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp (revision 111084)
+++ lib/AST/RecordLayoutBuilder.cpp (working copy)
@@ -73,22 +73,11 @@
/// member subobject that is empty.
void ComputeEmptySubobjectSizes();
- bool CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- uint64_t Offset) const;
-
void AddSubobjectAtOffset(const CXXRecordDecl *RD, uint64_t Offset);
- bool CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- uint64_t Offset);
void UpdateEmptyBaseSubobjects(const BaseSubobjectInfo *Info,
uint64_t Offset, bool PlacingEmptyBase);
- bool CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
- const CXXRecordDecl *Class,
- uint64_t Offset) const;
- bool CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
- uint64_t Offset) const;
-
void UpdateEmptyFieldSubobjects(const CXXRecordDecl *RD,
const CXXRecordDecl *Class,
uint64_t Offset);
@@ -100,6 +89,19 @@
return Offset <= MaxEmptyClassOffset;
}
+protected:
+ bool CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+ uint64_t Offset) const;
+
+ bool CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
+ uint64_t Offset);
+
+ bool CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
+ const CXXRecordDecl *Class,
+ uint64_t Offset) const;
+ bool CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
+ uint64_t Offset) const;
+
public:
/// This holds the size of the largest empty subobject (either a base
/// or a member). Will be zero if the record being built doesn't contain
@@ -513,6 +515,7 @@
}
class RecordLayoutBuilder {
+protected:
// FIXME: Remove this and make the appropriate fields public.
friend class clang::ASTContext;
@@ -623,12 +626,14 @@
void SelectPrimaryVBase(const CXXRecordDecl *RD);
+ virtual uint64_t GetVirtualPointersSize(const CXXRecordDecl *RD) const;
+
/// IdentifyPrimaryBases - Identify all virtual base classes, direct or
/// indirect, that are primary base classes for some other direct or indirect
/// base class.
void IdentifyPrimaryBases(const CXXRecordDecl *RD);
- bool IsNearlyEmpty(const CXXRecordDecl *RD) const;
+ virtual bool IsNearlyEmpty(const CXXRecordDecl *RD) const;
/// LayoutNonVirtualBases - Determines the primary base class (if any) and
/// lays it out. Will then proceed to lay out all non-virtual base clasess.
@@ -638,7 +643,7 @@
void LayoutNonVirtualBase(const BaseSubobjectInfo *Base);
void AddPrimaryVirtualBaseOffsets(const BaseSubobjectInfo *Info,
- uint64_t Offset);
+ uint64_t Offset);
/// LayoutVirtualBases - Lays out all the virtual bases.
void LayoutVirtualBases(const CXXRecordDecl *RD,
@@ -734,6 +739,11 @@
}
}
+uint64_t
+RecordLayoutBuilder::GetVirtualPointersSize(const CXXRecordDecl *RD) const {
+ return Context.Target.getPointerWidth(0);
+}
+
/// DeterminePrimaryBase - Determine the primary base of the given class.
void RecordLayoutBuilder::DeterminePrimaryBase(const CXXRecordDecl *RD) {
// If the class isn't dynamic, it won't have a primary base.
@@ -794,7 +804,7 @@
assert(DataSize == 0 && "Vtable pointer must be at offset zero!");
// Update the size.
- Size += Context.Target.getPointerWidth(0);
+ Size += GetVirtualPointersSize(RD);
DataSize = Size;
// Update the alignment.
@@ -1453,6 +1463,38 @@
return 0;
}
+// This class implements layout specific to the Microsoft ABI.
+class MSRecordLayoutBuilder: public RecordLayoutBuilder {
+ friend class ASTContext;
+
+ MSRecordLayoutBuilder(ASTContext& Ctx, EmptySubobjectMap *EmptySubobjects):
+ RecordLayoutBuilder(Ctx, EmptySubobjects) {}
+
+ virtual bool IsNearlyEmpty(const CXXRecordDecl *RD) const;
+ virtual uint64_t GetVirtualPointersSize(const CXXRecordDecl *RD) const;
+};
+
+bool MSRecordLayoutBuilder::IsNearlyEmpty(const CXXRecordDecl *RD) const {
+ // FIXME: Audit the corners
+ if (!RD->isDynamicClass())
+ return false;
+ const ASTRecordLayout &BaseInfo = Context.getASTRecordLayout(RD);
+ // In the Microsoft ABI, classes can have one or two vtable pointers.
+ if (BaseInfo.getNonVirtualSize() == Context.Target.getPointerWidth(0) ||
+ BaseInfo.getNonVirtualSize() == Context.Target.getPointerWidth(0) * 2)
+ return true;
+ return false;
+}
+
+uint64_t
+MSRecordLayoutBuilder::GetVirtualPointersSize(const CXXRecordDecl *RD) const {
+ // We should reserve space for two pointers if the class has both
+ // virtual functions and virtual bases.
+ if (RD->isPolymorphic() && RD->getNumVBases() > 0)
+ return 2 * Context.Target.getPointerWidth(0);
+ return Context.Target.getPointerWidth(0);
+}
+
/// getASTRecordLayout - Get or compute information about the layout of the
/// specified record (struct/union/class), which indicates its size and field
/// position information.
@@ -1471,8 +1513,13 @@
if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D)) {
EmptySubobjectMap EmptySubobjects(*this, RD);
- RecordLayoutBuilder Builder(*this, &EmptySubobjects);
- Builder.Layout(RD);
+ // When compiling for Microsoft, use the special MS builder.
+ RecordLayoutBuilder *Builder;
+ if (Target.getCXXABI() == "microsoft")
+ Builder = new MSRecordLayoutBuilder(*this, &EmptySubobjects);
+ else
+ Builder = new RecordLayoutBuilder(*this, &EmptySubobjects);
+ Builder->Layout(RD);
// FIXME: This is not always correct. See the part about bitfields at
// http://www.codesourcery.com/public/cxx-abi/abi.html#POD for more info.
@@ -1481,20 +1528,21 @@
// FIXME: This should be done in FinalizeLayout.
uint64_t DataSize =
- IsPODForThePurposeOfLayout ? Builder.Size : Builder.DataSize;
+ IsPODForThePurposeOfLayout ? Builder->Size : Builder->DataSize;
uint64_t NonVirtualSize =
- IsPODForThePurposeOfLayout ? DataSize : Builder.NonVirtualSize;
+ IsPODForThePurposeOfLayout ? DataSize : Builder->NonVirtualSize;
NewEntry =
- new (*this) ASTRecordLayout(*this, Builder.Size, Builder.Alignment,
- DataSize, Builder.FieldOffsets.data(),
- Builder.FieldOffsets.size(),
+ new (*this) ASTRecordLayout(*this, Builder->Size, Builder->Alignment,
+ DataSize, Builder->FieldOffsets.data(),
+ Builder->FieldOffsets.size(),
NonVirtualSize,
- Builder.NonVirtualAlignment,
+ Builder->NonVirtualAlignment,
EmptySubobjects.SizeOfLargestEmptySubobject,
- Builder.PrimaryBase,
- Builder.PrimaryBaseIsVirtual,
- Builder.Bases, Builder.VBases);
+ Builder->PrimaryBase,
+ Builder->PrimaryBaseIsVirtual,
+ Builder->Bases, Builder->VBases);
+ delete Builder;
} else {
RecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/0);
Builder.Layout(D);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
