[PATCH] Simplify CGF::Build*Virtual*Call
Eli Friedman
eli.friedman at gmail.com
Thu Jul 18 18:00:22 PDT 2013
The change to the signature of BuildVirtualCall is fine. Your patch
is moving so much code around that it's difficult to compare, but if
you're sure there isn't any functionality change, the patch is fine.
-Eli
On Wed, Jul 17, 2013 at 2:01 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
> Hi rjmccall,
>
> Hi John,
>
> Can you please review this simple patch?
> It removes some code duplication and simplifies the logic a bit.
>
> --
> Timur
>
> http://llvm-reviews.chandlerc.com/D1161
>
> Files:
> lib/CodeGen/ItaniumCXXABI.cpp
> lib/CodeGen/CodeGenFunction.h
> lib/CodeGen/MicrosoftCXXABI.cpp
> lib/CodeGen/CGCXX.cpp
>
> Index: lib/CodeGen/ItaniumCXXABI.cpp
> ===================================================================
> --- lib/CodeGen/ItaniumCXXABI.cpp
> +++ lib/CodeGen/ItaniumCXXABI.cpp
> @@ -834,7 +834,8 @@
> const CGFunctionInfo *FInfo
> = &CGM.getTypes().arrangeCXXDestructor(Dtor, DtorType);
> llvm::Type *Ty = CGF.CGM.getTypes().GetFunctionType(*FInfo);
> - llvm::Value *Callee = CGF.BuildVirtualCall(Dtor, DtorType, This, Ty);
> + llvm::Value *Callee
> + = CGF.BuildVirtualCall(GlobalDecl(Dtor, DtorType), This, Ty);
>
> CGF.EmitCXXMemberCall(Dtor, CallLoc, Callee, ReturnValueSlot(), This,
> /*ImplicitParam=*/0, QualType(), 0, 0);
> Index: lib/CodeGen/CodeGenFunction.h
> ===================================================================
> --- lib/CodeGen/CodeGenFunction.h
> +++ lib/CodeGen/CodeGenFunction.h
> @@ -2103,10 +2103,8 @@
> void EmitNoreturnRuntimeCallOrInvoke(llvm::Value *callee,
> ArrayRef<llvm::Value*> args);
>
> - llvm::Value *BuildVirtualCall(const CXXMethodDecl *MD, llvm::Value *This,
> + llvm::Value *BuildVirtualCall(GlobalDecl GD, llvm::Value *This,
> llvm::Type *Ty);
> - llvm::Value *BuildVirtualCall(const CXXDestructorDecl *DD, CXXDtorType Type,
> - llvm::Value *This, llvm::Type *Ty);
> llvm::Value *BuildAppleKextVirtualCall(const CXXMethodDecl *MD,
> NestedNameSpecifier *Qual,
> llvm::Type *Ty);
> Index: lib/CodeGen/MicrosoftCXXABI.cpp
> ===================================================================
> --- lib/CodeGen/MicrosoftCXXABI.cpp
> +++ lib/CodeGen/MicrosoftCXXABI.cpp
> @@ -490,7 +490,8 @@
> const CGFunctionInfo *FInfo
> = &CGM.getTypes().arrangeCXXDestructor(Dtor, Dtor_Deleting);
> llvm::Type *Ty = CGF.CGM.getTypes().GetFunctionType(*FInfo);
> - llvm::Value *Callee = CGF.BuildVirtualCall(Dtor, Dtor_Deleting, This, Ty);
> + llvm::Value *Callee
> + = CGF.BuildVirtualCall(GlobalDecl(Dtor, Dtor_Deleting), This, Ty);
>
> ASTContext &Context = CGF.getContext();
> llvm::Value *ImplicitParam
> Index: lib/CodeGen/CGCXX.cpp
> ===================================================================
> --- lib/CodeGen/CGCXX.cpp
> +++ lib/CodeGen/CGCXX.cpp
> @@ -291,33 +291,46 @@
> /*ForVTable=*/false));
> }
>
> -static llvm::Value *BuildVirtualCall(CodeGenFunction &CGF, uint64_t VTableIndex,
> - llvm::Value *This, llvm::Type *Ty) {
> +llvm::Value *
> +CodeGenFunction::BuildVirtualCall(GlobalDecl GD, llvm::Value *This,
> + llvm::Type *Ty) {
> + GD = GD.getCanonicalDecl();
> + uint64_t VTableIndex = CGM.getVTableContext().getMethodVTableIndex(GD);
> +
> Ty = Ty->getPointerTo()->getPointerTo();
> -
> - llvm::Value *VTable = CGF.GetVTablePtr(This, Ty);
> - llvm::Value *VFuncPtr =
> - CGF.Builder.CreateConstInBoundsGEP1_64(VTable, VTableIndex, "vfn");
> - return CGF.Builder.CreateLoad(VFuncPtr);
> + llvm::Value *VTable = GetVTablePtr(This, Ty);
> + llvm::Value *VFuncPtr =
> + Builder.CreateConstInBoundsGEP1_64(VTable, VTableIndex, "vfn");
> + return Builder.CreateLoad(VFuncPtr);
> }
>
> -llvm::Value *
> -CodeGenFunction::BuildVirtualCall(const CXXMethodDecl *MD, llvm::Value *This,
> - llvm::Type *Ty) {
> - MD = MD->getCanonicalDecl();
> - uint64_t VTableIndex = CGM.getVTableContext().getMethodVTableIndex(MD);
> -
> - return ::BuildVirtualCall(*this, VTableIndex, This, Ty);
> +static llvm::Value *BuildAppleKextVirtualCall(CodeGenFunction &CGF,
> + GlobalDecl GD,
> + llvm::Type *Ty,
> + const CXXRecordDecl *RD) {
> + GD = GD.getCanonicalDecl();
> + CodeGenModule &CGM = CGF.CGM;
> + llvm::Value *VTable = CGM.getVTables().GetAddrOfVTable(RD);
> + Ty = Ty->getPointerTo()->getPointerTo();
> + VTable = CGF.Builder.CreateBitCast(VTable, Ty);
> + assert(VTable && "BuildVirtualCall = kext vtbl pointer is null");
> + uint64_t VTableIndex = CGM.getVTableContext().getMethodVTableIndex(GD);
> + uint64_t AddressPoint =
> + CGM.getVTableContext().getVTableLayout(RD)
> + .getAddressPoint(BaseSubobject(RD, CharUnits::Zero()));
> + VTableIndex += AddressPoint;
> + llvm::Value *VFuncPtr =
> + CGF.Builder.CreateConstInBoundsGEP1_64(VTable, VTableIndex, "vfnkxt");
> + return CGF.Builder.CreateLoad(VFuncPtr);
> }
>
> -/// BuildVirtualCall - This routine is to support gcc's kext ABI making
> +/// BuildAppleKextVirtualCall - This routine is to support gcc's kext ABI making
> /// indirect call to virtual functions. It makes the call through indexing
> /// into the vtable.
> llvm::Value *
> CodeGenFunction::BuildAppleKextVirtualCall(const CXXMethodDecl *MD,
> NestedNameSpecifier *Qual,
> llvm::Type *Ty) {
> - llvm::Value *VTable = 0;
> assert((Qual->getKind() == NestedNameSpecifier::TypeSpec) &&
> "BuildAppleKextVirtualCall - bad Qual kind");
>
> @@ -329,20 +342,8 @@
>
> if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD))
> return BuildAppleKextVirtualDestructorCall(DD, Dtor_Complete, RD);
> -
> - VTable = CGM.getVTables().GetAddrOfVTable(RD);
> - Ty = Ty->getPointerTo()->getPointerTo();
> - VTable = Builder.CreateBitCast(VTable, Ty);
> - assert(VTable && "BuildVirtualCall = kext vtbl pointer is null");
> - MD = MD->getCanonicalDecl();
> - uint64_t VTableIndex = CGM.getVTableContext().getMethodVTableIndex(MD);
> - uint64_t AddressPoint =
> - CGM.getVTableContext().getVTableLayout(RD)
> - .getAddressPoint(BaseSubobject(RD, CharUnits::Zero()));
> - VTableIndex += AddressPoint;
> - llvm::Value *VFuncPtr =
> - Builder.CreateConstInBoundsGEP1_64(VTable, VTableIndex, "vfnkxt");
> - return Builder.CreateLoad(VFuncPtr);
> +
> + return ::BuildAppleKextVirtualCall(*this, MD, Ty, RD);
> }
>
> /// BuildVirtualCall - This routine makes indirect vtable call for
> @@ -352,42 +353,16 @@
> const CXXDestructorDecl *DD,
> CXXDtorType Type,
> const CXXRecordDecl *RD) {
> - llvm::Value * Callee = 0;
> const CXXMethodDecl *MD = cast<CXXMethodDecl>(DD);
> // FIXME. Dtor_Base dtor is always direct!!
> // It need be somehow inline expanded into the caller.
> // -O does that. But need to support -O0 as well.
> if (MD->isVirtual() && Type != Dtor_Base) {
> // Compute the function type we're calling.
> - const CGFunctionInfo &FInfo =
> - CGM.getTypes().arrangeCXXDestructor(cast<CXXDestructorDecl>(MD),
> - Dtor_Complete);
> + const CGFunctionInfo &FInfo =
> + CGM.getTypes().arrangeCXXDestructor(DD, Dtor_Complete);
> llvm::Type *Ty = CGM.getTypes().GetFunctionType(FInfo);
> -
> - llvm::Value *VTable = CGM.getVTables().GetAddrOfVTable(RD);
> - Ty = Ty->getPointerTo()->getPointerTo();
> - VTable = Builder.CreateBitCast(VTable, Ty);
> - DD = cast<CXXDestructorDecl>(DD->getCanonicalDecl());
> - uint64_t VTableIndex =
> - CGM.getVTableContext().getMethodVTableIndex(GlobalDecl(DD, Type));
> - uint64_t AddressPoint =
> - CGM.getVTableContext().getVTableLayout(RD)
> - .getAddressPoint(BaseSubobject(RD, CharUnits::Zero()));
> - VTableIndex += AddressPoint;
> - llvm::Value *VFuncPtr =
> - Builder.CreateConstInBoundsGEP1_64(VTable, VTableIndex, "vfnkxt");
> - Callee = Builder.CreateLoad(VFuncPtr);
> + return ::BuildAppleKextVirtualCall(*this, GlobalDecl(DD, Type), Ty, RD);
> }
> - return Callee;
> + return 0;
> }
> -
> -llvm::Value *
> -CodeGenFunction::BuildVirtualCall(const CXXDestructorDecl *DD, CXXDtorType Type,
> - llvm::Value *This, llvm::Type *Ty) {
> - DD = cast<CXXDestructorDecl>(DD->getCanonicalDecl());
> - uint64_t VTableIndex =
> - CGM.getVTableContext().getMethodVTableIndex(GlobalDecl(DD, Type));
> -
> - return ::BuildVirtualCall(*this, VTableIndex, This, Ty);
> -}
> -
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list