Hi majnemer, rnk,
This also simplifies the handling of this argument adjustment when calling
destructors.
David,
Reid is on vacation and the revision log says you're quite familiar with these
files.
I'd like you to do a general review of the patch so I can commit sooner.
Reid,
I assume you'll do a thorough post-commit review when you're back.
http://llvm-reviews.chandlerc.com/D3054
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGExprCXX.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
Index: lib/CodeGen/CGCXXABI.h
===================================================================
--- lib/CodeGen/CGCXXABI.h
+++ lib/CodeGen/CGCXXABI.h
@@ -258,10 +258,12 @@
}
/// Perform ABI-specific "this" argument adjustment required prior to
- /// a virtual function call.
- virtual llvm::Value *adjustThisArgumentForVirtualCall(CodeGenFunction &CGF,
- GlobalDecl GD,
- llvm::Value *This) {
+ /// a call of a virtual function.
+ /// The "VirtualCall" argument is true iff the call itself is virtual.
+ virtual llvm::Value *
+ adjustThisArgumentForVirtualFunctionCall(CodeGenFunction &CGF, GlobalDecl GD,
+ llvm::Value *This,
+ bool VirtualCall) {
return This;
}
Index: lib/CodeGen/CGExprCXX.cpp
===================================================================
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -220,8 +220,10 @@
}
}
- if (MD->isVirtual())
- This = CGM.getCXXABI().adjustThisArgumentForVirtualCall(*this, MD, This);
+ if (MD->isVirtual()) {
+ This = CGM.getCXXABI().adjustThisArgumentForVirtualFunctionCall(
+ *this, MD, This, UseVirtualCall);
+ }
return EmitCXXMemberCall(MD, CE->getExprLoc(), Callee, ReturnValue, This,
/*ImplicitParam=*/0, QualType(),
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -935,9 +935,6 @@
if (!Callee)
Callee = CGM.GetAddrOfCXXDestructor(DD, Type);
- if (DD->isVirtual())
- This = adjustThisArgumentForVirtualCall(CGF, GD, This);
-
// FIXME: Provide a source location here.
CGF.EmitCXXMemberCall(DD, SourceLocation(), Callee, ReturnValueSlot(), This,
VTT, VTTTy, 0, 0);
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -148,9 +148,10 @@
return MD->getParent();
}
- llvm::Value *adjustThisArgumentForVirtualCall(CodeGenFunction &CGF,
- GlobalDecl GD,
- llvm::Value *This) override;
+ llvm::Value *
+ adjustThisArgumentForVirtualFunctionCall(CodeGenFunction &CGF, GlobalDecl GD,
+ llvm::Value *This,
+ bool VirtualCall) override;
void addImplicitStructorParams(CodeGenFunction &CGF, QualType &ResTy,
FunctionArgList &Params) override;
@@ -282,6 +283,8 @@
return C ? C : getZeroInt();
}
+ CharUnits getVirtualFunctionPrologueThisAdjustment(GlobalDecl GD);
+
void
GetNullMemberPointerFields(const MemberPointerType *MPT,
llvm::SmallVectorImpl<llvm::Constant *> &fields);
@@ -582,12 +585,61 @@
CGM.EmitGlobal(GlobalDecl(D, Dtor_Base));
}
-llvm::Value *MicrosoftCXXABI::adjustThisArgumentForVirtualCall(
- CodeGenFunction &CGF, GlobalDecl GD, llvm::Value *This) {
+CharUnits
+MicrosoftCXXABI::getVirtualFunctionPrologueThisAdjustment(GlobalDecl GD) {
+ GD = GD.getCanonicalDecl();
+ const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
+
+ GlobalDecl LookupGD = GD;
+ if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
+ // Complete destructors take a pointer to the complete object as a
+ // parameter, thus don't need this adjustment.
+ if (GD.getDtorType() == Dtor_Complete)
+ return CharUnits();
+
+ // There's no Dtor_Base in vftable but it shares the this adjustment with
+ // the deleting one, so look it up instead.
+ LookupGD = GlobalDecl(DD, Dtor_Deleting);
+ }
+
+ MicrosoftVTableContext::MethodVFTableLocation ML =
+ CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
+ CharUnits Adjustment = ML.VFPtrOffset;
+
+ // Normal virtual instance methods need to adjust from the vfptr that first
+ // defined the virtual method to the virtual base subobject, but destructors
+ // do not. The vector deleting destructor thunk applies this adjustment for
+ // us if necessary.
+ if (isa<CXXDestructorDecl>(MD))
+ Adjustment = CharUnits::Zero();
+
+ if (ML.VBase) {
+ const ASTRecordLayout &DerivedLayout =
+ CGM.getContext().getASTRecordLayout(MD->getParent());
+ Adjustment += DerivedLayout.getVBaseClassOffset(ML.VBase);
+ }
+
+ return Adjustment;
+}
+
+llvm::Value *MicrosoftCXXABI::adjustThisArgumentForVirtualFunctionCall(
+ CodeGenFunction &CGF, GlobalDecl GD, llvm::Value *This, bool VirtualCall) {
+ if (!VirtualCall) {
+ // If the call of a virtual function is not virtual, we just have to
+ // compensate for the adjustment the virtual function does in its prologue.
+ CharUnits Adjustment = getVirtualFunctionPrologueThisAdjustment(GD);
+ if (Adjustment.isZero())
+ return This;
+
+ unsigned AS = cast<llvm::PointerType>(This->getType())->getAddressSpace();
+ llvm::Type *charPtrTy = CGF.Int8Ty->getPointerTo(AS);
+ This = CGF.Builder.CreateBitCast(This, charPtrTy);
+ assert(Adjustment.isPositive());
+ return CGF.Builder.CreateConstGEP1_32(This, Adjustment.getQuantity());
+ }
+
GD = GD.getCanonicalDecl();
const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
- // FIXME: consider splitting the vdtor vs regular method code into two
- // functions.
GlobalDecl LookupGD = GD;
if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
@@ -614,50 +666,10 @@
StaticOffset = CharUnits::Zero();
if (ML.VBase) {
- bool AvoidVirtualOffset = false;
- if (isa<CXXDestructorDecl>(MD) && GD.getDtorType() == Dtor_Base) {
- // A base destructor can only be called from a complete destructor of the
- // same record type or another destructor of a more derived type;
- // or a constructor of the same record type if an exception is thrown.
- assert(isa<CXXDestructorDecl>(CGF.CurGD.getDecl()) ||
- isa<CXXConstructorDecl>(CGF.CurGD.getDecl()));
- const CXXRecordDecl *CurRD =
- cast<CXXMethodDecl>(CGF.CurGD.getDecl())->getParent();
-
- if (MD->getParent() == CurRD) {
- if (isa<CXXDestructorDecl>(CGF.CurGD.getDecl()))
- assert(CGF.CurGD.getDtorType() == Dtor_Complete);
- if (isa<CXXConstructorDecl>(CGF.CurGD.getDecl()))
- assert(CGF.CurGD.getCtorType() == Ctor_Complete);
- // We're calling the main base dtor from a complete structor,
- // so we know the "this" offset statically.
- AvoidVirtualOffset = true;
- } else {
- // Let's see if we try to call a destructor of a non-virtual base.
- for (CXXRecordDecl::base_class_const_iterator I = CurRD->bases_begin(),
- E = CurRD->bases_end(); I != E; ++I) {
- if (I->getType()->getAsCXXRecordDecl() != MD->getParent())
- continue;
- // If we call a base destructor for a non-virtual base, we statically
- // know where it expects the vfptr and "this" to be.
- // The total offset should reflect the adjustment done by
- // adjustThisParameterInVirtualFunctionPrologue().
- AvoidVirtualOffset = true;
- break;
- }
- }
- }
-
- if (AvoidVirtualOffset) {
- const ASTRecordLayout &Layout =
- CGF.getContext().getASTRecordLayout(MD->getParent());
- StaticOffset += Layout.getVBaseClassOffset(ML.VBase);
- } else {
- This = CGF.Builder.CreateBitCast(This, charPtrTy);
- llvm::Value *VBaseOffset =
- GetVirtualBaseClassOffset(CGF, This, MD->getParent(), ML.VBase);
- This = CGF.Builder.CreateInBoundsGEP(This, VBaseOffset);
- }
+ This = CGF.Builder.CreateBitCast(This, charPtrTy);
+ llvm::Value *VBaseOffset =
+ GetVirtualBaseClassOffset(CGF, This, MD->getParent(), ML.VBase);
+ This = CGF.Builder.CreateInBoundsGEP(This, VBaseOffset);
}
if (!StaticOffset.isZero()) {
assert(StaticOffset.isPositive());
@@ -717,44 +729,12 @@
llvm::Value *MicrosoftCXXABI::adjustThisParameterInVirtualFunctionPrologue(
CodeGenFunction &CGF, GlobalDecl GD, llvm::Value *This) {
- GD = GD.getCanonicalDecl();
- const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
-
- GlobalDecl LookupGD = GD;
- if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
- // Complete destructors take a pointer to the complete object as a
- // parameter, thus don't need this adjustment.
- if (GD.getDtorType() == Dtor_Complete)
- return This;
-
- // There's no Dtor_Base in vftable but it shares the this adjustment with
- // the deleting one, so look it up instead.
- LookupGD = GlobalDecl(DD, Dtor_Deleting);
- }
-
// In this ABI, every virtual function takes a pointer to one of the
// subobjects that first defines it as the 'this' parameter, rather than a
// pointer to the final overrider subobject. Thus, we need to adjust it back
// to the final overrider subobject before use.
// See comments in the MicrosoftVFTableContext implementation for the details.
-
- MicrosoftVTableContext::MethodVFTableLocation ML =
- CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
- CharUnits Adjustment = ML.VFPtrOffset;
-
- // Normal virtual instance methods need to adjust from the vfptr that first
- // defined the virtual method to the virtual base subobject, but destructors
- // do not. The vector deleting destructor thunk applies this adjustment for
- // us if necessary.
- if (isa<CXXDestructorDecl>(MD))
- Adjustment = CharUnits::Zero();
-
- if (ML.VBase) {
- const ASTRecordLayout &DerivedLayout =
- CGF.getContext().getASTRecordLayout(MD->getParent());
- Adjustment += DerivedLayout.getVBaseClassOffset(ML.VBase);
- }
-
+ CharUnits Adjustment = getVirtualFunctionPrologueThisAdjustment(GD);
if (Adjustment.isZero())
return This;
@@ -834,8 +814,12 @@
bool Delegating, llvm::Value *This) {
llvm::Value *Callee = CGM.GetAddrOfCXXDestructor(DD, Type);
- if (DD->isVirtual())
- This = adjustThisArgumentForVirtualCall(CGF, GlobalDecl(DD, Type), This);
+ if (DD->isVirtual()) {
+ assert(Type != CXXDtorType::Dtor_Deleting &&
+ "The deleting destructor should only be called via a virtual call");
+ This = adjustThisArgumentForVirtualFunctionCall(CGF, GlobalDecl(DD, Type),
+ This, false);
+ }
// FIXME: Provide a source location here.
CGF.EmitCXXMemberCall(DD, SourceLocation(), Callee, ReturnValueSlot(), This,
@@ -959,7 +943,8 @@
CGBuilderTy &Builder = CGF.Builder;
Ty = Ty->getPointerTo()->getPointerTo();
- llvm::Value *VPtr = adjustThisArgumentForVirtualCall(CGF, GD, This);
+ llvm::Value *VPtr =
+ adjustThisArgumentForVirtualFunctionCall(CGF, GD, This, true);
llvm::Value *VTable = CGF.GetVTablePtr(VPtr, Ty);
MicrosoftVTableContext::MethodVFTableLocation ML =
@@ -989,7 +974,7 @@
llvm::ConstantInt::get(llvm::IntegerType::getInt32Ty(CGF.getLLVMContext()),
DtorType == Dtor_Deleting);
- This = adjustThisArgumentForVirtualCall(CGF, GD, This);
+ This = adjustThisArgumentForVirtualFunctionCall(CGF, GD, This, true);
CGF.EmitCXXMemberCall(Dtor, CallLoc, Callee, ReturnValueSlot(), This,
ImplicitParam, Context.IntTy, 0, 0);
}
Index: test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
+++ test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
@@ -339,3 +339,41 @@
// CHECK2: ret
}
+
+namespace test3 {
+// PR19104: A non-virtual call of a virtual method doesn't use vftable thunks,
+// so requires only static adjustment which is different to the one used
+// for virtual calls.
+struct A {
+ virtual void foo();
+};
+
+struct B : virtual A {
+ virtual void bar();
+};
+
+struct C : virtual A {
+ virtual void foo();
+};
+
+struct D : B, C {
+ virtual void bar();
+ int field; // Laid out between C and A subobjects in D.
+};
+
+void D::bar() {
+ // CHECK-LABEL: define x86_thiscallcc void @"\01?bar@D@test3@@UAEXXZ"(%"struct.test3::D"* %this)
+
+ C::foo();
+ // Shouldn't need any vbtable lookups. All we have to do is adjust to C*,
+ // then compensate for the adjustment performed in the C::foo() prologue.
+ // CHECK-NOT: load i8**
+ // CHECK: %[[OBJ_i8:.*]] = bitcast %"struct.test3::D"* %{{.*}} to i8*
+ // CHECK: %[[C_i8:.*]] = getelementptr inbounds i8* %[[OBJ_i8]], i32 8
+ // CHECK: %[[C:.*]] = bitcast i8* %[[C_i8]] to %"struct.test3::C"*
+ // CHECK: %[[C_i8:.*]] = bitcast %"struct.test3::C"* %[[C]] to i8*
+ // CHECK: %[[ARG:.*]] = getelementptr i8* %[[C_i8]], i32 4
+ // CHECK: call x86_thiscallcc void @"\01?foo@C@test3@@UAEXXZ"(i8* %[[ARG]])
+ // CHECK: ret
+}
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
