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.

Updated from r177211.
rdar://12818789

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177541 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/CGCXXABI.h b/lib/CodeGen/CGCXXABI.h
index cdc87b70e554107cf405fb5320b0168966a432c9..d0384ecc128250f1f40f9470a17ceda5891d2206 100644 (file)
--- a/lib/CodeGen/CGCXXABI.h
+++ b/lib/CodeGen/CGCXXABI.h
@@ -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,

diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp
index 9e97bce6c3eafabc971070b7e7b3e102e0b00bde..9318973e64e5b2547399b1f084683aa0d138f908 100644 (file)
--- a/lib/CodeGen/CGCall.cpp
+++ b/lib/CodeGen/CGCall.cpp
@@ -1614,6 +1614,18 @@ static llvm::StoreInst *findDominatingStoreToReturnValue(CodeGenFunction &CGF) {
   return store;
 }
 
+/// Check whether 'this' argument of a callsite matches 'this' of the caller.
+static bool checkThisPointer(llvm::Value *ThisArg, llvm::Value *This) {
+  if (ThisArg == This)
+    return true;
+  // Check whether ThisArg is a bitcast of This.
+  llvm::BitCastInst *Bitcast;
+  if ((Bitcast = dyn_cast<llvm::BitCastInst>(ThisArg)) &&
+      Bitcast->getOperand(0) == This)
+    return true;
+  return false;
+}
+
 void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo &FI) {
   // Functions with no result always return void.
   if (ReturnValue == 0) {
@@ -1705,6 +1717,19 @@ void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo &FI) {
     llvm_unreachable("Invalid ABI kind for return argument");
   }
 
+  // If this function returns 'this', the last instruction is a CallInst
+  // that returns 'this', and 'this' argument of the CallInst points to
+  // the same object as CXXThisValue, 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 &&
+        checkThisPointer(Callsite->getOperand(0), CXXThisValue))
+      RV = Builder.CreateBitCast(Callsite, RetAI.getCoerceToType());
+  }
   llvm::Instruction *Ret = RV ? Builder.CreateRet(RV) : Builder.CreateRetVoid();
   if (!RetDbgLoc.isUnknown())
     Ret->setDebugLoc(RetDbgLoc);

diff --git a/lib/CodeGen/CGClass.cpp b/lib/CodeGen/CGClass.cpp
index 287d164cb9924bfdfcb7d5f5fef66c604333f47e..2ececb03651ad3b47f15225bb3e2769aeadad6f8 100644 (file)
--- a/lib/CodeGen/CGClass.cpp
+++ b/lib/CodeGen/CGClass.cpp
@@ -1666,8 +1666,11 @@ CodeGenFunction::EmitCXXConstructorCall(const CXXConstructorDecl *D,
   }
 
   // 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;
 }
 
 void
