[ObjC][ARC] Ignore lifetime markers between *ReturnValue calls

[thegameg]

Summary

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.

[rjmccall]

Generally LGTM, but one minor thing.

llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp

664

isSafeBetweenRVCalls also checks IsNoopInstruction.

[thegameg]

Remove the double check for IsNoopInstruction.

[rjmccall]

LGTM, thanks!

llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp

 
 void ObjCARCOpt::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<ObjCARCAAWrapperPass>();
   AU.addRequired<AAResultsWrapperPass>();
   // ARC optimization doesn't currently split critical edges.
   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.
 bool
 ObjCARCOpt::OptimizeRetainRVCall(Function &F, Instruction *RetainRV) {
   // Check for the argument being from an immediately preceding call or invoke.
   const Value *Arg = GetArgRCIdentityRoot(RetainRV);
   ImmutableCallSite CS(Arg);
ObjCARCOpt::OptimizeRetainRVCall(Function &F, Instruction *RetainRV) {
 
   // Check for being preceded by an objc_autoreleaseReturnValue on the same
   // pointer. In this case, we can delete the pair.
   BasicBlock::iterator I = RetainRV->getIterator(),
                        Begin = RetainRV->getParent()->begin();
   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;
       ++NumPeeps;
 
       LLVM_DEBUG(dbgs() << "Erasing autoreleaseRV,retainRV pair: " << *I << "\n"
                         << "Erasing " << *RetainRV << "\n");
 

llvm/test/Transforms/ObjCARC/post-inlining.ll

 
 declare void @llvm.lifetime.start.p0i8(i64, i8*)
 declare void @llvm.lifetime.end.p0i8(i64, i8*)
 
 ; CHECK-LABEL: define i8* @testLifetime(
 ; 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) {
 entry:
   %obj = alloca i8
   call void @llvm.lifetime.start.p0i8(i64 8, i8* %obj)
   %0 = tail call i8* @llvm.objc.autoreleaseReturnValue(i8* %call.i) nounwind
   call void @llvm.lifetime.end.p0i8(i64 8, i8* %obj)
   %1 = tail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %call.i) nounwind
   ret i8* %call.i
 }
 
 ; 3) Dynamic alloca markers.
 
 declare i8* @llvm.stacksave()
 declare void @llvm.stackrestore(i8*)
 
 ; CHECK-LABEL: define i8* @testStack(
 ; 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) {
 entry:
   %save = tail call i8* @llvm.stacksave()
   %obj = alloca i8, i8 %arg
   %0 = tail call i8* @llvm.objc.autoreleaseReturnValue(i8* %call.i) nounwind
   call void @llvm.stackrestore(i8* %save)
   %1 = tail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %call.i) nounwind
   ret i8* %call.i
 }