[ObjCARC] Allow removal of autoreleasepools

[AtariDreams]

We did this for inserted pools, but we can do this with user-inserted ones if we can prove they do not.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: AtariDreams (AtariDreams)

Changes

We did this for inserted pools, but we can do this with user-inserted ones if we can prove they do not.

---

Full diff: https://github.com/llvm/llvm-project/pull/85493.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/ObjCARC/ObjCARCAPElim.cpp (+5-4)
• (modified) llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp (+77-1)

diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCAPElim.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCAPElim.cpp
index dceb2ebb1863e7..ee8febe4033b02 100644
--- a/llvm/lib/Transforms/ObjCARC/ObjCARCAPElim.cpp
+++ b/llvm/lib/Transforms/ObjCARC/ObjCARCAPElim.cpp
@@ -46,13 +46,14 @@ bool MayAutorelease(const CallBase &CB, unsigned Depth = 0) {
   if (const Function *Callee = CB.getCalledFunction()) {
     if (!Callee->hasExactDefinition())
       return true;
+    // This recursion depth limit is big enough to cover the vast majority
+    // of cases this will ever require.
+    if (Depth > 64)
+      return true;
     for (const BasicBlock &BB : *Callee) {
       for (const Instruction &I : BB)
         if (const CallBase *JCB = dyn_cast<CallBase>(&I))
-          // This recursion depth limit is arbitrary. It's just great
-          // enough to cover known interesting testcases.
-          if (Depth < 3 && !JCB->onlyReadsMemory() &&
-              MayAutorelease(*JCB, Depth + 1))
+          if (!JCB->onlyReadsMemory() && MayAutorelease(*JCB, Depth + 1))
             return true;
     }
     return false;
diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
index 72e860d7dcfa61..c0a0863816d120 100644
--- a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
+++ b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
@@ -521,6 +521,9 @@ class ObjCARCOpt {
                                         Instruction *AutoreleaseRV,
                                         const Value *&AutoreleaseRVArg);
 
+  /// Remove any autorelease pools if nothing is released between them
+  void ObjCARCOpt::OptimizeAutoreleasePools(Function &F);
+
   void CheckForCFGHazards(const BasicBlock *BB,
                           DenseMap<const BasicBlock *, BBState> &BBStates,
                           BBState &MyStates) const;
@@ -756,6 +759,73 @@ void ObjCARCOpt::OptimizeAutoreleaseRVCall(Function &F,
   LLVM_DEBUG(dbgs() << "New: " << *AutoreleaseRV << "\n");
 }
 
+/// Interprocedurally determine if calls made by the given call site can
+/// possibly produce autoreleases.
+bool MayAutorelease(const CallBase &CB, unsigned Depth = 0) {
+  if (const Function *Callee = CB.getCalledFunction()) {
+    if (!Callee->hasExactDefinition())
+      return true;
+    // This recursion depth limit is big enough to cover the vast majority
+    // of cases this will ever require.
+    if (Depth > 64)
+      return true;
+    for (const BasicBlock &BB : *Callee) {
+      for (const Instruction &I : BB)
+        if (const CallBase *JCB = dyn_cast<CallBase>(&I))
+          if (!JCB->onlyReadsMemory() && MayAutorelease(*JCB, Depth + 1))
+            return true;
+    }
+    return false;
+  }
+
+  return true;
+}
+
+// Remove autorelease pools if nothing is in between then
+void ObjCARCOpt::OptimizeAutoreleasePools(Function &F) {
+  Instruction *Push = nullptr;
+  for (BasicBlock &BB : F) {
+    for (Instruction &Inst : BB) {
+      // FIXME: For now, only support pairs that are in the same block.
+      // It is possible to do this across other blocks, but only if you can
+      // prove every single pop call site corresponds to the push via going
+      // through each successor and checking every single one, and I don't even
+      // know if the push call could be split across call sites.
+      bool foundAnyInBlock = false;
+      switch (GetBasicARCInstKind(&Inst)) {
+      case ARCInstKind::AutoreleasepoolPush:
+        Push = &Inst;
+        foundAnyInBlock = true;
+        break;
+      case ARCInstKind::AutoreleasepoolPop:
+        // If this pop matches a push and nothing in between can autorelease,
+        // zap the pair.
+        if (Push && cast<CallInst>(&Inst)->getArgOperand(0) == Push &&
+            foundAnyInBlock) {
+          Changed = true;
+          LLVM_DEBUG(dbgs()
+                     << "ObjCARCAPElim::OptimizeBB: Zapping push pop "
+                        "autorelease pair:\n"
+                        "                           Pop: "
+                     << Inst << "\n"
+                     << "                           Push: " << *Push << "\n");
+
+          Inst.eraseFromParent();
+          Push->eraseFromParent();
+        }
+        Push = nullptr;
+        break;
+      case ARCInstKind::CallOrUser:
+        if (MayAutorelease(cast<CallBase>(Inst)))
+          Push = nullptr;
+        break;
+      default:
+        break;
+      }
+    }
+  }
+}
+
 /// Visit each call, one at a time, and make simplifications without doing any
 /// additional analysis.
 void ObjCARCOpt::OptimizeIndividualCalls(Function &F) {
@@ -2485,7 +2555,13 @@ bool ObjCARCOpt::run(Function &F, AAResults &AA) {
                             (1 << unsigned(ARCInstKind::AutoreleaseRV))))
     OptimizeReturns(F);
 
-  // Gather statistics after optimization.
+  if (UsedInThisFunction & (1 << unsigned(ARCInstKind::AutoreleasepoolPush)))
+    // Normally should not happen but there has been code where manual calls to
+    // objc autorelease have been made.
+    if (UsedInThisFunction & (1 << unsigned(ARCInstKind::AutoreleasepoolPop)))
+      OptimizeAutoreleasePools(F);
+
+      // Gather statistics after optimization.
 #ifndef NDEBUG
   if (AreStatisticsEnabled()) {
     GatherStatistics(F, true);

[fhahn]

Could you add some tests for the new functionality?

[rjmccall]

Please only do this analysis if there's actually a push we're considering optimizing.

[ahatanak]

Can you make changes to OptimizeRetainRVCall in a separate patch? The function always returns false, so it'll be a NFC patch.