From: Akira Hatanaka Date: Fri, 13 Nov 2020 18:40:40 +0000 (-0800) Subject: [ObjC][ARC] Add and use a function which finds and returns the single X-Git-Tag: llvmorg-13-init~6108 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2ed3a76745f9be48b75e12c71a9b349fde9f0108;p=platform%2Fupstream%2Fllvm.git [ObjC][ARC] Add and use a function which finds and returns the single 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 index 1d90c3f..6a5e48b 100644 --- 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 &DependingInsts, - ProvenanceAnalysis &PA) { +static bool findDependencies(DependenceKind Flavor, const Value *Arg, + BasicBlock *StartBB, Instruction *StartInst, + SmallPtrSetImpl &DependingInsts, + ProvenanceAnalysis &PA) { BasicBlock::iterator StartPos = StartInst->getIterator(); SmallPtrSet 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(-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 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 index 25ac93f..cf4c05e 100644 --- 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 &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 index a5367cd..3904f4d 100644 --- 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 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(*DependingInstructions.begin()); + DependenceKind DK = Class == ARCInstKind::AutoreleaseRV + ? RetainAutoreleaseRVDep + : RetainAutoreleaseDep; + auto *Retain = dyn_cast_or_null( + 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 index ae197e6..d1cab08 100644 --- 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 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 &DepInsts, - ProvenanceAnalysis &PA) { - FindDependencies(CanChangeRetainCount, Arg, Retain->getParent(), Retain, - DepInsts, PA); - if (DepInsts.size() != 1) - return false; - - auto *Call = dyn_cast_or_null(*DepInsts.begin()); +static CallInst *HasSafePathToPredecessorCall(const Value *Arg, + Instruction *Retain, + ProvenanceAnalysis &PA) { + auto *Call = dyn_cast_or_null(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 DepInsts; - FindDependencies(CanChangeRetainCount, Arg, BB, Autorelease, DepInsts, PA); - if (DepInsts.size() != 1) - return nullptr; - - auto *Retain = dyn_cast_or_null(*DepInsts.begin()); + auto *Retain = dyn_cast_or_null( + 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 DepInsts; - FindDependencies(NeedsPositiveRetainCount, Arg, BB, Ret, DepInsts, PA); - if (DepInsts.size() != 1) - return nullptr; + auto *Autorelease = dyn_cast_or_null( + findSingleDependency(NeedsPositiveRetainCount, Arg, BB, Ret, PA)); - auto *Autorelease = dyn_cast_or_null(*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 DependingInstructions; for (BasicBlock &BB: F) { ReturnInst *Ret = dyn_cast(&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(*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.