diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp index c29cba6f675c5..66c63a367eaeb 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp @@ -244,11 +244,10 @@ class PointerReplacer { void replacePointer(Value *V); private: - bool collectUsersRecursive(Instruction &I); void replace(Instruction *I); Value *getReplacement(Value *I); bool isAvailable(Instruction *I) const { - return I == &Root || Worklist.contains(I); + return I == &Root || UsersToReplace.contains(I); } bool isEqualOrValidAddrSpaceCast(const Instruction *I, @@ -260,8 +259,7 @@ class PointerReplacer { return (FromAS == ToAS) || IC.isValidAddrSpaceCast(FromAS, ToAS); } - SmallPtrSet ValuesToRevisit; - SmallSetVector Worklist; + SmallSetVector UsersToReplace; MapVector WorkMap; InstCombinerImpl &IC; Instruction &Root; @@ -270,77 +268,88 @@ class PointerReplacer { } // end anonymous namespace bool PointerReplacer::collectUsers() { - if (!collectUsersRecursive(Root)) - return false; - - // Ensure that all outstanding (indirect) users of I - // are inserted into the Worklist. Return false - // otherwise. - return llvm::set_is_subset(ValuesToRevisit, Worklist); -} + SmallVector Worklist; + SmallSetVector ValuesToRevisit; + + auto PushUsersToWorklist = [&](Instruction *Inst) { + for (auto *U : Inst->users()) { + if (auto *I = dyn_cast(U)) { + if (!isAvailable(I) && !ValuesToRevisit.contains(I)) + Worklist.emplace_back(I); + } + } + }; -bool PointerReplacer::collectUsersRecursive(Instruction &I) { - for (auto *U : I.users()) { - auto *Inst = cast(&*U); + PushUsersToWorklist(&Root); + while (!Worklist.empty()) { + auto *Inst = Worklist.pop_back_val(); + if (!Inst) + return false; + if (isAvailable(Inst)) + continue; if (auto *Load = dyn_cast(Inst)) { if (Load->isVolatile()) return false; - Worklist.insert(Load); + UsersToReplace.insert(Load); } else if (auto *PHI = dyn_cast(Inst)) { // All incoming values must be instructions for replacability if (any_of(PHI->incoming_values(), [](Value *V) { return !isa(V); })) return false; - // If at least one incoming value of the PHI is not in Worklist, - // store the PHI for revisiting and skip this iteration of the - // loop. - if (any_of(PHI->incoming_values(), [this](Value *V) { - return !isAvailable(cast(V)); - })) { - ValuesToRevisit.insert(Inst); + // If all incoming values are available, mark this PHI as + // replacable and push it's users into the worklist. + if (all_of(PHI->incoming_values(), + [&](Value *V) { return isAvailable(cast(V)); })) { + UsersToReplace.insert(PHI); + PushUsersToWorklist(PHI); continue; } - Worklist.insert(PHI); - if (!collectUsersRecursive(*PHI)) - return false; - } else if (auto *SI = dyn_cast(Inst)) { - if (!isa(SI->getTrueValue()) || - !isa(SI->getFalseValue())) + // Not all incoming values are available. If this PHI was already + // visited prior to this iteration, return false. + if (!ValuesToRevisit.insert(PHI)) return false; - if (!isAvailable(cast(SI->getTrueValue())) || - !isAvailable(cast(SI->getFalseValue()))) { - ValuesToRevisit.insert(Inst); - continue; + // Push PHI back into the stack, followed by unavailable + // incoming values. + Worklist.emplace_back(PHI); + for (unsigned Idx = 0; Idx < PHI->getNumIncomingValues(); ++Idx) { + auto *IncomingValue = cast(PHI->getIncomingValue(Idx)); + if (UsersToReplace.contains(IncomingValue)) + continue; + if (!ValuesToRevisit.insert(IncomingValue)) + return false; + Worklist.emplace_back(IncomingValue); } - Worklist.insert(SI); - if (!collectUsersRecursive(*SI)) - return false; - } else if (isa(Inst)) { - Worklist.insert(Inst); - if (!collectUsersRecursive(*Inst)) + } else if (auto *SI = dyn_cast(Inst)) { + auto *TrueInst = dyn_cast(SI->getTrueValue()); + auto *FalseInst = dyn_cast(SI->getFalseValue()); + if (!TrueInst || !FalseInst) return false; + + UsersToReplace.insert(SI); + PushUsersToWorklist(SI); + } else if (auto *GEP = dyn_cast(Inst)) { + UsersToReplace.insert(GEP); + PushUsersToWorklist(GEP); } else if (auto *MI = dyn_cast(Inst)) { if (MI->isVolatile()) return false; - Worklist.insert(Inst); + UsersToReplace.insert(Inst); } else if (isEqualOrValidAddrSpaceCast(Inst, FromAS)) { - Worklist.insert(Inst); - if (!collectUsersRecursive(*Inst)) - return false; + UsersToReplace.insert(Inst); + PushUsersToWorklist(Inst); } else if (Inst->isLifetimeStartOrEnd()) { continue; } else { // TODO: For arbitrary uses with address space mismatches, should we check // if we can introduce a valid addrspacecast? - LLVM_DEBUG(dbgs() << "Cannot handle pointer user: " << *U << '\n'); + LLVM_DEBUG(dbgs() << "Cannot handle pointer user: " << *Inst << '\n'); return false; } } - - return true; + return llvm::set_is_subset(ValuesToRevisit, UsersToReplace); } Value *PointerReplacer::getReplacement(Value *V) { return WorkMap.lookup(V); } @@ -443,7 +452,7 @@ void PointerReplacer::replacePointer(Value *V) { #endif WorkMap[&Root] = V; - for (Instruction *Workitem : Worklist) + for (Instruction *Workitem : UsersToReplace) replace(Workitem); }