From e67dc3072522570207a16724ea459f81a58e4621 Mon Sep 17 00:00:00 2001 From: Anders Carlsson Date: Sun, 14 Feb 2010 00:16:19 +0000 Subject: [PATCH] Improve support for non-virtual 'this' pointer adjustments. With this, it should be possible to use the new vtable layout code for all class hierarchies that do not involve virtual bases. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@96137 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGVtable.cpp | 68 +++++++++++++++++++------------ test/CodeGenCXX/vtable-layout.cpp | 43 ++++++++++++++++++- 2 files changed, 83 insertions(+), 28 deletions(-) diff --git a/lib/CodeGen/CGVtable.cpp b/lib/CodeGen/CGVtable.cpp index 15b9a924df..3651d1c214 100644 --- a/lib/CodeGen/CGVtable.cpp +++ b/lib/CodeGen/CGVtable.cpp @@ -136,9 +136,9 @@ private: SubobjectOffsetsMapTy &Offsets); /// ComputeThisAdjustmentBaseOffset - Compute the base offset for adjusting - /// the 'this' pointer from BaseRD to DerivedRD. - BaseOffset ComputeThisAdjustmentBaseOffset(const CXXRecordDecl *BaseRD, - const CXXRecordDecl *DerivedRD); + /// the 'this' pointer from the base subobject to the derived subobject. + BaseOffset ComputeThisAdjustmentBaseOffset(BaseSubobject BaseSubobject, + BaseSubobject Derived); static void MergeSubobjectOffsets(const SubobjectOffsetsMapTy &NewOffsets, SubobjectOffsetsMapTy &Offsets); @@ -229,22 +229,11 @@ void FinalOverriders::AddOverriders(BaseSubobject Base, } } -static FinalOverriders::BaseOffset -ComputeBaseOffset(ASTContext &Context, const CXXRecordDecl *BaseRD, - const CXXRecordDecl *DerivedRD) { - CXXBasePaths Paths(/*FindAmbiguities=*/false, - /*RecordPaths=*/true, /*DetectVirtual=*/false); - - if (!const_cast(DerivedRD)-> - isDerivedFrom(const_cast(BaseRD), Paths)) { - assert(false && "Class must be derived from the passed in base class!"); - return FinalOverriders::BaseOffset(); - } - +static FinalOverriders::BaseOffset +ComputeBaseOffset(ASTContext &Context, const CXXRecordDecl *DerivedRD, + const CXXBasePath &Path) { int64_t NonVirtualOffset = 0; - const CXXBasePath &Path = Paths.front(); - unsigned NonVirtualStart = 0; const CXXRecordDecl *VirtualBase = 0; @@ -280,6 +269,22 @@ ComputeBaseOffset(ASTContext &Context, const CXXRecordDecl *BaseRD, // CharUnits. return FinalOverriders::BaseOffset(DerivedRD, VirtualBase, NonVirtualOffset / 8); + +} + +static FinalOverriders::BaseOffset +ComputeBaseOffset(ASTContext &Context, const CXXRecordDecl *BaseRD, + const CXXRecordDecl *DerivedRD) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, + /*RecordPaths=*/true, /*DetectVirtual=*/false); + + if (!const_cast(DerivedRD)-> + isDerivedFrom(const_cast(BaseRD), Paths)) { + assert(false && "Class must be derived from the passed in base class!"); + return FinalOverriders::BaseOffset(); + } + + return ComputeBaseOffset(Context, DerivedRD, Paths.front()); } static FinalOverriders::BaseOffset @@ -337,9 +342,12 @@ ComputeReturnAdjustmentBaseOffset(ASTContext &Context, } FinalOverriders::BaseOffset -FinalOverriders::ComputeThisAdjustmentBaseOffset(const CXXRecordDecl *BaseRD, - const CXXRecordDecl *DerivedRD) { - CXXBasePaths Paths(/*FindAmbiguities=*/false, +FinalOverriders::ComputeThisAdjustmentBaseOffset(BaseSubobject Base, + BaseSubobject Derived) { + const CXXRecordDecl *BaseRD = Base.getBase(); + const CXXRecordDecl *DerivedRD = Derived.getBase(); + + CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true, /*DetectVirtual=*/true); if (!const_cast(DerivedRD)-> @@ -350,10 +358,15 @@ FinalOverriders::ComputeThisAdjustmentBaseOffset(const CXXRecordDecl *BaseRD, assert(!Paths.getDetectedVirtual() && "FIXME: Handle virtual bases!"); - // FIXME: We need to go through all paths here, just not the first one. - BaseOffset Offset = ComputeBaseOffset(Context, BaseRD, DerivedRD); + BaseOffset Offset; + + // FIXME: This is not going to be enough with virtual bases. + // FIXME: We should not use / 8 here. + int64_t DerivedToBaseOffset = + (Base.getBaseOffset() - Derived.getBaseOffset()) / 8; + + Offset.NonVirtualOffset = -DerivedToBaseOffset; - Offset.NonVirtualOffset = -Offset.NonVirtualOffset; return Offset; } @@ -380,8 +393,9 @@ void FinalOverriders::PropagateOverrider(const CXXMethodDecl *OldMD, for (unsigned I = 0, E = OffsetVector.size(); I != E; ++I) { uint64_t Offset = OffsetVector[I]; + BaseSubobject OverriddenSubobject = BaseSubobject(OverriddenRD, Offset); BaseSubobjectMethodPairTy SubobjectAndMethod = - std::make_pair(BaseSubobject(OverriddenRD, Offset), OverriddenMD); + std::make_pair(OverriddenSubobject, OverriddenMD); OverriderInfo &Overrider = OverridersMap[SubobjectAndMethod]; @@ -402,8 +416,8 @@ void FinalOverriders::PropagateOverrider(const CXXMethodDecl *OldMD, // Check if we need a 'this' adjustment base offset as well. if (Offset != NewBase.getBaseOffset()) { BaseOffset ThisBaseOffset = - ComputeThisAdjustmentBaseOffset(OverriddenMD->getParent(), - NewMD->getParent()); + ComputeThisAdjustmentBaseOffset(OverriddenSubobject, + NewBase); assert(!ThisBaseOffset.isEmpty() && "Should not get an empty 'this' adjustment!"); @@ -921,7 +935,7 @@ void VtableBuilder::layoutVtable(BaseSubobject Base) { // First, add the offset to top. // FIXME: This is not going to be right for construction vtables. - // FIXME: We should not use -8 here. + // FIXME: We should not use / 8 here. int64_t OffsetToTop = -(int64_t)Base.getBaseOffset() / 8; Components.push_back(VtableComponent::MakeOffsetToTop(OffsetToTop)); diff --git a/test/CodeGenCXX/vtable-layout.cpp b/test/CodeGenCXX/vtable-layout.cpp index 181ebc5c3e..e151de14ed 100644 --- a/test/CodeGenCXX/vtable-layout.cpp +++ b/test/CodeGenCXX/vtable-layout.cpp @@ -279,4 +279,45 @@ struct C : A1, A2 { }; void C::f() { } -} \ No newline at end of file +} + +namespace Test7 { + +// Test that the D::f overrider for A::f have different 'this' pointer +// adjustments in the two A base subobjects. + +struct A { + virtual void f(); + int a; +}; + +struct B1 : A { }; +struct B2 : A { }; + +struct C { virtual void c(); }; + +// CHECK: Vtable for 'Test7::D' (10 entries). +// CHECK-NEXT: 0 | offset_to_top (0) +// CHECK-NEXT: 1 | Test7::D RTTI +// CHECK-NEXT: -- (Test7::C, 0) vtable address -- +// CHECK-NEXT: -- (Test7::D, 0) vtable address -- +// CHECK-NEXT: 2 | void Test7::C::c() +// CHECK-NEXT: 3 | void Test7::D::f() +// CHECK-NEXT: 4 | offset_to_top (-8) +// CHECK-NEXT: 5 | Test7::D RTTI +// CHECK-NEXT: -- (Test7::A, 8) vtable address -- +// CHECK-NEXT: -- (Test7::B1, 8) vtable address -- +// CHECK-NEXT: 6 | void Test7::D::f() +// CHECK-NEXT: [this adjustment: -8 non-virtual] +// CHECK-NEXT: 7 | offset_to_top (-24) +// CHECK-NEXT: 8 | Test7::D RTTI +// CHECK-NEXT: -- (Test7::A, 24) vtable address -- +// CHECK-NEXT: -- (Test7::B2, 24) vtable address -- +// CHECK-NEXT: 9 | void Test7::D::f() +// CHECK-NEXT: [this adjustment: -24 non-virtual] +struct D : C, B1, B2 { + virtual void f(); +}; +void D::f() { } + +} -- 2.40.0