Hi Richard & Stephen, I checked in an updated version at r177541. I took Richard's suggestion by checking the 'this' argument of the called constructor/destructor with 'this' pointer of the current function. Also updated the comments a little.
If you see any problem with it, please let me know. Thanks, Manman On Mar 15, 2013, at 5:28 PM, Richard Smith wrote: > On Fri, Mar 15, 2013 at 5:11 PM, Manman Ren <[email protected]> wrote: > Author: mren > Date: Fri Mar 15 19:11:09 2013 > New Revision: 177211 > > URL: http://llvm.org/viewvc/llvm-project?rev=177211&view=rev > Log: > Exploit this-return of a callsite in a this-return function. > > For constructors/desctructors that return 'this', if there exists a callsite > that returns 'this' and is immediately before the return instruction, make > sure we are using the return value from the callsite. > > We don't need to keep 'this' alive through the callsite. It also enables > optimizations in the backend, such as tail call optimization. > > rdar://12818789 > > Modified: > cfe/trunk/lib/CodeGen/CGCXXABI.h > cfe/trunk/lib/CodeGen/CGCall.cpp > cfe/trunk/lib/CodeGen/CGClass.cpp > cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > cfe/trunk/lib/CodeGen/CodeGenFunction.h > cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp > cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp > cfe/trunk/test/CodeGenCXX/arm.cpp > > Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=177211&r1=177210&r2=177211&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGCXXABI.h (original) > +++ cfe/trunk/lib/CodeGen/CGCXXABI.h Fri Mar 15 19:11:09 2013 > @@ -91,6 +91,10 @@ public: > return *MangleCtx; > } > > + /// Returns true if the given instance method is one of the > + /// kinds that the ABI says returns 'this'. > + virtual bool HasThisReturn(GlobalDecl GD) const { return false; } > + > /// Find the LLVM type used to represent the given member pointer > /// type. > virtual llvm::Type * > @@ -209,7 +213,8 @@ public: > /// Emit the ABI-specific prolog for the function. > virtual void EmitInstanceFunctionProlog(CodeGenFunction &CGF) = 0; > > - virtual void EmitConstructorCall(CodeGenFunction &CGF, > + /// Emit the constructor call. Return the function that is called. > + virtual llvm::Value *EmitConstructorCall(CodeGenFunction &CGF, > const CXXConstructorDecl *D, > CXXCtorType Type, bool ForVirtualBase, > bool Delegating, > > Modified: cfe/trunk/lib/CodeGen/CGCall.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=177211&r1=177210&r2=177211&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGCall.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGCall.cpp Fri Mar 15 19:11:09 2013 > @@ -1705,6 +1705,18 @@ void CodeGenFunction::EmitFunctionEpilog > llvm_unreachable("Invalid ABI kind for return argument"); > } > > + // If this function returns 'this' and the last instruction is a CallInst > + // that returns 'this', use the return value from the CallInst. We will not > + // need to keep 'this' alive through the callsite. It also enables > + // optimizations in the backend, such as tail call optimization. > + if (CalleeWithThisReturn && CGM.getCXXABI().HasThisReturn(CurGD)) { > + llvm::BasicBlock *IP = Builder.GetInsertBlock(); > + llvm::CallInst *Callsite; > + if (!IP->empty() && (Callsite = dyn_cast<llvm::CallInst>(&IP->back())) && > + Callsite->getCalledFunction() == CalleeWithThisReturn) > > This doesn't look right. Suppose we have: > > struct A { A(); /* trivial dtor */ }; > struct B : A { > B() : A() { A(); } > }; > > It looks like we'll return the 'this' from the temporary. Why not compare the > llvm::Value* corresponding to the CallInst itself, rather than comparing the > callee function? (Or maybe check that the 'this' argument to the callee > matches the caller's 'this'? That'd also solve the problems below.) > > + // Create a bitcast of Callsite. > + RV = Builder.CreateBitCast(Callsite, RetAI.getCoerceToType()); > + } > llvm::Instruction *Ret = RV ? Builder.CreateRet(RV) : > Builder.CreateRetVoid(); > if (!RetDbgLoc.isUnknown()) > Ret->setDebugLoc(RetDbgLoc); > > Modified: cfe/trunk/lib/CodeGen/CGClass.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=177211&r1=177210&r2=177211&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGClass.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGClass.cpp Fri Mar 15 19:11:09 2013 > @@ -1666,8 +1666,11 @@ CodeGenFunction::EmitCXXConstructorCall( > } > > // Non-trivial constructors are handled in an ABI-specific manner. > - CGM.getCXXABI().EmitConstructorCall(*this, D, Type, ForVirtualBase, > - Delegating, This, ArgBeg, ArgEnd); > + llvm::Value *Callee = CGM.getCXXABI().EmitConstructorCall(*this, D, Type, > + ForVirtualBase, Delegating, This, ArgBeg, > ArgEnd); > + if (CGM.getCXXABI().HasThisReturn(CurGD) && > + CGM.getCXXABI().HasThisReturn(GlobalDecl(D, Type))) > + CalleeWithThisReturn = Callee; > > There's no guarantee that the caller and the callee are constructing the same > object here. > > } > > void > @@ -1756,9 +1759,12 @@ CodeGenFunction::EmitDelegateCXXConstruc > EmitDelegateCallArg(DelegateArgs, param); > } > > + llvm::Value *Callee = CGM.GetAddrOfCXXConstructor(Ctor, CtorType); > EmitCall(CGM.getTypes().arrangeCXXConstructorDeclaration(Ctor, CtorType), > - CGM.GetAddrOfCXXConstructor(Ctor, CtorType), > - ReturnValueSlot(), DelegateArgs, Ctor); > + Callee, ReturnValueSlot(), DelegateArgs, Ctor); > + if (CGM.getCXXABI().HasThisReturn(CurGD) && > + CGM.getCXXABI().HasThisReturn(GlobalDecl(Ctor, CtorType))) > + CalleeWithThisReturn = Callee; > } > > namespace { > @@ -1825,6 +1831,9 @@ void CodeGenFunction::EmitCXXDestructorC > EmitCXXMemberCall(DD, SourceLocation(), Callee, ReturnValueSlot(), This, > VTT, getContext().getPointerType(getContext().VoidPtrTy), > 0, 0); > + if (CGM.getCXXABI().HasThisReturn(CurGD) && > + CGM.getCXXABI().HasThisReturn(GlobalDecl(DD, Type))) > + CalleeWithThisReturn = Callee; > > There's no guarantee that the caller and callee are destroying the same > object here. > > } > > namespace { > > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=177211&r1=177210&r2=177211&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Fri Mar 15 19:11:09 2013 > @@ -564,6 +564,9 @@ void CodeGenFunction::GenerateCode(Globa > SourceRange BodyRange; > if (Stmt *Body = FD->getBody()) BodyRange = Body->getSourceRange(); > > + // Reset CalleeWithThisReturn. > > Well, obviously. It would be better to explain why this is being done, not > what is being done. > > + CalleeWithThisReturn = 0; > + > // Emit the standard function prologue. > StartFunction(GD, ResTy, Fn, FnInfo, Args, BodyRange.getBegin()); > > @@ -615,6 +618,8 @@ void CodeGenFunction::GenerateCode(Globa > > // Emit the standard function epilogue. > FinishFunction(BodyRange.getEnd()); > + // Reset CalleeWithThisReturn. > + CalleeWithThisReturn = 0; > > // If we haven't marked the function nothrow through other means, do > // a quick pass now to see if we can. > > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=177211&r1=177210&r2=177211&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original) > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri Mar 15 19:11:09 2013 > @@ -1131,6 +1131,10 @@ private: > CGDebugInfo *DebugInfo; > bool DisableDebugInfo; > > + /// If the current function returns 'this', use the field to keep track of > + /// the callee that returns 'this'. > + llvm::Value *CalleeWithThisReturn; > + > /// DidCallStackSave - Whether llvm.stacksave has been called. Used to > avoid > /// calling llvm.stacksave for multiple VLAs in the same scope. > bool DidCallStackSave; > > Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=177211&r1=177210&r2=177211&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original) > +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Fri Mar 15 19:11:09 2013 > @@ -112,7 +112,7 @@ public: > > void EmitInstanceFunctionProlog(CodeGenFunction &CGF); > > - void EmitConstructorCall(CodeGenFunction &CGF, > + llvm::Value *EmitConstructorCall(CodeGenFunction &CGF, > const CXXConstructorDecl *D, > CXXCtorType Type, bool ForVirtualBase, > bool Delegating, > @@ -177,11 +177,11 @@ public: > llvm::Value *readArrayCookieImpl(CodeGenFunction &CGF, llvm::Value > *allocPtr, > CharUnits cookieSize); > > -private: > /// \brief Returns true if the given instance method is one of the > /// kinds that the ARM ABI says returns 'this'. > - static bool HasThisReturn(GlobalDecl GD) { > - const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl()); > + bool HasThisReturn(GlobalDecl GD) const { > + const CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(GD.getDecl()); > + if (!MD) return false; > return ((isa<CXXDestructorDecl>(MD) && GD.getDtorType() != > Dtor_Deleting) || > (isa<CXXConstructorDecl>(MD))); > } > @@ -834,7 +834,7 @@ void ARMCXXABI::EmitInstanceFunctionProl > CGF.Builder.CreateStore(getThisValue(CGF), CGF.ReturnValue); > } > > -void ItaniumCXXABI::EmitConstructorCall(CodeGenFunction &CGF, > +llvm::Value *ItaniumCXXABI::EmitConstructorCall(CodeGenFunction &CGF, > const CXXConstructorDecl *D, > CXXCtorType Type, bool > ForVirtualBase, > bool Delegating, > @@ -849,6 +849,7 @@ void ItaniumCXXABI::EmitConstructorCall( > // FIXME: Provide a source location here. > CGF.EmitCXXMemberCall(D, SourceLocation(), Callee, ReturnValueSlot(), This, > VTT, VTTTy, ArgBeg, ArgEnd); > + return Callee; > } > > RValue ItaniumCXXABI::EmitVirtualDestructorCall(CodeGenFunction &CGF, > > Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=177211&r1=177210&r2=177211&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original) > +++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Fri Mar 15 19:11:09 2013 > @@ -55,7 +55,7 @@ public: > > void EmitInstanceFunctionProlog(CodeGenFunction &CGF); > > - void EmitConstructorCall(CodeGenFunction &CGF, > + llvm::Value *EmitConstructorCall(CodeGenFunction &CGF, > const CXXConstructorDecl *D, > CXXCtorType Type, bool ForVirtualBase, > bool Delegating, > @@ -238,7 +238,7 @@ void MicrosoftCXXABI::EmitInstanceFuncti > } > } > > -void MicrosoftCXXABI::EmitConstructorCall(CodeGenFunction &CGF, > +llvm::Value *MicrosoftCXXABI::EmitConstructorCall(CodeGenFunction &CGF, > const CXXConstructorDecl *D, > CXXCtorType Type, bool > ForVirtualBase, > bool Delegating, > @@ -259,6 +259,7 @@ void MicrosoftCXXABI::EmitConstructorCal > CGF.EmitCXXMemberCall(D, SourceLocation(), Callee, ReturnValueSlot(), This, > ImplicitParam, ImplicitParamTy, > ArgBeg, ArgEnd); > + return Callee; > } > > RValue MicrosoftCXXABI::EmitVirtualDestructorCall(CodeGenFunction &CGF, > > Modified: cfe/trunk/test/CodeGenCXX/arm.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/arm.cpp?rev=177211&r1=177210&r2=177211&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenCXX/arm.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/arm.cpp Fri Mar 15 19:11:09 2013 > @@ -56,15 +56,15 @@ namespace test1 { > // CHECK: [[THIS:%.*]] = alloca [[A]]*, align 4 > // CHECK: store [[A]]* {{.*}}, [[A]]** [[THIS]] > // CHECK: [[THIS1:%.*]] = load [[A]]** [[THIS]] > - // CHECK: call [[A]]* @_ZN5test11AC2Ei( > - // CHECK: ret [[A]]* [[THIS1]] > + // CHECK: [[THIS2:%.*]] = call [[A]]* @_ZN5test11AC2Ei( > + // CHECK: ret [[A]]* [[THIS2]] > > // CHECK: define linkonce_odr [[A]]* @_ZN5test11AD1Ev([[A]]* %this) > unnamed_addr > // CHECK: [[THIS:%.*]] = alloca [[A]]*, align 4 > // CHECK: store [[A]]* {{.*}}, [[A]]** [[THIS]] > // CHECK: [[THIS1:%.*]] = load [[A]]** [[THIS]] > - // CHECK: call [[A]]* @_ZN5test11AD2Ev( > - // CHECK: ret [[A]]* [[THIS1]] > + // CHECK: [[THIS2:%.*]] = call [[A]]* @_ZN5test11AD2Ev( > + // CHECK: ret [[A]]* [[THIS2]] > } > > // Awkward virtual cases. > > > _______________________________________________ > 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
