From 3d8f842712d49b0767832b6e3f65df2d3f19af4e Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sun, 11 Oct 2020 21:56:09 +0200 Subject: [PATCH] [LICM] Make promotion faster Even when MemorySSA-based LICM is used, an AST is still populated for scalar promotion. As the AST has quadratic complexity, a lot of time is spent in this step despite the existing access count limit. This patch optimizes the identification of promotable stores. The idea here is pretty simple: We're only interested in must-alias mod sets of loop invariant pointers. As such, only populate the AST with loop-invariant loads and stores (anything else is definitely not promotable) and then discard any sets which alias with any of the remaining, definitely non-promotable accesses. If we promoted something, check whether this has made some other accesses loop invariant and thus possible promotion candidates. This is much faster in practice, because we need to perform AA queries for O(NumPromotable^2 + NumPromotable*NumNonPromotable) instead of O(NumTotal^2), and NumPromotable tends to be small. Additionally, promotable accesses have loop invariant pointers, for which AA is cheaper. This has a signicant positive compile-time impact. We save ~1.8% geomean on CTMark at O3, with 6% on lencod in particular and 25% on individual files. Conceptually, this change is NFC, but may not be so in practice, because the AST is only an approximation, and can produce different results depending on the order in which accesses are added. However, there is at least no impact on the number of promotions (licm.NumPromoted) in test-suite O3 configuration with this change. Differential Revision: https://reviews.llvm.org/D89264 --- llvm/include/llvm/Analysis/AliasSetTracker.h | 6 +- llvm/lib/Analysis/AliasSetTracker.cpp | 10 -- llvm/lib/Transforms/Scalar/LICM.cpp | 156 ++++++++++++++----- 3 files changed, 120 insertions(+), 52 deletions(-) diff --git a/llvm/include/llvm/Analysis/AliasSetTracker.h b/llvm/include/llvm/Analysis/AliasSetTracker.h index b27fd5aa92a7f8..b8e0559db1f877 100644 --- a/llvm/include/llvm/Analysis/AliasSetTracker.h +++ b/llvm/include/llvm/Analysis/AliasSetTracker.h @@ -20,6 +20,7 @@ #include "llvm/ADT/DenseMapInfo.h" #include "llvm/ADT/ilist.h" #include "llvm/ADT/ilist_node.h" +#include "llvm/Analysis/AliasAnalysis.h" #include "llvm/Analysis/MemoryLocation.h" #include "llvm/IR/Instruction.h" #include "llvm/IR/Metadata.h" @@ -39,7 +40,6 @@ class AliasSetTracker; class BasicBlock; class LoadInst; class Loop; -class MemorySSA; class AnyMemSetInst; class AnyMemTransferInst; class raw_ostream; @@ -343,7 +343,6 @@ class AliasSetTracker { struct ASTCallbackVHDenseMapInfo : public DenseMapInfo {}; AAResults &AA; - MemorySSA *MSSA = nullptr; Loop *L = nullptr; ilist AliasSets; @@ -357,8 +356,6 @@ class AliasSetTracker { /// Create an empty collection of AliasSets, and use the specified alias /// analysis object to disambiguate load and store addresses. explicit AliasSetTracker(AAResults &AA) : AA(AA) {} - explicit AliasSetTracker(AAResults &AA, MemorySSA *MSSA, Loop *L) - : AA(AA), MSSA(MSSA), L(L) {} ~AliasSetTracker() { clear(); } /// These methods are used to add different types of instructions to the alias @@ -383,7 +380,6 @@ class AliasSetTracker { void add(BasicBlock &BB); // Add all instructions in basic block void add(const AliasSetTracker &AST); // Add alias relations from another AST void addUnknown(Instruction *I); - void addAllInstructionsInLoopUsingMSSA(); void clear(); diff --git a/llvm/lib/Analysis/AliasSetTracker.cpp b/llvm/lib/Analysis/AliasSetTracker.cpp index 3e923c220486cd..e43416deb6433a 100644 --- a/llvm/lib/Analysis/AliasSetTracker.cpp +++ b/llvm/lib/Analysis/AliasSetTracker.cpp @@ -14,7 +14,6 @@ #include "llvm/Analysis/GuardUtils.h" #include "llvm/Analysis/LoopInfo.h" #include "llvm/Analysis/MemoryLocation.h" -#include "llvm/Analysis/MemorySSA.h" #include "llvm/Config/llvm-config.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DataLayout.h" @@ -536,15 +535,6 @@ void AliasSetTracker::add(const AliasSetTracker &AST) { } } -void AliasSetTracker::addAllInstructionsInLoopUsingMSSA() { - assert(MSSA && L && "MSSA and L must be available"); - for (const BasicBlock *BB : L->blocks()) - if (auto *Accesses = MSSA->getBlockAccesses(BB)) - for (auto &Access : *Accesses) - if (auto *MUD = dyn_cast(&Access)) - add(MUD->getMemoryInst()); -} - // deleteValue method - This method is used to remove a pointer value from the // AliasSetTracker entirely. It should be used when an instruction is deleted // from the program to update the AST. If you don't use this, you would have diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp index d82142f6d77f48..47cf4a76f89f88 100644 --- a/llvm/lib/Transforms/Scalar/LICM.cpp +++ b/llvm/lib/Transforms/Scalar/LICM.cpp @@ -185,6 +185,12 @@ static void moveInstructionBefore(Instruction &I, Instruction &Dest, ICFLoopSafetyInfo &SafetyInfo, MemorySSAUpdater *MSSAU, ScalarEvolution *SE); +static void foreachMemoryAccess(MemorySSA *MSSA, Loop *L, + function_ref Fn); +static SmallVector, 0> +collectPromotionCandidates(MemorySSA *MSSA, AliasAnalysis *AA, Loop *L, + SmallVectorImpl &MaybePromotable); + namespace { struct LoopInvariantCodeMotion { bool runOnLoop(Loop *L, AAResults *AA, LoopInfo *LI, DominatorTree *DT, @@ -203,9 +209,6 @@ struct LoopInvariantCodeMotion { std::unique_ptr collectAliasInfoForLoop(Loop *L, LoopInfo *LI, AAResults *AA); - std::unique_ptr - collectAliasInfoForLoopWithMSSA(Loop *L, AAResults *AA, - MemorySSAUpdater *MSSAU); }; struct LegacyLICMPass : public LoopPass { @@ -430,31 +433,48 @@ bool LoopInvariantCodeMotion::runOnLoop( PredIteratorCache PIC; bool Promoted = false; - - // Build an AST using MSSA. - if (!CurAST.get()) - CurAST = collectAliasInfoForLoopWithMSSA(L, AA, MSSAU.get()); - - // Loop over all of the alias sets in the tracker object. - for (AliasSet &AS : *CurAST) { - // We can promote this alias set if it has a store, if it is a "Must" - // alias set, if the pointer is loop invariant, and if we are not - // eliminating any volatile loads or stores. - if (AS.isForwardingAliasSet() || !AS.isMod() || !AS.isMustAlias() || - !L->isLoopInvariant(AS.begin()->getValue())) - continue; - - assert( - !AS.empty() && - "Must alias set should have at least one pointer element in it!"); - - SmallSetVector PointerMustAliases; - for (const auto &ASI : AS) - PointerMustAliases.insert(ASI.getValue()); - - Promoted |= promoteLoopAccessesToScalars( - PointerMustAliases, ExitBlocks, InsertPts, MSSAInsertPts, PIC, LI, - DT, TLI, L, CurAST.get(), MSSAU.get(), &SafetyInfo, ORE); + if (CurAST.get()) { + // Loop over all of the alias sets in the tracker object. + for (AliasSet &AS : *CurAST) { + // We can promote this alias set if it has a store, if it is a "Must" + // alias set, if the pointer is loop invariant, and if we are not + // eliminating any volatile loads or stores. + if (AS.isForwardingAliasSet() || !AS.isMod() || !AS.isMustAlias() || + !L->isLoopInvariant(AS.begin()->getValue())) + continue; + + assert( + !AS.empty() && + "Must alias set should have at least one pointer element in it!"); + + SmallSetVector PointerMustAliases; + for (const auto &ASI : AS) + PointerMustAliases.insert(ASI.getValue()); + + Promoted |= promoteLoopAccessesToScalars( + PointerMustAliases, ExitBlocks, InsertPts, MSSAInsertPts, PIC, LI, + DT, TLI, L, CurAST.get(), MSSAU.get(), &SafetyInfo, ORE); + } + } else { + SmallVector MaybePromotable; + foreachMemoryAccess(MSSA, L, [&](Instruction *I) { + MaybePromotable.push_back(I); + }); + + // Promoting one set of accesses may make the pointers for another set + // loop invariant, so run this in a loop (with the MaybePromotable set + // decreasing in size over time). + bool LocalPromoted; + do { + LocalPromoted = false; + for (const SmallSetVector &PointerMustAliases : + collectPromotionCandidates(MSSA, AA, L, MaybePromotable)) { + LocalPromoted |= promoteLoopAccessesToScalars( + PointerMustAliases, ExitBlocks, InsertPts, MSSAInsertPts, PIC, + LI, DT, TLI, L, /*AST*/nullptr, MSSAU.get(), &SafetyInfo, ORE); + } + Promoted |= LocalPromoted; + } while (LocalPromoted); } // Once we have promoted values across the loop body we have to @@ -2217,6 +2237,77 @@ bool llvm::promoteLoopAccessesToScalars( return true; } +static void foreachMemoryAccess(MemorySSA *MSSA, Loop *L, + function_ref Fn) { + for (const BasicBlock *BB : L->blocks()) + if (const auto *Accesses = MSSA->getBlockAccesses(BB)) + for (const auto &Access : *Accesses) + if (const auto *MUD = dyn_cast(&Access)) + Fn(MUD->getMemoryInst()); +} + +static SmallVector, 0> +collectPromotionCandidates(MemorySSA *MSSA, AliasAnalysis *AA, Loop *L, + SmallVectorImpl &MaybePromotable) { + AliasSetTracker AST(*AA); + + auto IsPotentiallyPromotable = [L](const Instruction *I) { + if (const auto *SI = dyn_cast(I)) + return L->isLoopInvariant(SI->getPointerOperand()); + if (const auto *LI = dyn_cast(I)) + return L->isLoopInvariant(LI->getPointerOperand()); + return false; + }; + + // Populate AST with potentially promotable accesses and remove them from + // MaybePromotable, so they will not be checked again on the next iteration. + SmallPtrSet AttemptingPromotion; + llvm::erase_if(MaybePromotable, [&](Instruction *I) { + if (IsPotentiallyPromotable(I)) { + AttemptingPromotion.insert(I); + AST.add(I); + return true; + } + return false; + }); + + // We're only interested in must-alias sets that contain a mod. + SmallVector Sets; + for (AliasSet &AS : AST) + if (!AS.isForwardingAliasSet() && AS.isMod() && AS.isMustAlias()) + Sets.push_back(&AS); + + if (Sets.empty()) + return {}; // Nothing to promote... + + // Discard any sets for which there is an aliasing non-promotable access. + foreachMemoryAccess(MSSA, L, [&](Instruction *I) { + if (AttemptingPromotion.contains(I)) + return; + + if (Optional Loc = MemoryLocation::getOrNone(I)) { + llvm::erase_if(Sets, [&](const AliasSet *AS) { + return AS->aliasesPointer(Loc->Ptr, Loc->Size, Loc->AATags, *AA) + != NoAlias; + }); + } else { + llvm::erase_if(Sets, [&](const AliasSet *AS) { + return AS->aliasesUnknownInst(I, *AA); + }); + } + }); + + SmallVector, 0> Result; + for (const AliasSet *Set : Sets) { + SmallSetVector PointerMustAliases; + for (const auto &ASI : *Set) + PointerMustAliases.insert(ASI.getValue()); + Result.push_back(std::move(PointerMustAliases)); + } + + return Result; +} + /// Returns an owning pointer to an alias set which incorporates aliasing info /// from L and all subloops of L. std::unique_ptr @@ -2237,15 +2328,6 @@ LoopInvariantCodeMotion::collectAliasInfoForLoop(Loop *L, LoopInfo *LI, return CurAST; } -std::unique_ptr -LoopInvariantCodeMotion::collectAliasInfoForLoopWithMSSA( - Loop *L, AAResults *AA, MemorySSAUpdater *MSSAU) { - auto *MSSA = MSSAU->getMemorySSA(); - auto CurAST = std::make_unique(*AA, MSSA, L); - CurAST->addAllInstructionsInLoopUsingMSSA(); - return CurAST; -} - static bool pointerInvalidatedByLoop(MemoryLocation MemLoc, AliasSetTracker *CurAST, Loop *CurLoop, AAResults *AA) {