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

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg created this revision.
thegameg added reviewers: ahatanak, rjmccall.
Herald added subscribers: dexonsmith, hiraditya.
Herald added a project: LLVM.

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.


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 && (IsNoopInstruction(&*I) || isSafeBetweenRVCalls(&*I)));
     if (GetBasicARCInstKind(&*I) == ARCInstKind::AutoreleaseRV &&
         EquivalentArgs.count(GetArgRCIdentityRoot(&*I))) {
       Changed = true;


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