[PATCH] D69833: [ObjC][ARC] Ignore lifetime markers between *ReturnValue calls

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg updated this revision to Diff 227805.
thegameg added a comment.

Remove the double check for `IsNoopInstruction`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69833/new/

https://reviews.llvm.org/D69833

Files:
  llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
  llvm/test/Transforms/ObjCARC/post-inlining.ll


Index: llvm/test/Transforms/ObjCARC/post-inlining.ll
===================================================================
--- llvm/test/Transforms/ObjCARC/post-inlining.ll
+++ llvm/test/Transforms/ObjCARC/post-inlining.ll
@@ -76,9 +76,7 @@
 ; 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 @@
 ; 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) {
Index: llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
===================================================================
--- llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
+++ llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
@@ -588,6 +588,33 @@
   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 @@
   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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69833.227805.patch
Type: text/x-patch
Size: 2617 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191105/c99b3547/attachment.bin>