[ObjC][ARC] Add and use a function which finds and returns the single

ahatanaka · ahatanaka · commit 2ed3a76745f9 · 2020-11-13T14:02:58.000-08:00
dependency. NFC

Use findSingleDependency in place of FindDependencies and stop passing a
set of Instructions around. Modify FindDependencies to return a boolean
flag which indicates whether the dependencies it has found are all
valid.
diff --git a/llvm/lib/Transforms/ObjCARC/DependencyAnalysis.cpp b/llvm/lib/Transforms/ObjCARC/DependencyAnalysis.cpp
@@ -208,12 +208,10 @@ llvm::objcarc::Depends(DependenceKind Flavor, Instruction *Inst,
 /// non-local dependencies on Arg.
 ///
 /// TODO: Cache results?
-void
-llvm::objcarc::FindDependencies(DependenceKind Flavor,
-                                const Value *Arg,
-                                BasicBlock *StartBB, Instruction *StartInst,
-                                SmallPtrSetImpl<Instruction *> &DependingInsts,
-                                ProvenanceAnalysis &PA) {
+static bool findDependencies(DependenceKind Flavor, const Value *Arg,
+                             BasicBlock *StartBB, Instruction *StartInst,
+                             SmallPtrSetImpl<Instruction *> &DependingInsts,
+                             ProvenanceAnalysis &PA) {
   BasicBlock::iterator StartPos = StartInst->getIterator();
 
   SmallPtrSet<const BasicBlock *, 4> Visited;
@@ -229,15 +227,14 @@ llvm::objcarc::FindDependencies(DependenceKind Flavor,
       if (LocalStartPos == StartBBBegin) {
         pred_iterator PI(LocalStartBB), PE(LocalStartBB, false);
         if (PI == PE)
-          // If we've reached the function entry, produce a null dependence.
-          DependingInsts.insert(nullptr);
-        else
-          // Add the predecessors to the worklist.
-          do {
-            BasicBlock *PredBB = *PI;
-            if (Visited.insert(PredBB).second)
-              Worklist.push_back(std::make_pair(PredBB, PredBB->end()));
-          } while (++PI != PE);
+          // Return if we've reached the function entry.
+          return false;
+        // Add the predecessors to the worklist.
+        do {
+          BasicBlock *PredBB = *PI;
+          if (Visited.insert(PredBB).second)
+            Worklist.push_back(std::make_pair(PredBB, PredBB->end()));
+        } while (++PI != PE);
         break;
       }
 
@@ -256,9 +253,22 @@ llvm::objcarc::FindDependencies(DependenceKind Flavor,
     if (BB == StartBB)
       continue;
     for (const BasicBlock *Succ : successors(BB))
-      if (Succ != StartBB && !Visited.count(Succ)) {
-        DependingInsts.insert(reinterpret_cast<Instruction *>(-1));
-        return;
-      }
+      if (Succ != StartBB && !Visited.count(Succ))
+        return false;
   }
+
+  return true;
+}
+
+llvm::Instruction *llvm::objcarc::findSingleDependency(DependenceKind Flavor,
+                                                       const Value *Arg,
+                                                       BasicBlock *StartBB,
+                                                       Instruction *StartInst,
+                                                       ProvenanceAnalysis &PA) {
+  SmallPtrSet<Instruction *, 4> DependingInsts;
+
+  if (!findDependencies(Flavor, Arg, StartBB, StartInst, DependingInsts, PA) ||
+      DependingInsts.size() != 1)
+    return nullptr;
+  return *DependingInsts.begin();
 }
diff --git a/llvm/lib/Transforms/ObjCARC/DependencyAnalysis.h b/llvm/lib/Transforms/ObjCARC/DependencyAnalysis.h
@@ -50,11 +50,12 @@ enum DependenceKind {
   RetainRVDep                 ///< Blocks objc_retainAutoreleasedReturnValue.
 };
 
-void FindDependencies(DependenceKind Flavor,
-                      const Value *Arg,
-                      BasicBlock *StartBB, Instruction *StartInst,
-                      SmallPtrSetImpl<Instruction *> &DependingInstructions,
-                      ProvenanceAnalysis &PA);
+/// Find dependent instructions. If there is exactly one dependent instruction,
+/// return it. Otherwise, return null.
+llvm::Instruction *findSingleDependency(DependenceKind Flavor, const Value *Arg,
+                                        BasicBlock *StartBB,
+                                        Instruction *StartInst,
+                                        ProvenanceAnalysis &PA);
 
 bool
 Depends(DependenceKind Flavor, Instruction *Inst, const Value *Arg,
diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
@@ -161,20 +161,11 @@ bool ObjCARCContract::contractAutorelease(Function &F, Instruction *Autorelease,
 
   // Check that there are no instructions between the retain and the autorelease
   // (such as an autorelease_pop) which may change the count.
-  CallInst *Retain = nullptr;
-  SmallPtrSet<Instruction *, 4> DependingInstructions;
-
-  if (Class == ARCInstKind::AutoreleaseRV)
-    FindDependencies(RetainAutoreleaseRVDep, Arg, Autorelease->getParent(),
-                     Autorelease, DependingInstructions, PA);
-  else
-    FindDependencies(RetainAutoreleaseDep, Arg, Autorelease->getParent(),
-                     Autorelease, DependingInstructions, PA);
-
-  if (DependingInstructions.size() != 1)
-    return false;
-
-  Retain = dyn_cast_or_null<CallInst>(*DependingInstructions.begin());
+  DependenceKind DK = Class == ARCInstKind::AutoreleaseRV
+                          ? RetainAutoreleaseRVDep
+                          : RetainAutoreleaseDep;
+  auto *Retain = dyn_cast_or_null<CallInst>(
+      findSingleDependency(DK, Arg, Autorelease->getParent(), Autorelease, PA));
 
   if (!Retain || GetBasicARCInstKind(Retain) != ARCInstKind::Retain ||
       GetArgRCIdentityRoot(Retain) != Arg)
diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
@@ -1125,7 +1125,7 @@ void ObjCARCOpt::OptimizeIndividualCallImpl(
     if (!HasNull)
       continue;
 
-    SmallPtrSet<Instruction *, 4> DependingInstructions;
+    Instruction *DepInst = nullptr;
 
     // Check that there is nothing that cares about the reference
     // count between the call and the phi.
@@ -1137,13 +1137,13 @@ void ObjCARCOpt::OptimizeIndividualCallImpl(
     case ARCInstKind::Release:
       // These can't be moved across things that care about the retain
       // count.
-      FindDependencies(NeedsPositiveRetainCount, Arg, Inst->getParent(), Inst,
-                       DependingInstructions, PA);
+      DepInst = findSingleDependency(NeedsPositiveRetainCount, Arg,
+                                     Inst->getParent(), Inst, PA);
       break;
     case ARCInstKind::Autorelease:
       // These can't be moved across autorelease pool scope boundaries.
-      FindDependencies(AutoreleasePoolBoundary, Arg, Inst->getParent(), Inst,
-                       DependingInstructions, PA);
+      DepInst = findSingleDependency(AutoreleasePoolBoundary, Arg,
+                                     Inst->getParent(), Inst, PA);
       break;
     case ARCInstKind::ClaimRV:
     case ARCInstKind::RetainRV:
@@ -1157,9 +1157,7 @@ void ObjCARCOpt::OptimizeIndividualCallImpl(
       llvm_unreachable("Invalid dependence flavor");
     }
 
-    if (DependingInstructions.size() != 1)
-      continue;
-    if (*DependingInstructions.begin() != PN)
+    if (DepInst != PN)
       continue;
 
     Changed = true;
@@ -2233,24 +2231,21 @@ bool ObjCARCOpt::OptimizeSequences(Function &F) {
 /// Check if there is a dependent call earlier that does not have anything in
 /// between the Retain and the call that can affect the reference count of their
 /// shared pointer argument. Note that Retain need not be in BB.
-static bool
-HasSafePathToPredecessorCall(const Value *Arg, Instruction *Retain,
-                             SmallPtrSetImpl<Instruction *> &DepInsts,
-                             ProvenanceAnalysis &PA) {
-  FindDependencies(CanChangeRetainCount, Arg, Retain->getParent(), Retain,
-                   DepInsts, PA);
-  if (DepInsts.size() != 1)
-    return false;
-
-  auto *Call = dyn_cast_or_null<CallInst>(*DepInsts.begin());
+static CallInst *HasSafePathToPredecessorCall(const Value *Arg,
+                                              Instruction *Retain,
+                                              ProvenanceAnalysis &PA) {
+  auto *Call = dyn_cast_or_null<CallInst>(findSingleDependency(
+      CanChangeRetainCount, Arg, Retain->getParent(), Retain, PA));
 
   // Check that the pointer is the return value of the call.
   if (!Call || Arg != Call)
-    return false;
+    return nullptr;
 
   // Check that the call is a regular call.
   ARCInstKind Class = GetBasicARCInstKind(Call);
-  return Class == ARCInstKind::CallOrUser || Class == ARCInstKind::Call;
+  return Class == ARCInstKind::CallOrUser || Class == ARCInstKind::Call
+             ? Call
+             : nullptr;
 }
 
 /// Find a dependent retain that precedes the given autorelease for which there
@@ -2260,12 +2255,8 @@ static CallInst *
 FindPredecessorRetainWithSafePath(const Value *Arg, BasicBlock *BB,
                                   Instruction *Autorelease,
                                   ProvenanceAnalysis &PA) {
-  SmallPtrSet<Instruction *, 4> DepInsts;
-  FindDependencies(CanChangeRetainCount, Arg, BB, Autorelease, DepInsts, PA);
-  if (DepInsts.size() != 1)
-    return nullptr;
-
-  auto *Retain = dyn_cast_or_null<CallInst>(*DepInsts.begin());
+  auto *Retain = dyn_cast_or_null<CallInst>(
+      findSingleDependency(CanChangeRetainCount, Arg, BB, Autorelease, PA));
 
   // Check that we found a retain with the same argument.
   if (!Retain || !IsRetain(GetBasicARCInstKind(Retain)) ||
@@ -2284,11 +2275,9 @@ FindPredecessorAutoreleaseWithSafePath(const Value *Arg, BasicBlock *BB,
                                        ReturnInst *Ret,
                                        ProvenanceAnalysis &PA) {
   SmallPtrSet<Instruction *, 4> DepInsts;
-  FindDependencies(NeedsPositiveRetainCount, Arg, BB, Ret, DepInsts, PA);
-  if (DepInsts.size() != 1)
-    return nullptr;
+  auto *Autorelease = dyn_cast_or_null<CallInst>(
+      findSingleDependency(NeedsPositiveRetainCount, Arg, BB, Ret, PA));
 
-  auto *Autorelease = dyn_cast_or_null<CallInst>(*DepInsts.begin());
   if (!Autorelease)
     return nullptr;
   ARCInstKind AutoreleaseClass = GetBasicARCInstKind(Autorelease);
@@ -2314,7 +2303,6 @@ void ObjCARCOpt::OptimizeReturns(Function &F) {
 
   LLVM_DEBUG(dbgs() << "\n== ObjCARCOpt::OptimizeReturns ==\n");
 
-  SmallPtrSet<Instruction *, 4> DependingInstructions;
   for (BasicBlock &BB: F) {
     ReturnInst *Ret = dyn_cast<ReturnInst>(&BB.back());
     if (!Ret)
@@ -2341,21 +2329,13 @@ void ObjCARCOpt::OptimizeReturns(Function &F) {
 
     // Check that there is nothing that can affect the reference count
     // between the retain and the call.  Note that Retain need not be in BB.
-    bool HasSafePathToCall =
-        HasSafePathToPredecessorCall(Arg, Retain, DependingInstructions, PA);
+    CallInst *Call = HasSafePathToPredecessorCall(Arg, Retain, PA);
 
     // Don't remove retainRV/autoreleaseRV pairs if the call isn't a tail call.
-    if (HasSafePathToCall &&
-        GetBasicARCInstKind(Retain) == ARCInstKind::RetainRV &&
-        GetBasicARCInstKind(Autorelease) == ARCInstKind::AutoreleaseRV &&
-        !cast<CallInst>(*DependingInstructions.begin())->isTailCall()) {
-      DependingInstructions.clear();
-      continue;
-    }
-
-    DependingInstructions.clear();
-
-    if (!HasSafePathToCall)
+    if (!Call ||
+        (!Call->isTailCall() &&
+         GetBasicARCInstKind(Retain) == ARCInstKind::RetainRV &&
+         GetBasicARCInstKind(Autorelease) == ARCInstKind::AutoreleaseRV))
       continue;
 
     // If so, we can zap the retain and autorelease.
