[LoopUnroll] Add CSE to remove redundant loads after unrolling. #83860
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch adds loadCSE support to simplifyLoopAfterUnroll. It is based on EarlyCSE's implementation using ScopeHashTable and is using SCEV for accessed pointers to check to find redundant loads after unrolling. This applies to the late unroll pass only, for full unrolling those redundant loads will be cleaned up by the regular pipeline. If we agree to move forward with this approach, there may be potential to share some logic with EarlyCSE and to split off the MemorySSA changes separately. The current approach constructs MSSA on-dmeand per-loop, but there is still small but notable compile-time impact: stage1-O3 +0.04% stage1-ReleaseThinLTO +0.06% stage1-ReleaseLTO-g +0.05% stage1-O0-g +0.02% stage2-O3 +0.09% stage2-O0-g +0.04% stage2-clang +0.02% https://llvm-compile-time-tracker.com/compare.php?from=c089fa5a729e217d0c0d4647656386dac1a1b135&to=ec7c0f27cb5c12b600d9adfc8543d131765ec7be&stat=instructions:u This benefits some workloads with runtime-unrolling disabled, where users use pragmas to force unrolling, as well as with runtime unrolling enabled. On SPEC/MultiSource, this removes a number of loads after unrolling on AArch64 with runtime unrolling enabled. External/S...te/526.blender_r/526.blender_r 96 MultiSourc...rks/mediabench/gsm/toast/toast 39 SingleSource/Benchmarks/Misc/ffbench 4 External/SPEC/CINT2006/403.gcc/403.gcc 18 MultiSourc.../Applications/JM/ldecod/ldecod 4 MultiSourc.../mediabench/jpeg/jpeg-6a/cjpeg 6 MultiSourc...OE-ProxyApps-C/miniGMG/miniGMG 9 MultiSourc...e/Applications/ClamAV/clamscan 4 MultiSourc.../MallocBench/espresso/espresso 3 MultiSourc...dence-flt/LinearDependence-flt 2 MultiSourc...ch/office-ispell/office-ispell 4 MultiSourc...ch/consumer-jpeg/consumer-jpeg 6 MultiSourc...ench/security-sha/security-sha 11 MultiSourc...chmarks/McCat/04-bisect/bisect 3 SingleSour...tTests/2020-01-06-coverage-009 12 MultiSourc...ench/telecomm-gsm/telecomm-gsm 39 MultiSourc...lds-flt/CrossingThresholds-flt 24 MultiSourc...dence-dbl/LinearDependence-dbl 2 External/S...C/CINT2006/445.gobmk/445.gobmk 6 MultiSourc...enchmarks/mafft/pairlocalalign 53 External/S...31.deepsjeng_r/531.deepsjeng_r 3 External/S...rate/510.parest_r/510.parest_r 58 External/S...NT2006/464.h264ref/464.h264ref 29 External/S...NT2017rate/502.gcc_r/502.gcc_r 45 External/S...C/CINT2006/456.hmmer/456.hmmer 6 External/S...te/538.imagick_r/538.imagick_r 18 External/S.../CFP2006/447.dealII/447.dealII 4 MultiSourc...OE-ProxyApps-C++/miniFE/miniFE 12 External/S...2017rate/525.x264_r/525.x264_r 36 MultiSourc...Benchmarks/7zip/7zip-benchmark 33 MultiSourc...hmarks/ASC_Sequoia/AMGmk/AMGmk 2 MultiSourc...chmarks/VersaBench/8b10b/8b10b 1 MultiSourc.../Applications/JM/lencod/lencod 116 MultiSourc...lds-dbl/CrossingThresholds-dbl 24 MultiSource/Benchmarks/McCat/05-eks/eks 15
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-analysis Author: Florian Hahn (fhahn) ChangesThis patch adds loadCSE support to simplifyLoopAfterUnroll. It is based This applies to the late unroll pass only, for full unrolling those If we agree to move forward with this approach, there may be potential The current approach constructs MSSA on-dmeand per-loop, but there is stage1-O3 +0.04% This benefits some workloads with runtime-unrolling disabled, On SPEC/MultiSource, this removes a number of loads after unrolling Patch is 54.09 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83860.diff 7 Files Affected:
diff --git a/llvm/include/llvm/Analysis/MemorySSA.h b/llvm/include/llvm/Analysis/MemorySSA.h
index caf0e31fd37d6c..2ca5c281166cad 100644
--- a/llvm/include/llvm/Analysis/MemorySSA.h
+++ b/llvm/include/llvm/Analysis/MemorySSA.h
@@ -110,6 +110,7 @@ namespace llvm {
template <class GraphType> struct GraphTraits;
class BasicBlock;
class Function;
+class Loop;
class Instruction;
class LLVMContext;
class MemoryAccess;
@@ -700,6 +701,7 @@ DEFINE_TRANSPARENT_OPERAND_ACCESSORS(MemoryPhi, MemoryAccess)
class MemorySSA {
public:
MemorySSA(Function &, AliasAnalysis *, DominatorTree *);
+ MemorySSA(Loop &, AliasAnalysis *, DominatorTree *);
// MemorySSA must remain where it's constructed; Walkers it creates store
// pointers to it.
@@ -800,10 +802,11 @@ class MemorySSA {
// Used by Memory SSA dumpers and wrapper pass
friend class MemorySSAUpdater;
+ template <typename IterT>
void verifyOrderingDominationAndDefUses(
- Function &F, VerificationLevel = VerificationLevel::Fast) const;
- void verifyDominationNumbers(const Function &F) const;
- void verifyPrevDefInPhis(Function &F) const;
+ IterT Blocks, VerificationLevel = VerificationLevel::Fast) const;
+ template <typename IterT> void verifyDominationNumbers(IterT Blocks) const;
+ template <typename IterT> void verifyPrevDefInPhis(IterT Blocks) const;
// This is used by the use optimizer and updater.
AccessList *getWritableBlockAccesses(const BasicBlock *BB) const {
@@ -847,7 +850,8 @@ class MemorySSA {
class OptimizeUses;
CachingWalker *getWalkerImpl();
- void buildMemorySSA(BatchAAResults &BAA);
+ template <typename IterT>
+ void buildMemorySSA(BatchAAResults &BAA, IterT Blocks);
void prepareForMoveTo(MemoryAccess *, BasicBlock *);
void verifyUseInDefs(MemoryAccess *, MemoryAccess *) const;
@@ -871,7 +875,8 @@ class MemorySSA {
void renumberBlock(const BasicBlock *) const;
AliasAnalysis *AA = nullptr;
DominatorTree *DT;
- Function &F;
+ Function *F = nullptr;
+ Loop *L = nullptr;
// Memory SSA mappings
DenseMap<const Value *, MemoryAccess *> ValueToMemoryAccess;
diff --git a/llvm/include/llvm/Transforms/Utils/UnrollLoop.h b/llvm/include/llvm/Transforms/Utils/UnrollLoop.h
index e8b03f81b34830..bd804dc1126624 100644
--- a/llvm/include/llvm/Transforms/Utils/UnrollLoop.h
+++ b/llvm/include/llvm/Transforms/Utils/UnrollLoop.h
@@ -22,6 +22,7 @@
namespace llvm {
class AssumptionCache;
+class AAResults;
class BasicBlock;
class BlockFrequencyInfo;
class DependenceInfo;
@@ -79,7 +80,8 @@ LoopUnrollResult UnrollLoop(Loop *L, UnrollLoopOptions ULO, LoopInfo *LI,
AssumptionCache *AC,
const llvm::TargetTransformInfo *TTI,
OptimizationRemarkEmitter *ORE, bool PreserveLCSSA,
- Loop **RemainderLoop = nullptr);
+ Loop **RemainderLoop = nullptr,
+ AAResults *AA = nullptr);
bool UnrollRuntimeLoopRemainder(
Loop *L, unsigned Count, bool AllowExpensiveTripCount,
@@ -102,7 +104,8 @@ bool isSafeToUnrollAndJam(Loop *L, ScalarEvolution &SE, DominatorTree &DT,
void simplifyLoopAfterUnroll(Loop *L, bool SimplifyIVs, LoopInfo *LI,
ScalarEvolution *SE, DominatorTree *DT,
AssumptionCache *AC,
- const TargetTransformInfo *TTI);
+ const TargetTransformInfo *TTI,
+ AAResults *AA = nullptr);
MDNode *GetUnrollMetadata(MDNode *LoopID, StringRef Name);
diff --git a/llvm/lib/Analysis/MemorySSA.cpp b/llvm/lib/Analysis/MemorySSA.cpp
index 82a6c470650cc9..d88eaceca1a2e7 100644
--- a/llvm/lib/Analysis/MemorySSA.cpp
+++ b/llvm/lib/Analysis/MemorySSA.cpp
@@ -25,6 +25,7 @@
#include "llvm/Analysis/AliasAnalysis.h"
#include "llvm/Analysis/CFGPrinter.h"
#include "llvm/Analysis/IteratedDominanceFrontier.h"
+#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/MemoryLocation.h"
#include "llvm/Config/llvm-config.h"
#include "llvm/IR/AssemblyAnnotationWriter.h"
@@ -1228,7 +1229,7 @@ void MemorySSA::markUnreachableAsLiveOnEntry(BasicBlock *BB) {
}
MemorySSA::MemorySSA(Function &Func, AliasAnalysis *AA, DominatorTree *DT)
- : DT(DT), F(Func), LiveOnEntryDef(nullptr), Walker(nullptr),
+ : DT(DT), F(&Func), LiveOnEntryDef(nullptr), Walker(nullptr),
SkipWalker(nullptr) {
// Build MemorySSA using a batch alias analysis. This reuses the internal
// state that AA collects during an alias()/getModRefInfo() call. This is
@@ -1237,7 +1238,28 @@ MemorySSA::MemorySSA(Function &Func, AliasAnalysis *AA, DominatorTree *DT)
// make queries about all the instructions in the Function.
assert(AA && "No alias analysis?");
BatchAAResults BatchAA(*AA);
- buildMemorySSA(BatchAA);
+ buildMemorySSA(BatchAA, iterator_range(F->begin(), F->end()));
+ // Intentionally leave AA to nullptr while building so we don't accidently
+ // use non-batch AliasAnalysis.
+ this->AA = AA;
+ // Also create the walker here.
+ getWalker();
+}
+
+MemorySSA::MemorySSA(Loop &L, AliasAnalysis *AA, DominatorTree *DT)
+ : DT(DT), L(&L), LiveOnEntryDef(nullptr), Walker(nullptr),
+ SkipWalker(nullptr) {
+ // Build MemorySSA using a batch alias analysis. This reuses the internal
+ // state that AA collects during an alias()/getModRefInfo() call. This is
+ // safe because there are no CFG changes while building MemorySSA and can
+ // significantly reduce the time spent by the compiler in AA, because we will
+ // make queries about all the instructions in the Function.
+ assert(AA && "No alias analysis?");
+ BatchAAResults BatchAA(*AA);
+ buildMemorySSA(
+ BatchAA, map_range(L.blocks(), [](const BasicBlock *BB) -> BasicBlock & {
+ return *const_cast<BasicBlock *>(BB);
+ }));
// Intentionally leave AA to nullptr while building so we don't accidently
// use non-batch AliasAnalysis.
this->AA = AA;
@@ -1491,16 +1513,17 @@ void MemorySSA::placePHINodes(
createMemoryPhi(BB);
}
-void MemorySSA::buildMemorySSA(BatchAAResults &BAA) {
+template <typename IterT>
+void MemorySSA::buildMemorySSA(BatchAAResults &BAA, IterT Blocks) {
// We create an access to represent "live on entry", for things like
// arguments or users of globals, where the memory they use is defined before
// the beginning of the function. We do not actually insert it into the IR.
// We do not define a live on exit for the immediate uses, and thus our
// semantics do *not* imply that something with no immediate uses can simply
// be removed.
- BasicBlock &StartingPoint = F.getEntryBlock();
- LiveOnEntryDef.reset(new MemoryDef(F.getContext(), nullptr, nullptr,
- &StartingPoint, NextID++));
+ BasicBlock &StartingPoint = *Blocks.begin();
+ LiveOnEntryDef.reset(new MemoryDef(StartingPoint.getContext(), nullptr,
+ nullptr, &StartingPoint, NextID++));
// We maintain lists of memory accesses per-block, trading memory for time. We
// could just look up the memory access for every possible instruction in the
@@ -1508,7 +1531,7 @@ void MemorySSA::buildMemorySSA(BatchAAResults &BAA) {
SmallPtrSet<BasicBlock *, 32> DefiningBlocks;
// Go through each block, figure out where defs occur, and chain together all
// the accesses.
- for (BasicBlock &B : F) {
+ for (BasicBlock &B : Blocks) {
bool InsertIntoDef = false;
AccessList *Accesses = nullptr;
DefsList *Defs = nullptr;
@@ -1535,11 +1558,26 @@ void MemorySSA::buildMemorySSA(BatchAAResults &BAA) {
// Now do regular SSA renaming on the MemoryDef/MemoryUse. Visited will get
// filled in with all blocks.
SmallPtrSet<BasicBlock *, 16> Visited;
- renamePass(DT->getRootNode(), LiveOnEntryDef.get(), Visited);
+ if (L) {
+ if (auto *P = getMemoryAccess(L->getLoopPreheader())) {
+ for (Use &U : make_early_inc_range(P->uses())) {
+ U.set(LiveOnEntryDef.get());
+ }
+ removeFromLists(P);
+ }
+ SmallVector<BasicBlock *> ExitBlocks;
+ L->getExitBlocks(ExitBlocks);
+ Visited.insert(ExitBlocks.begin(), ExitBlocks.end());
+ renamePass(DT->getNode(L->getLoopPreheader()), LiveOnEntryDef.get(),
+ Visited);
+
+ } else {
+ renamePass(DT->getRootNode(), LiveOnEntryDef.get(), Visited);
+ }
// Mark the uses in unreachable blocks as live on entry, so that they go
// somewhere.
- for (auto &BB : F)
+ for (auto &BB : Blocks)
if (!Visited.count(&BB))
markUnreachableAsLiveOnEntry(&BB);
}
@@ -1847,7 +1885,10 @@ void MemorySSA::removeFromLists(MemoryAccess *MA, bool ShouldDelete) {
void MemorySSA::print(raw_ostream &OS) const {
MemorySSAAnnotatedWriter Writer(this);
- F.print(OS, &Writer);
+ Function *F = this->F;
+ if (L)
+ F = L->getHeader()->getParent();
+ F->print(OS, &Writer);
}
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
@@ -1860,10 +1901,23 @@ void MemorySSA::verifyMemorySSA(VerificationLevel VL) const {
#endif
#ifndef NDEBUG
- verifyOrderingDominationAndDefUses(F, VL);
- verifyDominationNumbers(F);
- if (VL == VerificationLevel::Full)
- verifyPrevDefInPhis(F);
+ if (F) {
+ auto Blocks = iterator_range(F->begin(), F->end());
+ verifyOrderingDominationAndDefUses(Blocks, VL);
+ verifyDominationNumbers(Blocks);
+ if (VL == VerificationLevel::Full)
+ verifyPrevDefInPhis(Blocks);
+ } else {
+ assert(L && "must either have loop or function");
+ auto Blocks =
+ map_range(L->blocks(), [](const BasicBlock *BB) -> BasicBlock & {
+ return *const_cast<BasicBlock *>(BB);
+ });
+ verifyOrderingDominationAndDefUses(Blocks, VL);
+ verifyDominationNumbers(Blocks);
+ if (VL == VerificationLevel::Full)
+ verifyPrevDefInPhis(Blocks);
+ }
#endif
// Previously, the verification used to also verify that the clobberingAccess
// cached by MemorySSA is the same as the clobberingAccess found at a later
@@ -1877,8 +1931,9 @@ void MemorySSA::verifyMemorySSA(VerificationLevel VL) const {
// example, see test4 added in D51960.
}
-void MemorySSA::verifyPrevDefInPhis(Function &F) const {
- for (const BasicBlock &BB : F) {
+template <typename IterT>
+void MemorySSA::verifyPrevDefInPhis(IterT Blocks) const {
+ for (const BasicBlock &BB : Blocks) {
if (MemoryPhi *Phi = getMemoryAccess(&BB)) {
for (unsigned I = 0, E = Phi->getNumIncomingValues(); I != E; ++I) {
auto *Pred = Phi->getIncomingBlock(I);
@@ -1913,12 +1968,13 @@ void MemorySSA::verifyPrevDefInPhis(Function &F) const {
/// Verify that all of the blocks we believe to have valid domination numbers
/// actually have valid domination numbers.
-void MemorySSA::verifyDominationNumbers(const Function &F) const {
+template <typename IterT>
+void MemorySSA::verifyDominationNumbers(IterT Blocks) const {
if (BlockNumberingValid.empty())
return;
SmallPtrSet<const BasicBlock *, 16> ValidBlocks = BlockNumberingValid;
- for (const BasicBlock &BB : F) {
+ for (const BasicBlock &BB : Blocks) {
if (!ValidBlocks.count(&BB))
continue;
@@ -1954,14 +2010,15 @@ void MemorySSA::verifyDominationNumbers(const Function &F) const {
/// Verify def-uses: the immediate use information - walk all the memory
/// accesses and verifying that, for each use, it appears in the appropriate
/// def's use list
-void MemorySSA::verifyOrderingDominationAndDefUses(Function &F,
+template <typename IterT>
+void MemorySSA::verifyOrderingDominationAndDefUses(IterT Blocks,
VerificationLevel VL) const {
// Walk all the blocks, comparing what the lookups think and what the access
// lists think, as well as the order in the blocks vs the order in the access
// lists.
SmallVector<MemoryAccess *, 32> ActualAccesses;
SmallVector<MemoryAccess *, 32> ActualDefs;
- for (BasicBlock &B : F) {
+ for (BasicBlock &B : Blocks) {
const AccessList *AL = getBlockAccesses(&B);
const auto *DL = getBlockDefs(&B);
MemoryPhi *Phi = getMemoryAccess(&B);
diff --git a/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp b/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
index 75fb8765061edf..5f38e64873084c 100644
--- a/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
@@ -16,6 +16,7 @@
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopedHashTable.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
@@ -27,6 +28,7 @@
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/LoopPass.h"
#include "llvm/Analysis/LoopUnrollAnalyzer.h"
+#include "llvm/Analysis/MemorySSA.h"
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
#include "llvm/Analysis/ProfileSummaryInfo.h"
#include "llvm/Analysis/ScalarEvolution.h"
@@ -1140,7 +1142,8 @@ tryToUnrollLoop(Loop *L, DominatorTree &DT, LoopInfo *LI, ScalarEvolution &SE,
std::optional<bool> ProvidedUpperBound,
std::optional<bool> ProvidedAllowPeeling,
std::optional<bool> ProvidedAllowProfileBasedPeeling,
- std::optional<unsigned> ProvidedFullUnrollMaxCount) {
+ std::optional<unsigned> ProvidedFullUnrollMaxCount,
+ AAResults *AA = nullptr) {
LLVM_DEBUG(dbgs() << "Loop Unroll: F["
<< L->getHeader()->getParent()->getName() << "] Loop %"
@@ -1292,7 +1295,7 @@ tryToUnrollLoop(Loop *L, DominatorTree &DT, LoopInfo *LI, ScalarEvolution &SE,
ValueToValueMapTy VMap;
if (peelLoop(L, PP.PeelCount, LI, &SE, DT, &AC, PreserveLCSSA, VMap)) {
- simplifyLoopAfterUnroll(L, true, LI, &SE, &DT, &AC, &TTI);
+ simplifyLoopAfterUnroll(L, true, LI, &SE, &DT, &AC, &TTI, nullptr);
// If the loop was peeled, we already "used up" the profile information
// we had, so we don't want to unroll or peel again.
if (PP.PeelProfiledIterations)
@@ -1325,7 +1328,7 @@ tryToUnrollLoop(Loop *L, DominatorTree &DT, LoopInfo *LI, ScalarEvolution &SE,
L,
{UP.Count, UP.Force, UP.Runtime, UP.AllowExpensiveTripCount,
UP.UnrollRemainder, ForgetAllSCEV},
- LI, &SE, &DT, &AC, &TTI, &ORE, PreserveLCSSA, &RemainderLoop);
+ LI, &SE, &DT, &AC, &TTI, &ORE, PreserveLCSSA, &RemainderLoop, AA);
if (UnrollResult == LoopUnrollResult::Unmodified)
return LoopUnrollResult::Unmodified;
@@ -1572,6 +1575,7 @@ PreservedAnalyses LoopUnrollPass::run(Function &F,
auto &DT = AM.getResult<DominatorTreeAnalysis>(F);
auto &AC = AM.getResult<AssumptionAnalysis>(F);
auto &ORE = AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
+ AAResults &AA = AM.getResult<AAManager>(F);
LoopAnalysisManager *LAM = nullptr;
if (auto *LAMProxy = AM.getCachedResult<LoopAnalysisManagerFunctionProxy>(F))
@@ -1601,6 +1605,7 @@ PreservedAnalyses LoopUnrollPass::run(Function &F,
SmallPriorityWorklist<Loop *, 4> Worklist;
appendLoopsToWorklist(LI, Worklist);
+ SmallVector<Loop *> LoopsForCSE;
while (!Worklist.empty()) {
// Because the LoopInfo stores the loops in RPO, we walk the worklist
// from back to front so that we work forward across the CFG, which
@@ -1627,7 +1632,8 @@ PreservedAnalyses LoopUnrollPass::run(Function &F,
/*Count*/ std::nullopt,
/*Threshold*/ std::nullopt, UnrollOpts.AllowPartial,
UnrollOpts.AllowRuntime, UnrollOpts.AllowUpperBound, LocalAllowPeeling,
- UnrollOpts.AllowProfileBasedPeeling, UnrollOpts.FullUnrollMaxCount);
+ UnrollOpts.AllowProfileBasedPeeling, UnrollOpts.FullUnrollMaxCount,
+ &AA);
Changed |= Result != LoopUnrollResult::Unmodified;
// The parent must not be damaged by unrolling!
diff --git a/llvm/lib/Transforms/Utils/LoopUnroll.cpp b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
index 6f0d000815726e..3aa3630c8a824e 100644
--- a/llvm/lib/Transforms/Utils/LoopUnroll.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
@@ -18,17 +18,20 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopedHashTable.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
#include "llvm/ADT/ilist_iterator.h"
+#include "llvm/Analysis/AliasAnalysis.h"
#include "llvm/Analysis/AssumptionCache.h"
#include "llvm/Analysis/DomTreeUpdater.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/LoopIterator.h"
+#include "llvm/Analysis/MemorySSA.h"
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
#include "llvm/Analysis/ScalarEvolution.h"
#include "llvm/IR/BasicBlock.h"
@@ -209,13 +212,143 @@ static bool isEpilogProfitable(Loop *L) {
return false;
}
+struct LoadValue {
+ Instruction *DefI = nullptr;
+ unsigned Generation = 0;
+ LoadValue() = default;
+ LoadValue(Instruction *Inst, unsigned Generation)
+ : DefI(Inst), Generation(Generation) {}
+};
+
+class StackNode {
+ ScopedHashTable<const SCEV *, LoadValue>::ScopeTy LoadScope;
+ unsigned CurrentGeneration;
+ unsigned ChildGeneration;
+ DomTreeNode *Node;
+ DomTreeNode::const_iterator ChildIter;
+ DomTreeNode::const_iterator EndIter;
+ bool Processed = false;
+
+public:
+ StackNode(ScopedHashTable<const SCEV *, LoadValue> &AvailableLoads,
+ unsigned cg, DomTreeNode *N, DomTreeNode::const_iterator Child,
+ DomTreeNode::const_iterator End)
+ : LoadScope(AvailableLoads), CurrentGeneration(cg), ChildGeneration(cg),
+ Node(N), ChildIter(Child), EndIter(End) {}
+ // Accessors.
+ unsigned currentGeneration() const { return CurrentGeneration; }
+ unsigned childGeneration() const { return ChildGeneration; }
+ void childGeneration(unsigned generation) { ChildGeneration = generation; }
+ DomTreeNode *node() { return Node; }
+ DomTreeNode::const_iterator childIter() const { return ChildIter; }
+
+ DomTreeNode *nextChild() {
+ DomTreeNode *child = *ChildIter;
+ ++ChildIter;
+ return child;
+ }
+
+ DomTreeNode::const_iterator end() const { return EndIter; }
+ bool isProcessed() const { return Processed; }
+ void process() { Processed = true; }
+};
+
+Value *getMatchingValue(LoadValue LV, LoadInst *LI, unsigned CurrentGeneration,
+ MemorySSA *MSSA) {
+ if (!LV.DefI)
+ return nullptr;
+ if (LV.Generation != CurrentGeneration) {
+ if (!MSSA)
+ return nullptr;
+ auto *EarlierMA = MSSA->getMemoryAccess(LV.DefI);
+ MemoryAccess *LaterDef;
+ LaterDef = MSSA->getWalker()->getClobberingMemoryAccess(LI);
+ if (!MSSA->dominates(LaterDef, EarlierMA))
+ return nullptr;
+ }
+
+ if (LV.DefI->getType() != LI->getType())
+ return nullptr;
+ return LV.DefI;
+}
+
+void loadCSE(Loop *L, DominatorTree &DT, ScalarEvolution &SE, LoopInfo &LI,
+ function_ref<MemorySSA *()> GetMSSA) {
+ ScopedHashTable<const SCEV *, LoadValue> AvailableLoads;
+ SmallVector<std::unique_ptr<StackNode>> NodesToProcess;
+ DomTreeNode *HeaderD = DT.getNode(L->getHeader());
+ NodesToProcess.emplace_back(new StackNode(AvailableLoads, 0, HeaderD,
+ HeaderD->begin(), HeaderD->end()));
+
+ unsigned CurrentGeneration = 0;
+ while (!NodesToProcess.empty()) {
+ // Grab the first item off the stack. Set the current generation, remove
+ // the node from the stack, and process it.
+ StackNode *NodeToProcess = &*NodesToProcess.back();
+
+ // Initialize class members.
+ CurrentGeneration = NodeToProcess->currentGeneration();
+
+ if (!NodeToProcess->isProcessed()) {
+ // Process the node.
+
+ // If this block has a single predecessor, then the predecessor is the
+ // parent
+ // of the domtree node and all of the live out memory values are still
+ // current in this block. If this block has multiple predecessors, then
+ // they could have invalidated the live-out memory values of our parent
+ // value. For now, just be conservative and invalidate memory if this
+ // block has multiple predecessors.
+ if (!NodeToProcess->node()->getBlock()->getSinglePredecessor())
+ +...
[truncated]
|
|
what about conditionally running EarlyCSE if LoopUnroll made a change? |
|
I guess that would be extending #81275 to also run EarlyCSE |
That would be another option, but I think we would need a loop-only version of EarlyCSE to avoid unnecessary compile-time increases. Also, regular EarlyCSE is not enough to eliminate loads that become redundant across unrolled iterations; to do that, this patch uses SCEV expressions as key to handle pointers from different iterations (which are equal due to different offsets). Happy to go down that route if we agree in general that we want to handle such cases by running (sets of) passes depending on results from earlier passes. I think overall that would be preferable, as it makes it easier to independently test those extra passes. |
|
I prototyped this as separate patch, and the compile-time impact appears noticeably larger https://llvm-compile-time-tracker.com/compare.php?from=5c3d001668ec6117045a9750a1f9d7e3995adfee&to=12953d86d1d0a9400b0f91b9a9cc949060ab9589&stat=instructions:u |
|
ping :) Looks like the separate pass approach adds notably more overhead. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
✅ With the latest revision this PR passed the Python code formatter. |
|
ping |
|
ping :) |
|
ping, any suggestions how to move forward with this? |
|
ping |
nikic
reviewed
Apr 24, 2024
There was a problem hiding this comment.
The general approach here seems okay to me. The late unroll pass is awkward because it's at the very end of the pipeline, and it's not really feasible to do much optimization after it. The targeted approach here looks fine in that sense, and doesn't seem to add that much additional code complexity. It's not something that will scale though -- there are more and more optimization we could in theory perform after late unrolling, but we can't reasonably keep replicating them in the post-unroll simplification.
Are you able to share what non-benchmark workload was the original motivation for this?
Conflicts: llvm/test/Transforms/LoopUnroll/unroll-loads-cse.ll llvm/test/Transforms/PhaseOrdering/AArch64/extra-unroll-simplifications.ll
fhahn
commented
Apr 24, 2024
There was a problem hiding this comment.
The general approach here seems okay to me. The late unroll pass is awkward because it's at the very end of the pipeline, and it's not really feasible to do much optimization after it. The targeted approach here looks fine in that sense, and doesn't seem to add that much additional code complexity. It's not something that will scale though -- there are more and more optimization we could in theory perform after late unrolling, but we can't reasonably keep replicating them in the post-unroll simplification.
Agreed, if more cleanups would be beneficial, we likely need to get back to the drawing board.
Are you able to share what non-benchmark workload was the original motivation for this?
Unfortunately the code is proprietary so I can't share the code, but it is an implementation of Deeplabv3 (https://arxiv.org/abs/1706.05587) tuned for AArch64 and used to blur video backgrounds. The regression is coming from loops with vector intrinsics + unroll pragmas.
|
ping :) |
nikic
reviewed
May 1, 2024
nikic
approved these changes
May 2, 2024
| DomTreeNode::const_iterator childIter() const { return ChildIter; } | ||
|
|
||
| DomTreeNode *nextChild() { | ||
| DomTreeNode *child = *ChildIter; |
There was a problem hiding this comment.
Suggested change
| DomTreeNode *child = *ChildIter; | |
| DomTreeNode *Child = *ChildIter; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds loadCSE support to simplifyLoopAfterUnroll. It is based
on EarlyCSE's implementation using ScopeHashTable and is using SCEV for
accessed pointers to check to find redundant loads after unrolling.
This applies to the late unroll pass only, for full unrolling those
redundant loads will be cleaned up by the regular pipeline.
The current approach constructs MSSA on-demand per-loop, but there is
still small but notable compile-time impact:
stage1-O3 +0.04%
stage1-ReleaseThinLTO +0.06%
stage1-ReleaseLTO-g +0.05%
stage1-O0-g +0.02%
stage2-O3 +0.09%
stage2-O0-g +0.04%
stage2-clang +0.02%
https://llvm-compile-time-tracker.com/compare.php?from=c089fa5a729e217d0c0d4647656386dac1a1b135&to=ec7c0f27cb5c12b600d9adfc8543d131765ec7be&stat=instructions:u
This benefits some workloads with runtime-unrolling disabled,
where users use pragmas to force unrolling, as well as with
runtime unrolling enabled.
On SPEC/MultiSource, this removes a number of loads after unrolling
on AArch64 with runtime unrolling enabled.