[llvm] 3d8f842 - [LICM] Make promotion faster
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 3 15:33:56 PST 2021
This is a really nice change to see. Code structure is clear, and the
algorithmic part is spot on. Thank you for doing this.
Philip
On 3/2/21 1:21 PM, Nikita Popov via llvm-commits wrote:
> Author: Nikita Popov
> Date: 2021-03-02T22:10:48+01:00
> New Revision: 3d8f842712d49b0767832b6e3f65df2d3f19af4e
>
> URL: https://github.com/llvm/llvm-project/commit/3d8f842712d49b0767832b6e3f65df2d3f19af4e
> DIFF: https://github.com/llvm/llvm-project/commit/3d8f842712d49b0767832b6e3f65df2d3f19af4e.diff
>
> LOG: [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
>
> Added:
>
>
> Modified:
> llvm/include/llvm/Analysis/AliasSetTracker.h
> llvm/lib/Analysis/AliasSetTracker.cpp
> llvm/lib/Transforms/Scalar/LICM.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/include/llvm/Analysis/AliasSetTracker.h b/llvm/include/llvm/Analysis/AliasSetTracker.h
> index b27fd5aa92a7..b8e0559db1f8 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<Value *> {};
>
> AAResults &AA;
> - MemorySSA *MSSA = nullptr;
> Loop *L = nullptr;
> ilist<AliasSet> 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
> diff erent 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 3e923c220486..e43416deb643 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<MemoryUseOrDef>(&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 d82142f6d77f..47cf4a76f89f 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<void(Instruction *)> Fn);
> +static SmallVector<SmallSetVector<Value *, 8>, 0>
> +collectPromotionCandidates(MemorySSA *MSSA, AliasAnalysis *AA, Loop *L,
> + SmallVectorImpl<Instruction *> &MaybePromotable);
> +
> namespace {
> struct LoopInvariantCodeMotion {
> bool runOnLoop(Loop *L, AAResults *AA, LoopInfo *LI, DominatorTree *DT,
> @@ -203,9 +209,6 @@ struct LoopInvariantCodeMotion {
>
> std::unique_ptr<AliasSetTracker>
> collectAliasInfoForLoop(Loop *L, LoopInfo *LI, AAResults *AA);
> - std::unique_ptr<AliasSetTracker>
> - 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<Value *, 8> 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<Value *, 8> 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<Instruction *, 16> 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<Value *, 8> &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<void(Instruction *)> 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<MemoryUseOrDef>(&Access))
> + Fn(MUD->getMemoryInst());
> +}
> +
> +static SmallVector<SmallSetVector<Value *, 8>, 0>
> +collectPromotionCandidates(MemorySSA *MSSA, AliasAnalysis *AA, Loop *L,
> + SmallVectorImpl<Instruction *> &MaybePromotable) {
> + AliasSetTracker AST(*AA);
> +
> + auto IsPotentiallyPromotable = [L](const Instruction *I) {
> + if (const auto *SI = dyn_cast<StoreInst>(I))
> + return L->isLoopInvariant(SI->getPointerOperand());
> + if (const auto *LI = dyn_cast<LoadInst>(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<Value *, 16> 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<const AliasSet *, 8> 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<MemoryLocation> 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<SmallSetVector<Value *, 8>, 0> Result;
> + for (const AliasSet *Set : Sets) {
> + SmallSetVector<Value *, 8> 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<AliasSetTracker>
> @@ -2237,15 +2328,6 @@ LoopInvariantCodeMotion::collectAliasInfoForLoop(Loop *L, LoopInfo *LI,
> return CurAST;
> }
>
> -std::unique_ptr<AliasSetTracker>
> -LoopInvariantCodeMotion::collectAliasInfoForLoopWithMSSA(
> - Loop *L, AAResults *AA, MemorySSAUpdater *MSSAU) {
> - auto *MSSA = MSSAU->getMemorySSA();
> - auto CurAST = std::make_unique<AliasSetTracker>(*AA, MSSA, L);
> - CurAST->addAllInstructionsInLoopUsingMSSA();
> - return CurAST;
> -}
> -
> static bool pointerInvalidatedByLoop(MemoryLocation MemLoc,
> AliasSetTracker *CurAST, Loop *CurLoop,
> AAResults *AA) {
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list