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 <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
