[ObjC][ARC] Ignore lifetime markers between *ReturnValue calls
authorFrancis Visoiu Mistrih <francisvm@yahoo.com>
Tue, 5 Nov 2019 00:28:23 +0000 (16:28 -0800)
committerFrancis Visoiu Mistrih <francisvm@yahoo.com>
Tue, 5 Nov 2019 14:39:22 +0000 (06:39 -0800)
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
llvm/test/Transforms/ObjCARC/post-inlining.ll

index 6653ff0..87d6bed 100644 (file)
@@ -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;
index 65b1153..8e4e762 100644 (file)
@@ -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) {