@@ -1756,9 +1759,12 @@ CodeGenFunction::EmitDelegateCXXConstructorCall(const CXXConstructorDecl *Ctor,
     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::EmitCXXDestructorCall(const CXXDestructorDecl *DD,
   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;
 }
 
 namespace {

diff --git a/lib/CodeGen/CodeGenFunction.cpp b/lib/CodeGen/CodeGenFunction.cpp
index 27ef65fa94b47bb598eb793fd7402cfa6025fd17..bd244355f0edd2f35f582a08944b50dd0cba124a 100644 (file)
--- a/lib/CodeGen/CodeGenFunction.cpp
+++ b/lib/CodeGen/CodeGenFunction.cpp
@@ -564,6 +564,10 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
   SourceRange BodyRange;
   if (Stmt *Body = FD->getBody()) BodyRange = Body->getSourceRange();
 
+  // CalleeWithThisReturn keeps track of the last callee inside this function
+  // that returns 'this'. Before starting the function, we set it to null.
+  CalleeWithThisReturn = 0;
+
   // Emit the standard function prologue.
   StartFunction(GD, ResTy, Fn, FnInfo, Args, BodyRange.getBegin());
 
@@ -615,6 +619,9 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
 
   // Emit the standard function epilogue.
   FinishFunction(BodyRange.getEnd());
+  // CalleeWithThisReturn keeps track of the last callee inside this function
+  // that returns 'this'. After finishing the function, we set it to null.
+  CalleeWithThisReturn = 0;
 
   // If we haven't marked the function nothrow through other means, do
   // a quick pass now to see if we can.

diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index 55c21e80ee2659182a896f8307b55d970435d61c..19a4d571ee2bc8ad6176e3243c9ef43e59e2bc09 100644 (file)
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -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;

diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp
index 8c7a759a43ea226c3e40cb12665d8acd1047407c..17e83a18b9723eaad544e0dad44acf0d2ef20a74 100644 (file)
--- a/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/lib/CodeGen/ItaniumCXXABI.cpp
@@ -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::EmitInstanceFunctionProlog(CodeGenFunction &CGF) {
     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(CodeGenFunction &CGF,
   // FIXME: Provide a source location here.
   CGF.EmitCXXMemberCall(D, SourceLocation(), Callee, ReturnValueSlot(), This,
                         VTT, VTTTy, ArgBeg, ArgEnd);
+  return Callee;
 }
 
 RValue ItaniumCXXABI::EmitVirtualDestructorCall(CodeGenFunction &CGF,

diff --git a/lib/CodeGen/MicrosoftCXXABI.cpp b/lib/CodeGen/MicrosoftCXXABI.cpp
index fb6b86d87883ddded20d7fad3a17744b1f5ac718..85d926023f38a7d6eac634de11922c7b149035e0 100644 (file)
--- a/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -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::EmitInstanceFunctionProlog(CodeGenFunction &CGF) {
   }
 }
 
-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::EmitConstructorCall(CodeGenFunction &CGF,
   CGF.EmitCXXMemberCall(D, SourceLocation(), Callee, ReturnValueSlot(), This,
                         ImplicitParam, ImplicitParamTy,
                         ArgBeg, ArgEnd);
+  return Callee;
 }
 
 RValue MicrosoftCXXABI::EmitVirtualDestructorCall(CodeGenFunction &CGF,

diff --git a/test/CodeGenCXX/arm.cpp b/test/CodeGenCXX/arm.cpp
index 3d3b147aa1ff736951300f0c0600d90c25737a50..48f2f008401568b75b1760676fddc1133df65c72 100644 (file)
--- a/test/CodeGenCXX/arm.cpp
+++ b/test/CodeGenCXX/arm.cpp
@@ -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.

diff --git a/test/CodeGenCXX/constructor-destructor-return-this.cpp b/test/CodeGenCXX/constructor-destructor-return-this.cpp
new file mode 100644 (file)
index 0000000..1ff922d
--- /dev/null
+++ b/test/CodeGenCXX/constructor-destructor-return-this.cpp
@@ -0,0 +1,60 @@
+//RUN: %clang_cc1 %s -emit-llvm -o - -triple=thumbv7-apple-ios3.0 -target-abi apcs-gnu | FileCheck %s
+
+// 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.
+// rdar://12818789
+
+// CHECK: define linkonce_odr [[A:%.*]] @_ZN11ObjectCacheC1Ev([[A]] %this) unnamed_addr
+// CHECK: [[THIS1:%.*]] = call [[A]] @_ZN11ObjectCacheC2Ev(
+// CHECK-NEXT: ret [[A]] [[THIS1]]
+
+// CHECK: define linkonce_odr [[A:%.*]] @_ZN5TimerI11ObjectCacheEC1EPS0_MS0_FvPS1_E([[A]] %this
+// CHECK: [[THIS1:%.*]] = call [[A]] @_ZN5TimerI11ObjectCacheEC2EPS0_MS0_FvPS1_E(
+// CHECK-NEXT: ret [[A]] [[THIS1]]
+
+// CHECK: define linkonce_odr [[A:%.*]] @_ZN5TimerI11ObjectCacheED1Ev([[A]] %this) unnamed_addr
+// CHECK: [[THIS1:%.*]] = call [[A]] @_ZN5TimerI11ObjectCacheED2Ev(
+// CHECK-NEXT: ret [[A]] [[THIS1]]
+
+// CHECK: define linkonce_odr [[A:%.*]] @_ZN5TimerI11ObjectCacheED2Ev([[A]] %this) unnamed_addr
+// CHECK: [[THIS1:%.*]] = call [[B:%.*]] @_ZN9TimerBaseD2Ev(
+// CHECK-NEXT: [[THIS2:%.*]] = bitcast [[B]] [[THIS1]] to [[A]]
+// CHECK-NEXT: ret [[A]] [[THIS2]]
+
+class TimerBase {
+public:
+    TimerBase();
+    virtual ~TimerBase();
+};
+
+template <typename TimerFiredClass> class Timer : public TimerBase {
+public:
+    typedef void (TimerFiredClass::*TimerFiredFunction)(Timer*);
+
+    Timer(TimerFiredClass* o, TimerFiredFunction f)
+        : m_object(o), m_function(f) { }
+
+private:
+    virtual void fired() { (m_object->*m_function)(this); }
+
+    TimerFiredClass* m_object;
+    TimerFiredFunction m_function;
+};
+
+class ObjectCache {
+public:
+    explicit ObjectCache();
+    ~ObjectCache();
+
+private:
+    Timer<ObjectCache> m_notificationPostTimer;
+};
+
+inline ObjectCache::ObjectCache() : m_notificationPostTimer(this, 0) { }
+inline ObjectCache::~ObjectCache() { }
+
+ObjectCache *test() {
+  ObjectCache *dd = new ObjectCache();
+  return dd;
+}