From d8fe7b2792b14a61749658d35e924c0811d395a9 Mon Sep 17 00:00:00 2001 From: Mike Stump Date: Wed, 5 Aug 2009 22:37:18 +0000 Subject: [PATCH] Calculate the primary base class better and use that when laying down the vtable. Still a work in progress. llvm-svn: 78252 --- clang/include/clang/AST/DeclCXX.h | 4 + clang/include/clang/AST/RecordLayout.h | 15 +++- clang/lib/AST/RecordLayoutBuilder.cpp | 120 +++++++++++++++++++++++-- clang/lib/AST/RecordLayoutBuilder.h | 8 ++ clang/lib/CodeGen/CGCXX.cpp | 49 +++++++--- clang/test/CodeGenCXX/virt.cpp | 21 ++++- 6 files changed, 194 insertions(+), 23 deletions(-) diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 18ff6a3b811c..19c21cba7756 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -398,6 +398,10 @@ public: virtual void Destroy(ASTContext& C); + bool isDynamicClass() const { + return Polymorphic || NumVBases!=0; + } + /// setBases - Sets the base classes of this struct or class. void setBases(ASTContext &C, CXXBaseSpecifier const * const *Bases, unsigned NumBases); diff --git a/clang/include/clang/AST/RecordLayout.h b/clang/include/clang/AST/RecordLayout.h index c657ddb869b8..bb40da7647ee 100644 --- a/clang/include/clang/AST/RecordLayout.h +++ b/clang/include/clang/AST/RecordLayout.h @@ -54,6 +54,9 @@ class ASTRecordLayout { /// which is the alignment of the object without virtual bases. uint64_t NonVirtualAlign; + /// PrimaryBase - The primary base for our vtable. + const CXXRecordDecl *PrimaryBase; + /// BaseOffsets - Contains a map from base classes to their offset. /// FIXME: Does it make sense to store offsets for virtual base classes /// here? @@ -83,8 +86,8 @@ class ASTRecordLayout { ASTRecordLayout(uint64_t size, unsigned alignment, uint64_t datasize, const uint64_t *fieldoffsets, unsigned fieldcount, uint64_t nonvirtualsize, unsigned nonvirtualalign, - const CXXRecordDecl **bases, const uint64_t *baseoffsets, - unsigned basecount) + const CXXRecordDecl *PB, const CXXRecordDecl **bases, + const uint64_t *baseoffsets, unsigned basecount) : Size(size), DataSize(datasize), FieldOffsets(0), Alignment(alignment), FieldCount(fieldcount), CXXInfo(new CXXRecordLayoutInfo) { if (FieldCount > 0) { @@ -93,6 +96,7 @@ class ASTRecordLayout { FieldOffsets[i] = fieldoffsets[i]; } + CXXInfo->PrimaryBase = PB; CXXInfo->NonVirtualSize = nonvirtualsize; CXXInfo->NonVirtualAlign = nonvirtualalign; for (unsigned i = 0; i != basecount; ++i) @@ -146,6 +150,13 @@ public: return CXXInfo->NonVirtualAlign; } + /// getPrimaryBase - Get the primary base. + const CXXRecordDecl *getPrimaryBase() const { + assert(CXXInfo && "Record layout does not have C++ specific info!"); + + return CXXInfo->PrimaryBase; + } + /// getBaseClassOffset - Get the offset, in bits, for the given base class. uint64_t getBaseClassOffset(const CXXRecordDecl *Base) const { assert(CXXInfo && "Record layout does not have C++ specific info!"); diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 901d5a5b26f3..10295f18835c 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -16,6 +16,7 @@ #include "clang/AST/Expr.h" #include "clang/AST/RecordLayout.h" #include "clang/Basic/TargetInfo.h" +#include #include using namespace clang; @@ -25,14 +26,17 @@ ASTRecordLayoutBuilder::ASTRecordLayoutBuilder(ASTContext &Ctx) IsUnion(false), NonVirtualSize(0), NonVirtualAlignment(8) {} void ASTRecordLayoutBuilder::LayoutVtable(const CXXRecordDecl *RD) { - if (RD->isPolymorphic() || RD->getNumVBases()) - { - // assert (RD->getNumBases() == 0 && "no polymorphic inheritance yet"); - int AS = 0; - UpdateAlignment(Ctx.Target.getPointerAlign(AS)); - Size += Ctx.Target.getPointerWidth(AS); - NextOffset = Size; - } + // FIXME: audit indirect virtual bases + if (!RD->isPolymorphic() && !RD->getNumVBases()) + return; + + SelectPrimaryBase(RD); + if (PrimaryBase == 0) { + int AS = 0; + UpdateAlignment(Ctx.Target.getPointerAlign(AS)); + Size += Ctx.Target.getPointerWidth(AS); + NextOffset = Size; + } } void @@ -47,6 +51,104 @@ ASTRecordLayoutBuilder::LayoutNonVirtualBases(const CXXRecordDecl *RD) { } } +// Helper routines related to the abi definition from: +// http://www.codesourcery.com/public/cxx-abi/abi.html +// +/// IsNearlyEmpty - Indicates when a class has a vtable pointer, but +/// no other data. +bool ASTRecordLayoutBuilder::IsNearlyEmpty(const CXXRecordDecl *RD) { + // FIXME: Audit the corners + if (!RD->isDynamicClass()) + return false; + const ASTRecordLayout &BaseInfo = Ctx.getASTRecordLayout(RD); + if (BaseInfo.getNonVirtualSize() == Ctx.Target.getPointerWidth(0)) + return true; + return false; +} + +void ASTRecordLayoutBuilder::SelectPrimaryForBase(const CXXRecordDecl *RD, + llvm::SmallSet &IndirectPrimary) { + for (CXXRecordDecl::base_class_const_iterator i = RD->bases_begin(), + e = RD->bases_end(); i != e; ++i) { + if (!i->isVirtual()) { + const CXXRecordDecl *Base = + cast(i->getType()->getAs()->getDecl()); + // Only bases with virtual bases participate in computing the + // indirect primary base classes. + // FIXME: audit indirect virtual bases + if (Base->getNumVBases() == 0) + return; + // FIXME: This information is recomputed a whole lot, cache it instead. + SelectPrimaryBase(Base); + IndirectPrimary.insert(PrimaryBase); + SelectPrimaryForBase(Base, IndirectPrimary); + } + } +} + +/// SelectPrimaryBase - Selects the primary base for the given class and +/// records that with setPrimaryBase. +void ASTRecordLayoutBuilder::SelectPrimaryBase(const CXXRecordDecl *RD) { + // The primary base is the first non-virtual indirect or direct base class, + // if one exists. + for (CXXRecordDecl::base_class_const_iterator i = RD->bases_begin(), + e = RD->bases_end(); i != e; ++i) { + if (!i->isVirtual()) { + const CXXRecordDecl *Base = + cast(i->getType()->getAs()->getDecl()); + if (Base->isDynamicClass()) { + setPrimaryBase(Base); + return; + } + } + } + + // Otherwise, it is the first nearly empty virtual base that is not an + // indirect primary base class, if one exists. + + // If we have no virtual bases at this point, bail out as the searching below + // is expensive. + // FIXME: audit indirect virtual bases + if (RD->getNumVBases() == 0) { + setPrimaryBase(0); + return; + } + + // First, we compute all the primary bases for all of out direct and indirect + // non-virtual bases, and record all their primary base classes. + const CXXRecordDecl *FirstPrimary = 0; + llvm::SmallSet IndirectPrimary; + for (CXXRecordDecl::base_class_const_iterator i = RD->bases_begin(), + e = RD->bases_end(); i != e; ++i) { + if (!i->isVirtual()) { + const CXXRecordDecl *Base = + cast(i->getType()->getAs()->getDecl()); + SelectPrimaryForBase(Base, IndirectPrimary); + } + } + + // Then we can search for the first nearly empty virtual base itself. + // FIXME: audit indirect virtual bases + for (CXXRecordDecl::base_class_const_iterator i = RD->vbases_begin(), + e = RD->vbases_end(); i != e; ++i) { + const CXXRecordDecl *Base = + cast(i->getType()->getAs()->getDecl()); + if (IsNearlyEmpty(Base)) { + if (FirstPrimary==0) + FirstPrimary = Base; + if (!IndirectPrimary.count(Base)) { + setPrimaryBase(Base); + return; + } + } + } + + // Otherwise if is the first nearly empty base, if one exists, otherwise + // there is no primary base class. + setPrimaryBase(FirstPrimary); + return; +} + void ASTRecordLayoutBuilder::LayoutNonVirtualBase(const CXXRecordDecl *RD) { const ASTRecordLayout &BaseInfo = Ctx.getASTRecordLayout(RD); assert(BaseInfo.getDataSize() > 0 && @@ -86,6 +188,7 @@ void ASTRecordLayoutBuilder::Layout(const RecordDecl *D) { LayoutVtable(RD); LayoutNonVirtualBases(RD); + // FIXME: audit indirect virtual bases assert (RD->getNumVBases() == 0 && "FIXME: We don't support virtual bases yet!"); // FIXME: We need to layout the virtual bases in the complete object layout. @@ -277,6 +380,7 @@ ASTRecordLayoutBuilder::ComputeLayout(ASTContext &Ctx, Builder.FieldOffsets.size(), NonVirtualSize, Builder.NonVirtualAlignment, + Builder.PrimaryBase, Builder.Bases.data(), Builder.BaseOffsets.data(), Builder.Bases.size()); diff --git a/clang/lib/AST/RecordLayoutBuilder.h b/clang/lib/AST/RecordLayoutBuilder.h index e21536077a51..a6c9c5e873db 100644 --- a/clang/lib/AST/RecordLayoutBuilder.h +++ b/clang/lib/AST/RecordLayoutBuilder.h @@ -11,6 +11,7 @@ #define LLVM_CLANG_AST_RECORDLAYOUTBUILDER_H #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/SmallSet.h" #include "llvm/Support/DataTypes.h" namespace clang { @@ -35,6 +36,8 @@ class ASTRecordLayoutBuilder { uint64_t NonVirtualSize; unsigned NonVirtualAlignment; + const CXXRecordDecl *PrimaryBase; + llvm::SmallVector Bases; llvm::SmallVector BaseOffsets; @@ -48,6 +51,11 @@ class ASTRecordLayoutBuilder { void LayoutFields(const RecordDecl *D); void LayoutField(const FieldDecl *D); + void SelectPrimaryBase(const CXXRecordDecl *RD); + void SelectPrimaryForBase(const CXXRecordDecl *RD, + llvm::SmallSet &IndirectPrimary); + void setPrimaryBase(const CXXRecordDecl *PB) { PrimaryBase = PB; } + bool IsNearlyEmpty(const CXXRecordDecl *RD); void LayoutVtable(const CXXRecordDecl *RD); void LayoutNonVirtualBases(const CXXRecordDecl *RD); void LayoutNonVirtualBase(const CXXRecordDecl *RD); diff --git a/clang/lib/CodeGen/CGCXX.cpp b/clang/lib/CodeGen/CGCXX.cpp index 97937ea89a13..375c9ac40e40 100644 --- a/clang/lib/CodeGen/CGCXX.cpp +++ b/clang/lib/CodeGen/CGCXX.cpp @@ -540,9 +540,12 @@ llvm::Value *CodeGenFunction::GenerateVtable(const CXXRecordDecl *RD) { llvm::Type *Ptr8Ty; Ptr8Ty = llvm::PointerType::get(llvm::Type::Int8Ty, 0); m = llvm::Constant::getNullValue(Ptr8Ty); - int64_t offset = 0; - methods.push_back(m); offset += LLVMPointerWidth; - methods.push_back(GenerateRtti(RD)); offset += LLVMPointerWidth; + int64_t Offset = 0; + methods.push_back(m); Offset += LLVMPointerWidth; + methods.push_back(GenerateRtti(RD)); Offset += LLVMPointerWidth; + + const ASTRecordLayout &Layout = getContext().getASTRecordLayout(RD); + const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase(); for (CXXRecordDecl::base_class_const_iterator i = RD->bases_begin(), e = RD->bases_end(); i != e; ++i) { @@ -550,6 +553,16 @@ llvm::Value *CodeGenFunction::GenerateVtable(const CXXRecordDecl *RD) { continue; const CXXRecordDecl *Base = cast(i->getType()->getAs()->getDecl()); + if (PrimaryBase != Base) { + const ASTRecordLayout &Layout = getContext().getASTRecordLayout(RD); + int64_t BaseOffset = -(Layout.getBaseClassOffset(Base) / 8); + m = llvm::ConstantInt::get(llvm::Type::Int64Ty, BaseOffset); + m = llvm::ConstantExpr::getIntToPtr(m, Ptr8Ty); + methods.push_back(m); + // FIXME: GenerateRtti for Base in RD. + m = llvm::Constant::getNullValue(Ptr8Ty); + methods.push_back(m); + } for (meth_iter mi = Base->method_begin(), me = Base->method_end(); mi != me; ++mi) { if (mi->isVirtual()) { @@ -558,16 +571,28 @@ llvm::Value *CodeGenFunction::GenerateVtable(const CXXRecordDecl *RD) { methods.push_back(m); } } + if (PrimaryBase == Base) { + for (meth_iter mi = RD->method_begin(), me = RD->method_end(); mi != me; + ++mi) { + if (mi->isVirtual()) { + m = CGM.GetAddrOfFunction(GlobalDecl(*mi)); + m = llvm::ConstantExpr::getBitCast(m, Ptr8Ty); + methods.push_back(m); + } + } + } + } + if (PrimaryBase == 0) { + for (meth_iter mi = RD->method_begin(), me = RD->method_end(); mi != me; + ++mi) { + if (mi->isVirtual()) { + m = CGM.GetAddrOfFunction(GlobalDecl(*mi)); + m = llvm::ConstantExpr::getBitCast(m, Ptr8Ty); + methods.push_back(m); + } + } } - for (meth_iter mi = RD->method_begin(), me = RD->method_end(); mi != me; - ++mi) { - if (mi->isVirtual()) { - m = CGM.GetAddrOfFunction(GlobalDecl(*mi)); - m = llvm::ConstantExpr::getBitCast(m, Ptr8Ty); - methods.push_back(m); - } - } llvm::Constant *C; llvm::ArrayType *type = llvm::ArrayType::get(Ptr8Ty, methods.size()); C = llvm::ConstantArray::get(type, methods); @@ -577,7 +602,7 @@ llvm::Value *CodeGenFunction::GenerateVtable(const CXXRecordDecl *RD) { // FIXME: finish layout for virtual bases vtable = Builder.CreateGEP(vtable, llvm::ConstantInt::get(llvm::Type::Int64Ty, - offset/8)); + Offset/8)); return vtable; } diff --git a/clang/test/CodeGenCXX/virt.cpp b/clang/test/CodeGenCXX/virt.cpp index e77abaa6f565..7a43026f825d 100644 --- a/clang/test/CodeGenCXX/virt.cpp +++ b/clang/test/CodeGenCXX/virt.cpp @@ -8,15 +8,26 @@ struct B { virtual void bar1(); virtual void bar2(); }; +void B::bar1() { } +void B::bar2() { } + +struct C { + virtual void bee1(); + virtual void bee2(); +}; +void C::bee1() { } +void C::bee2() { } static_assert (sizeof (B) == (sizeof(void *)), "vtable pointer layout"); -class A : public B { +class A : public B, public C { public: virtual void foo1(); virtual void foo2(); A() { } } *a; +void A::foo1() { } +void A::foo2() { } int main() { A a; @@ -29,6 +40,10 @@ int main() { // CHECK-LP64: .quad __ZN1B4bar2Ev // CHECK-LP64: .quad __ZN1A4foo1Ev // CHECK-LP64: .quad __ZN1A4foo2Ev +// CHECK-LP64: .quad 18446744073709551608 +// CHECK-LP64: .space 8 +// CHECK-LP64: .quad __ZN1C4bee1Ev +// CHECK-LP64: .quad __ZN1C4bee2Ev // CHECK-LP32: __ZTV1A: // CHECK-LP32: .space 4 @@ -37,3 +52,7 @@ int main() { // CHECK-LP32: .long __ZN1B4bar2Ev // CHECK-LP32: .long __ZN1A4foo1Ev // CHECK-LP32: .long __ZN1A4foo2Ev +// CHECK-LP32: .long 4294967292 +// CHECK-LP32: .space 4 +// CHECK-LP32: .long __ZN1C4bee1Ev +// CHECK-LP32: .long __ZN1C4bee2Ev