From 47d1029788b6744596bb1ab6791450eb32f2a488 Mon Sep 17 00:00:00 2001
From: Francis Visoiu Mistrih <francisvm@yahoo.com>
Date: Mon, 4 Nov 2019 16:28:23 -0800
Subject: [PATCH] [ObjC][ARC] Ignore lifetime markers between *ReturnValue
 calls

When eliminating a pair of

`llvm.objc.autoreleaseReturnValue`

followed by

`llvm.objc.retainAutoreleasedReturnValue`

we need to make sure that the instructions in between are safe to
ignore.

Other than bitcasts and useless GEPs, it's also safe to ignore lifetime
markers for both static allocas (lifetime.start/lifetime.end) and dynamic
allocas (stacksave/stackrestore).

These get added by the inliner as part of the return sequence and can
prevent the transformation from happening in practice.

Differential Revision: https://reviews.llvm.org/D69833
---
 llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp   | 29 ++++++++++++++++++-
 llvm/test/Transforms/ObjCARC/post-inlining.ll |  4 ---
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
index 6653ff0bb91a..87d6bed6b1bd 100644
--- a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
+++ b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
@@ -588,6 +588,33 @@ void ObjCARCOpt::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.setPreservesCFG();
 }
 
+static bool isSafeBetweenRVCalls(const Instruction *I) {
+  if (IsNoopInstruction(I))
+    return true;
+
+  auto *CB = dyn_cast<CallBase>(I);
+  if (!CB)
+    return false;
+
+  Intrinsic::ID IID = CB->getIntrinsicID();
+  if (IID == Intrinsic::not_intrinsic)
+    return false;
+
+  switch (IID) {
+  case Intrinsic::lifetime_start:
+  case Intrinsic::lifetime_end:
+    // The inliner adds new lifetime markers as part of the return sequence,
+    // which should be skipped when looking for paired return RV call.
+    LLVM_FALLTHROUGH;
+  case Intrinsic::stacksave:
+  case Intrinsic::stackrestore:
+    // If the inlined code contains dynamic allocas, the above applies as well.
+    return true;
+  default:
+    return false;
+  }
+}
+
 /// Turn objc_retainAutoreleasedReturnValue into objc_retain if the operand is
 /// not a return value.  Or, if it can be paired with an
 /// objc_autoreleaseReturnValue, delete the pair and return true.
@@ -634,7 +661,7 @@ ObjCARCOpt::OptimizeRetainRVCall(Function &F, Instruction *RetainRV) {
   if (I != Begin) {
     do
       --I;
-    while (I != Begin && IsNoopInstruction(&*I));
+    while (I != Begin && isSafeBetweenRVCalls(&*I));
     if (GetBasicARCInstKind(&*I) == ARCInstKind::AutoreleaseRV &&
         EquivalentArgs.count(GetArgRCIdentityRoot(&*I))) {
       Changed = true;
diff --git a/llvm/test/Transforms/ObjCARC/post-inlining.ll b/llvm/test/Transforms/ObjCARC/post-inlining.ll
index 65b1153c1367..8e4e762c656c 100644
--- a/llvm/test/Transforms/ObjCARC/post-inlining.ll
+++ b/llvm/test/Transforms/ObjCARC/post-inlining.ll
@@ -76,9 +76,7 @@ declare void @llvm.lifetime.end.p0i8(i64, i8*)
 ; CHECK: entry:
 ; CHECK-NEXT: %obj = alloca i8
 ; CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* %obj)
-; CHECK-NEXT: %0 = tail call i8* @llvm.objc.autoreleaseReturnValue(i8* %call.i)
 ; CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* %obj)
-; CHECK-NEXT: %1 = tail call i8* @llvm.objc.retain(i8* %call.i)
 ; CHECK-NEXT: ret i8* %call.i
 ; CHECK-NEXT: }
 define i8* @testLifetime(i8* %call.i) {
@@ -100,9 +98,7 @@ declare void @llvm.stackrestore(i8*)
 ; CHECK: entry:
 ; CHECK-NEXT: %save = tail call i8* @llvm.stacksave()
 ; CHECK-NEXT: %obj = alloca i8, i8 %arg
-; CHECK-NEXT: %0 = tail call i8* @llvm.objc.autoreleaseReturnValue(i8* %call.i)
 ; CHECK-NEXT: call void @llvm.stackrestore(i8* %save)
-; CHECK-NEXT: %1 = tail call i8* @llvm.objc.retain(i8* %call.i)
 ; CHECK-NEXT: ret i8* %call.i
 ; CHECK-NEXT: }
 define i8* @testStack(i8* %call.i, i8 %arg) {