diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h index ffdf57cd3d8f8..25b042eebf664 100644 --- a/llvm/include/llvm/Transforms/Scalar/GVN.h +++ b/llvm/include/llvm/Transforms/Scalar/GVN.h @@ -137,10 +137,7 @@ class GVNPass : public PassInfoMixin { /// This removes the specified instruction from /// our various maps and marks it for deletion. - void markInstructionForDeletion(Instruction *I) { - VN.erase(I); - InstrsToErase.push_back(I); - } + void salvageAndRemoveInstruction(Instruction *I); DominatorTree &getDominatorTree() const { return *DT; } AAResults *getAliasAnalysis() const { return VN.getAliasAnalysis(); } @@ -306,7 +303,6 @@ class GVNPass : public PassInfoMixin { // propagate to any successors. Entries added mid-block are applied // to the remaining instructions in the block. SmallMapVector ReplaceOperandsWithMap; - SmallVector InstrsToErase; // Map the block to reversed postorder traversal number. It is used to // find back edge easily. diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp index c0ae944ee82f2..5d5ab9eca448e 100644 --- a/llvm/lib/Transforms/Scalar/GVN.cpp +++ b/llvm/lib/Transforms/Scalar/GVN.cpp @@ -876,6 +876,12 @@ void GVNPass::printPipeline( OS << '>'; } +void GVNPass::salvageAndRemoveInstruction(Instruction *I) { + salvageKnowledge(I, AC); + salvageDebugInfo(*I); + removeInstruction(I); +} + #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) LLVM_DUMP_METHOD void GVNPass::dump(DenseMap &Map) const { errs() << "{\n"; @@ -1555,7 +1561,6 @@ void GVNPass::eliminatePartiallyRedundantLoad( replaceValuesPerBlockEntry(ValuesPerBlock, OldLoad, NewLoad); if (uint32_t ValNo = VN.lookup(OldLoad, false)) LeaderTable.erase(ValNo, OldLoad, OldLoad->getParent()); - VN.erase(OldLoad); removeInstruction(OldLoad); } } @@ -1572,11 +1577,11 @@ void GVNPass::eliminatePartiallyRedundantLoad( I->setDebugLoc(Load->getDebugLoc()); if (V->getType()->isPtrOrPtrVectorTy()) MD->invalidateCachedPointerInfo(V); - markInstructionForDeletion(Load); ORE->emit([&]() { return OptimizationRemark(DEBUG_TYPE, "LoadPRE", Load) << "load eliminated by PRE"; }); + salvageAndRemoveInstruction(Load); } bool GVNPass::PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock, @@ -1795,7 +1800,7 @@ bool GVNPass::PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock, // Erase instructions generated by the failed PHI translation before // trying to number them. PHI translation might insert instructions // in basic blocks other than the current one, and we delete them - // directly, as markInstructionForDeletion only allows removing from the + // directly, as salvageAndRemoveInstruction only allows removing from the // current basic block. NewInsts.pop_back_val()->eraseFromParent(); } @@ -1994,9 +1999,9 @@ bool GVNPass::processNonLocalLoad(LoadInst *Load) { I->setDebugLoc(Load->getDebugLoc()); if (V->getType()->isPtrOrPtrVectorTy()) MD->invalidateCachedPointerInfo(V); - markInstructionForDeletion(Load); ++NumGVNLoad; reportLoadElim(Load, V, ORE); + salvageAndRemoveInstruction(Load); return true; } @@ -2064,7 +2069,7 @@ bool GVNPass::processAssumeIntrinsic(AssumeInst *IntrinsicI) { } } if (isAssumeWithEmptyBundle(*IntrinsicI)) { - markInstructionForDeletion(IntrinsicI); + salvageAndRemoveInstruction(IntrinsicI); return true; } return false; @@ -2175,7 +2180,7 @@ bool GVNPass::processLoad(LoadInst *L) { return false; if (L->use_empty()) { - markInstructionForDeletion(L); + salvageAndRemoveInstruction(L); return true; } @@ -2205,11 +2210,11 @@ bool GVNPass::processLoad(LoadInst *L) { // MaterializeAdjustedValue is responsible for combining metadata. ICF->removeUsersOf(L); L->replaceAllUsesWith(AvailableValue); - markInstructionForDeletion(L); if (MSSAU) MSSAU->removeMemoryAccess(L); ++NumGVNLoad; reportLoadElim(L, AvailableValue, ORE); + salvageAndRemoveInstruction(L); // Tell MDA to reexamine the reused pointer since we might have more // information after forwarding it. if (MD && AvailableValue->getType()->isPtrOrPtrVectorTy()) @@ -2601,7 +2606,7 @@ bool GVNPass::processInstruction(Instruction *I) { Changed = true; } if (isInstructionTriviallyDead(I, TLI)) { - markInstructionForDeletion(I); + salvageAndRemoveInstruction(I); Changed = true; } if (Changed) { @@ -2718,7 +2723,7 @@ bool GVNPass::processInstruction(Instruction *I) { patchAndReplaceAllUsesWith(I, Repl); if (MD && Repl->getType()->isPtrOrPtrVectorTy()) MD->invalidateCachedPointerInfo(Repl); - markInstructionForDeletion(I); + salvageAndRemoveInstruction(I); return true; } @@ -2794,10 +2799,6 @@ bool GVNPass::runImpl(Function &F, AssumptionCache &RunAC, DominatorTree &RunDT, } bool GVNPass::processBlock(BasicBlock *BB) { - // FIXME: Kill off InstrsToErase by doing erasing eagerly in a helper function - // (and incrementing BI before processing an instruction). - assert(InstrsToErase.empty() && - "We expect InstrsToErase to be empty across iterations"); if (DeadBlocks.count(BB)) return false; @@ -2812,44 +2813,13 @@ bool GVNPass::processBlock(BasicBlock *BB) { SmallPtrSet PHINodesToRemove; ChangedFunction |= EliminateDuplicatePHINodes(BB, PHINodesToRemove); for (PHINode *PN : PHINodesToRemove) { - VN.erase(PN); removeInstruction(PN); } - - for (BasicBlock::iterator BI = BB->begin(), BE = BB->end(); - BI != BE;) { + for (Instruction &Inst : make_early_inc_range(*BB)) { if (!ReplaceOperandsWithMap.empty()) - ChangedFunction |= replaceOperandsForInBlockEquality(&*BI); - ChangedFunction |= processInstruction(&*BI); - - if (InstrsToErase.empty()) { - ++BI; - continue; - } - - // If we need some instructions deleted, do it now. - NumGVNInstr += InstrsToErase.size(); - - // Avoid iterator invalidation. - bool AtStart = BI == BB->begin(); - if (!AtStart) - --BI; - - for (auto *I : InstrsToErase) { - assert(I->getParent() == BB && "Removing instruction from wrong block?"); - LLVM_DEBUG(dbgs() << "GVN removed: " << *I << '\n'); - salvageKnowledge(I, AC); - salvageDebugInfo(*I); - removeInstruction(I); - } - InstrsToErase.clear(); - - if (AtStart) - BI = BB->begin(); - else - ++BI; + ChangedFunction |= replaceOperandsForInBlockEquality(&Inst); + ChangedFunction |= processInstruction(&Inst); } - return ChangedFunction; } @@ -3055,7 +3025,6 @@ bool GVNPass::performScalarPRE(Instruction *CurInst) { CurInst->replaceAllUsesWith(Phi); if (MD && Phi->getType()->isPtrOrPtrVectorTy()) MD->invalidateCachedPointerInfo(Phi); - VN.erase(CurInst); LeaderTable.erase(ValNo, CurInst, CurrentBlock); LLVM_DEBUG(dbgs() << "GVN PRE removed: " << *CurInst << '\n'); @@ -3155,6 +3124,7 @@ void GVNPass::cleanupGlobalSets() { } void GVNPass::removeInstruction(Instruction *I) { + VN.erase(I); if (MD) MD->removeInstruction(I); if (MSSAU) MSSAU->removeMemoryAccess(I);