AMDGPU: Add NextUseAnalysis Pass - #178873
Conversation
7804146 to
a46ffd9
Compare
d10c71b to
466868a
Compare
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
0d20ec4 to
3b16136
Compare
| if (!LoopTo) | ||
| return encodeLoopDepth(getEffectiveLoopDepth(LoopFrom, To, MLI)); | ||
|
|
||
| if (LoopFrom->contains(LoopTo)) // covers LoopFrom == LoopTo |
There was a problem hiding this comment.
Won't this be implied by the loop depth subtract below
There was a problem hiding this comment.
Not quite sure what you are asking here.
FWIW, I re-worked the logic a little bit (now in calcLoopExits).
There are four branches
- No from loop
Fromloop containsToloopToloop containsFromloop- All other loop relationships
Is this better?
linuxrocks123
left a comment
There was a problem hiding this comment.
Very good work. I am generally impressed with the quality of the code. My main overall criticism is that more in-code documentation is required for clarity. Please see my line comments as well.
| InstrIdTy Id = 0; | ||
| for (auto &MI : BB->instrs()) { | ||
| InstrToId[&MI] = Id; | ||
| if (!computeMode() || !MI.isPHI()) |
There was a problem hiding this comment.
What is the purpose of this conditional? It results in duplicate IDs which you are having to special case elsewhere. A comment explaining the reasoning would be helpful, and an explanation would help evaluate @arsenm's objection to the special casing that the duplicate IDs require.
There was a problem hiding this comment.
This is my solution to maintaining compatibility with PR #156079 AMDGPUNextUseAnalysis.cpp#L411-L412.
Basically PHIs do not contribute to distances.
This has resulted in two special cases:
LiveRegUse::isCloserThanAMDGPU/AMDGPUNextUseAnalysis.cpp#L97-L108calcDistanceToUseForComputeAMDGPUNextUseAnalysis.cpp#L1359-L1361
I'm open to other suggestions.
I'll at least add a comment in the next drop.
9ba0c5d to
2faaba6
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
2faaba6 to
7ba99b9
Compare
|
@llvm/pr-subscribers-backend-amdgpu Author: None (macurtis-amd) ChangesBased on
See those PRs for background. Provides a compatibility mode option Currently has performance charactistics similar to PR #171520. Patch is 29.47 MiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/178873.diff 40 Files Affected:
diff --git a/llvm/include/llvm/Passes/TargetPassRegistry.inc b/llvm/include/llvm/Passes/TargetPassRegistry.inc
index 068b27794191c..4e778e77fa128 100644
--- a/llvm/include/llvm/Passes/TargetPassRegistry.inc
+++ b/llvm/include/llvm/Passes/TargetPassRegistry.inc
@@ -92,6 +92,14 @@ if (PIC) {
return true; \
}
+#define ADD_ANALYSIS_PASS(NAME, CREATE_PASS) \
+ if (Name == "require<" NAME ">") { \
+ PM.addPass( \
+ RequireAnalysisPass<std::remove_reference_t<decltype(CREATE_PASS)>, \
+ MachineFunction>()); \
+ return true; \
+ }
+
PB.registerPipelineParsingCallback([=](StringRef Name, ModulePassManager &PM,
ArrayRef<PassBuilder::PipelineElement>) {
#define MODULE_PASS(NAME, CREATE_PASS) ADD_PASS(NAME, CREATE_PASS)
@@ -151,6 +159,16 @@ PB.registerPipelineParsingCallback([=](StringRef Name, FunctionPassManager &PM,
return false;
});
+PB.registerPipelineParsingCallback([=](StringRef Name,
+ MachineFunctionPassManager &PM,
+ ArrayRef<PassBuilder::PipelineElement>) {
+#define MACHINE_FUNCTION_ANALYSIS(NAME, CREATE_PASS) \
+ ADD_ANALYSIS_PASS(NAME, CREATE_PASS)
+#include GET_PASS_REGISTRY
+#undef MACHINE_FUNCTION_ANALYSIS
+ return false;
+});
+
#undef ADD_PASS
#undef ADD_PASS_WITH_PARAMS
diff --git a/llvm/lib/Target/AMDGPU/AMDGPU.h b/llvm/lib/Target/AMDGPU/AMDGPU.h
index 878f374110159..9680ac648c4de 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPU.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPU.h
@@ -47,6 +47,8 @@ FunctionPass *createSIWholeQuadModeLegacyPass();
FunctionPass *createSIFixControlFlowLiveIntervalsPass();
FunctionPass *createSIOptimizeExecMaskingPreRAPass();
FunctionPass *createSIOptimizeVGPRLiveRangeLegacyPass();
+FunctionPass *createAMDGPUNextUseAnalysisLegacyPass();
+FunctionPass *createAMDGPUNextUseAnalysisPrinterLegacyPass();
FunctionPass *createSIFixSGPRCopiesLegacyPass();
FunctionPass *createLowerWWMCopiesPass();
FunctionPass *createSIMemoryLegalizerPass();
@@ -193,6 +195,12 @@ extern char &SIFixSGPRCopiesLegacyID;
void initializeSIFixVGPRCopiesLegacyPass(PassRegistry &);
extern char &SIFixVGPRCopiesID;
+void initializeAMDGPUNextUseAnalysisLegacyPassPass(PassRegistry &);
+extern char &AMDGPUNextUseAnalysisLegacyID;
+
+void initializeAMDGPUNextUseAnalysisPrinterLegacyPassPass(PassRegistry &);
+extern char &AMDGPUNextUseAnalysisPrinterLegacyID;
+
void initializeSILowerWWMCopiesLegacyPass(PassRegistry &);
extern char &SILowerWWMCopiesLegacyID;
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUNextUseAnalysis.cpp b/llvm/lib/Target/AMDGPU/AMDGPUNextUseAnalysis.cpp
new file mode 100644
index 0000000000000..5e36737c6d2f8
--- /dev/null
+++ b/llvm/lib/Target/AMDGPU/AMDGPUNextUseAnalysis.cpp
@@ -0,0 +1,2296 @@
+//===---------------------- AMDGPUNextUseAnalysis.cpp ---------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AMDGPUNextUseAnalysis.h"
+#include "AMDGPU.h"
+#include "GCNRegPressure.h"
+#include "GCNSubtarget.h"
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/PostOrderIterator.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/CodeGen/MachineBasicBlock.h"
+#include "llvm/CodeGen/MachineDominators.h"
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineInstr.h"
+#include "llvm/CodeGen/MachineLoopInfo.h"
+#include "llvm/IR/ModuleSlotTracker.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/Timer.h"
+#include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include <algorithm>
+#include <cmath>
+#include <limits>
+#include <queue>
+#include <string>
+
+using namespace llvm;
+
+#define DEBUG_TYPE "amdgpu-next-use-analysis"
+
+//==============================================================================
+// Options etc
+//==============================================================================
+namespace {
+
+cl::opt<bool> DumpNextUseDistance("amdgpu-next-use-analysis-dump-distance",
+ cl::init(false), cl::Hidden);
+
+cl::opt<std::string>
+ DumpNextUseDistanceAsJson("amdgpu-next-use-analysis-dump-distance-as-json",
+ cl::Hidden);
+cl::opt<bool>
+ DumpNextUseDistanceVerbose("amdgpu-next-use-analysis-dump-distance-verbose",
+ cl::init(false), cl::Hidden);
+
+cl::opt<AMDGPUNextUseAnalysis::CompatibilityMode> CompatModeOpt(
+ "amdgpu-next-use-analysis-compatibility-mode", cl::Hidden,
+ cl::init(AMDGPUNextUseAnalysis::CompatibilityMode::Graphics),
+ cl::values(clEnumValN(AMDGPUNextUseAnalysis::CompatibilityMode::Graphics,
+ "graphics", "TBD"),
+ clEnumValN(AMDGPUNextUseAnalysis::CompatibilityMode::Compute,
+ "compute", "TBD")));
+} // namespace
+
+//==============================================================================
+// LiveRegUse - Represents a live register use with its distance. Used for
+// tracking and sorting register uses by distance.
+//==============================================================================
+namespace {
+using UseDistancePair = AMDGPUNextUseAnalysis::UseDistancePair;
+struct LiveRegUse : public UseDistancePair {
+ using Base = UseDistancePair;
+
+ // 'nullptr' indicates an unset/invalid state.
+ LiveRegUse() : UseDistancePair(nullptr, 0.0) {}
+ LiveRegUse(const MachineOperand *Use, double Dist)
+ : UseDistancePair(Use, Dist) {}
+ LiveRegUse(const UseDistancePair &P) : UseDistancePair(P) {}
+
+ bool isUnset() const { return Use == nullptr; }
+
+ Register getReg() const { return Use->getReg(); }
+ unsigned getSubReg() const { return Use->getSubReg(); }
+ LaneBitmask getLaneMask(const SIRegisterInfo *TRI) const {
+ return TRI->getSubRegIndexLaneMask(Use->getSubReg());
+ }
+
+ bool isCloserThan(const LiveRegUse &X) const {
+ if (Dist < X.Dist)
+ return true;
+
+ if (Dist > X.Dist)
+ return false;
+
+ if (Use == X.Use)
+ return false;
+
+ // Ugh. In computeMode PHIs and the first non-PHI instruction have id
+ // 0. In this case, consider PHIs as less than the first non-PHI
+ // instruction.
+ const MachineInstr *ThisMI = Use->getParent();
+ const MachineInstr *XMI = X.Use->getParent();
+ const MachineBasicBlock *ThisMBB = ThisMI->getParent();
+ if (ThisMBB == XMI->getParent()) {
+ bool XIsPhiOp = ThisMI->isPHI();
+ bool YIsPhiOp = XMI->isPHI();
+ if (XIsPhiOp && !YIsPhiOp && XMI == &(*ThisMBB->getFirstNonPHI()))
+ return true;
+ }
+
+ // Ensure deterministic results
+ return X.getReg() < getReg();
+ }
+};
+
+inline bool updateClosest(LiveRegUse &Closest, const LiveRegUse &X) {
+ if (!Closest.Use || X.isCloserThan(Closest)) {
+ Closest = X;
+ return true;
+ }
+ return false;
+}
+
+inline bool updateFurthest(LiveRegUse &Furthest, const LiveRegUse &X) {
+ if (!Furthest.Use || Furthest.isCloserThan(X)) {
+ Furthest = X;
+ return true;
+ }
+ return false;
+}
+} // namespace
+
+//==============================================================================
+// json helpers
+//==============================================================================
+namespace {
+template <typename Lambda>
+void printStringAttr(json::OStream &J, const char *Name, Lambda L) {
+ J.attributeBegin(Name);
+ raw_ostream &OS = J.rawValueBegin();
+ OS << '"';
+ L(OS);
+ OS << '"';
+ J.rawValueEnd();
+ J.attributeEnd();
+}
+void printStringAttr(json::OStream &J, const char *Name, Printable P) {
+ printStringAttr(J, Name, [&](raw_ostream &OS) { OS << P; });
+}
+
+void printStringAttr(json::OStream &J, const char *Name, const MachineInstr &MI,
+ ModuleSlotTracker &MST) {
+ printStringAttr(J, Name, [&](raw_ostream &OS) {
+ MI.print(OS, MST,
+ /* IsStandalone */ false,
+ /* SkipOpers */ false,
+ /* SkipDebugLoc */ false,
+ /* AddNewLine ---> */ false,
+ /* TargetInstrInfo */ nullptr);
+ });
+}
+
+void printMBBNameAttr(json::OStream &J, const char *Name,
+ const MachineBasicBlock &MBB, ModuleSlotTracker &MST) {
+ printStringAttr(J, Name, [&](raw_ostream &OS) {
+ MBB.printName(OS, MachineBasicBlock::PrintNameIr, &MST);
+ });
+}
+
+template <typename NameLambda, typename ValueT>
+void printAttr(json::OStream &J, NameLambda NL, ValueT V) {
+ std::string Name;
+ raw_string_ostream NameOS(Name);
+ NL(NameOS);
+ J.attribute(NameOS.str(), V);
+}
+
+template <typename ValueT>
+void printAttr(json::OStream &J, const Printable &P, ValueT V) {
+ printAttr(J, [&](raw_ostream &OS) { OS << P; }, V);
+}
+
+} // namespace
+
+//==============================================================================
+// AMDGPUNextUseAnalysisImpl
+//==============================================================================
+class llvm::AMDGPUNextUseAnalysisImpl {
+ using CompatibilityMode = AMDGPUNextUseAnalysis::CompatibilityMode;
+ const MachineFunction *MF = nullptr;
+ const SIRegisterInfo *TRI = nullptr;
+ const SIInstrInfo *TII = nullptr;
+ const MachineLoopInfo *MLI = nullptr;
+ const MachineRegisterInfo *MRI = nullptr;
+
+ using InstrIdTy = unsigned;
+ using InstrToIdMap = DenseMap<const MachineInstr *, InstrIdTy>;
+ InstrToIdMap InstrToId;
+ CompatibilityMode CompatMode;
+
+ void initializeTables() {
+ for (const MachineBasicBlock &BB : *MF)
+ calcInstrIds(&BB, InstrToId);
+ initializeCfgPaths();
+ initializeInterBlockDistances();
+ }
+
+ void clearTables() {
+ InstrToId.clear();
+ RegUseMap.clear();
+ Paths.clear();
+
+ LastMI = nullptr;
+ LastDistances.clear();
+ }
+
+ bool computeMode() const { return CompatMode == CompatibilityMode::Compute; }
+
+ bool graphicsMode() const {
+ return CompatMode == CompatibilityMode::Graphics;
+ }
+
+ //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ // Instruction Ids
+ //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+private:
+ void calcInstrIds(const MachineBasicBlock *BB,
+ InstrToIdMap &MutableInstrToId) const {
+ InstrIdTy Id = 0;
+ for (auto &MI : BB->instrs()) {
+ MutableInstrToId[&MI] = Id;
+ if (!computeMode() || !MI.isPHI())
+ ++Id;
+ }
+ }
+
+ /// Returns MI's instruction Id. It renumbers (part of) the BB if MI is not
+ /// found in the map.
+ InstrIdTy getInstrId(const MachineInstr *MI) const {
+ auto It = InstrToId.find(MI);
+ if (It != InstrToId.end())
+ return It->second;
+
+ // Renumber the MBB.
+ // TODO: Renumber from MI onwards.
+ auto &MutableInstrToId = const_cast<InstrToIdMap &>(InstrToId);
+ calcInstrIds(MI->getParent(), MutableInstrToId);
+ return InstrToId.find(MI)->second;
+ }
+
+ // Length of the segment from MI (inclusive) to the first instruction of the
+ // basic block.
+ InstrIdTy getHeadLen(const MachineInstr *MI) const {
+ const MachineBasicBlock *MBB = MI->getParent();
+ return getInstrId(MI) + getInstrId(&MBB->instr_front()) + 1;
+ }
+
+ // Length of the segment from MI (exclusive) to the last instruction of the
+ // basic block.
+ InstrIdTy getTailLen(const MachineInstr *MI) const {
+ const MachineBasicBlock *MBB = MI->getParent();
+ return getInstrId(&MBB->instr_back()) - getInstrId(MI);
+ }
+
+ // Length of the segment from 'From' to 'To' (exclusive). Both instructions
+ // must be in the same basic block.
+ InstrIdTy getDistance(const MachineInstr *From,
+ const MachineInstr *To) const {
+ assert(From->getParent() == To->getParent());
+ return getInstrId(To) - getInstrId(From);
+ }
+
+ //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ // RegUses - cache of uses by register
+ //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+private:
+ DenseMap<Register, SmallVector<const MachineOperand *>> RegUseMap;
+
+ const SmallVector<const MachineOperand *> &getRegisterUses(Register Reg) {
+ auto I = RegUseMap.find(Reg);
+ if (I != RegUseMap.end())
+ return I->second;
+
+ SmallVector<const MachineOperand *> &Uses = RegUseMap[Reg];
+ for (const MachineOperand &UseMO : MRI->use_nodbg_operands(Reg)) {
+ if (!UseMO.isUndef())
+ Uses.push_back(&UseMO);
+ }
+ return Uses;
+ }
+
+ //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ // Paths
+ //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+private:
+ class Path
+ : public std::pair<const MachineBasicBlock *, const MachineBasicBlock *> {
+ public:
+ using Base =
+ std::pair<const MachineBasicBlock *, const MachineBasicBlock *>;
+ using Base::pair;
+ Path(const Base &Pair) : Base(Pair) {};
+
+ const MachineBasicBlock *src() const { return first; }
+ const MachineBasicBlock *dst() const { return second; }
+
+ using DenseMapInfo = llvm::DenseMapInfo<Base>;
+ };
+
+ enum EdgeKind { Back = -1, None = 0, Tree = 1, Forward = 2, Cross = 3 };
+ struct PathInfo {
+ EdgeKind EK;
+ bool Reachable;
+ int ForwardReachable;
+ double LoopWeight;
+ std::optional<double> ShortestDistance;
+ std::optional<double> ShortestUnweightedDistance;
+ InstrIdTy Size;
+
+ bool isBackedge() const { return EK == EdgeKind::Back; }
+
+ bool isForwardReachableSet() const { return 0 <= ForwardReachable; }
+ bool isForwardReachableUnset() const { return ForwardReachable < 0; }
+ bool isForwardReachable() const { return ForwardReachable == 1; }
+ bool isNotForwardReachable() const { return ForwardReachable == 0; }
+ };
+
+ //----------------------------------------------------------------------------
+ // Path Storage - 'Paths' is lazily populated and some members are lazily
+ // computed. All mutations should go through one of the 'initializePathInfo*'
+ // flavors below.
+ //----------------------------------------------------------------------------
+ DenseMap<Path, PathInfo, Path::DenseMapInfo> Paths;
+
+ const PathInfo *maybePathInfoFor(const MachineBasicBlock *From,
+ const MachineBasicBlock *To) const {
+ auto I = Paths.find({From, To});
+ return I == Paths.end() ? nullptr : &I->second;
+ }
+
+ PathInfo &getOrInitPathInfo(const MachineBasicBlock *From,
+ const MachineBasicBlock *To) const {
+ auto *NonConstThis = const_cast<AMDGPUNextUseAnalysisImpl *>(this);
+ auto &MutablePaths = NonConstThis->Paths;
+
+ Path P(From, To);
+ auto [I, Inserted] = MutablePaths.try_emplace(P);
+ if (!Inserted)
+ return I->second;
+
+ bool Reachable = calcIsReachable(P.src(), P.dst());
+
+ // Iterator may have been invalidated by calcIsReachable, so get a fresh
+ // reference to the slot.
+ return NonConstThis->initializePathInfo(MutablePaths.at(P), P,
+ EdgeKind::None, Reachable);
+ }
+
+ const PathInfo &pathInfoFor(const MachineBasicBlock *From,
+ const MachineBasicBlock *To) const {
+ return getOrInitPathInfo(From, To);
+ }
+
+ //----------------------------------------------------------------------------
+ // initializePathInfo* - various flavors of PathInfo initialization. They
+ // (should) always funnel to the first flavor below.
+ //----------------------------------------------------------------------------
+ PathInfo &initializePathInfo(PathInfo &Slot, Path P, EdgeKind EK,
+ bool Reachable) {
+ Slot.EK = EK;
+ Slot.Reachable = Reachable;
+ Slot.ForwardReachable = EK != EdgeKind::None ? (0 < EK) : -1;
+ Slot.LoopWeight = Slot.Reachable ? calcLoopWeight(P.src(), P.dst()) : 0.0;
+ Slot.Size = P.src() == P.dst() ? calcSize(P.src()) : 0;
+ if (EK != EdgeKind::None)
+ Slot.ShortestUnweightedDistance = 0.0;
+ return Slot;
+ }
+
+ PathInfo &initializePathInfo(Path P, EdgeKind EK, bool Reachable) const {
+ auto *NonConstThis = const_cast<AMDGPUNextUseAnalysisImpl *>(this);
+ auto &MutablePaths = NonConstThis->Paths;
+ return NonConstThis->initializePathInfo(MutablePaths[P], P, EK, Reachable);
+ }
+
+ std::pair<PathInfo *, bool> maybeInitializePathInfo(Path P, EdgeKind EK,
+ bool Reachable) const {
+ auto *NonConstThis = const_cast<AMDGPUNextUseAnalysisImpl *>(this);
+ auto &MutablePaths = NonConstThis->Paths;
+ auto [I, Inserted] = MutablePaths.try_emplace(P);
+ if (Inserted)
+ NonConstThis->initializePathInfo(I->second, P, EK, Reachable);
+ return {&I->second, Inserted};
+ }
+
+ bool initializePathInfoForwardReachable(const MachineBasicBlock *From,
+ const MachineBasicBlock *To,
+ bool Value) const {
+ PathInfo &Slot = getOrInitPathInfo(From, To);
+ assert(Slot.isForwardReachableUnset());
+ Slot.ForwardReachable = Value;
+ return Value;
+ }
+
+ double initializePathInfoShortestDistance(const MachineBasicBlock *From,
+ const MachineBasicBlock *To,
+ double Value) const {
+ PathInfo &Slot = getOrInitPathInfo(From, To);
+ assert(!Slot.ShortestDistance.has_value());
+ Slot.ShortestDistance = Value;
+ return Value;
+ }
+
+ double
+ initializePathInfoShortestUnweightedDistance(const MachineBasicBlock *From,
+ const MachineBasicBlock *To,
+ double Value) const {
+ PathInfo &Slot = getOrInitPathInfo(From, To);
+ assert(!Slot.ShortestUnweightedDistance.has_value());
+ Slot.ShortestUnweightedDistance = Value;
+ return Value;
+ }
+
+ //----------------------------------------------------------------------------
+ // initialize*Paths
+ //----------------------------------------------------------------------------
+private:
+ void initializePaths(const SmallVector<Path> &ReachablePaths,
+ const SmallVector<Path> &UnreachablePaths) const {
+ for (bool R : {true, false}) {
+ const auto &ToInit = R ? ReachablePaths : UnreachablePaths;
+ for (const Path &P : ToInit)
+ initializePathInfo(P, EdgeKind::None, R);
+ }
+ }
+
+ void
+ initializeForwardOnlyPaths(const SmallVector<Path> &ReachablePaths,
+ const SmallVector<Path> &UnreachablePaths) const {
+ for (bool R : {true, false}) {
+ const auto &ToInit = R ? ReachablePaths : UnreachablePaths;
+ for (const Path &P : ToInit) {
+ PathInfo &Slot = getOrInitPathInfo(P.src(), P.dst());
+ assert(Slot.isForwardReachableUnset() || Slot.ForwardReachable == R);
+ Slot.ForwardReachable = R;
+ }
+ }
+ }
+
+ // Follow the control flow graph starting at the entry block until all blocks
+ // have been visited. Along the way, initialize the PathInfo for each edge
+ // traversed.
+ void initializeCfgPaths() {
+ Paths.clear();
+
+ int LastOrdinal = 0;
+ struct Ordinals {
+ int Discovered;
+ int Visited;
+ int Finished;
+ };
+ DenseMap<const MachineBasicBlock *, Ordinals> OrdFor;
+
+ SmallVector<const MachineBasicBlock *> Work{&MF->front()};
+ OrdFor[&MF->front()].Discovered = ++LastOrdinal;
+
+ while (!Work.empty()) {
+
+ const MachineBasicBlock *Src = Work.back();
+ Ordinals &SrcOrd = OrdFor[Src];
+
+ if (SrcOrd.Visited) {
+ Work.pop_back();
+ SrcOrd.Finished = ++LastOrdinal;
+ continue;
+ }
+
+ SrcOrd.Vis...
[truncated]
|
|
LGTM |
* Replaced `double` distance values with a class - `NextUseDistance`. And then replaced double representation with int64_t which included changing how loop weights are encoded/computed. * Minor fixes to distance calculations uncovered by the change in distance representation. * Regenerated test MIR CHECKs based on new distance values. * Removed unneeded `EdgeKind`s and simplified `initializeCfgPaths`. * Updated test MIR so that virtual register numbering matches what the analysis pass sees.
* Add cache support in graphics mode * Minor cleanup
Address feedback from Konstantina. * Distance cache cleanup/improvements: - Add `-amdgpu-next-use-analysis-distance-cache` option. - Add `resetDistanceCache()`. - Prefix data member names with `DistanceCache`. - Add hit/miss tracking and reporting. * Make `getRegisterUses` `const`. * Rename `LoopExits` to `RelativeLoopDepth`. * Minor rework of `calcRelativeLoopDepth` for clarity. * Simplify calculation of Inter-block distances by removing negative seeding. * Fix DFS bug fix in `initializeCfgPaths`. * Add `print`/`dump` methods for all key data structures. * Add `LLVM_DEBUG` hooks. * Add test files two new tests: - `acyclic-014bb.mir` — CFG that triggered the DFS bug - `acyclic-770bb.mir` — new larger acyclic test case * Other minor changes as requested.
* Replace graphics/compute mode with finer-grained semantic config options. However, for now, the only valid permutations of the config options correspond to the 'graphics' and 'compute' presets. * Fix an issue in 'graphics' mode where distance values that were intended to remain equal when entering a loop were not. This was caused by the change to loop weight calculation. 'fromLoopDepth' was no longer multiplicative, f(a+b) != f(a) * f(b), but other distance calculation code assumed it was so. * Fix a few minor issues in calculating through-loop distances. * Address other minor review comments. * Regenerate test MIR CHECKs based on new distance values.
39292dc to
75a54c7
Compare
|
LGTM |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/225/builds/6067 Here is the relevant piece of the build log for the reference |
| void initializeSIFixVGPRCopiesLegacyPass(PassRegistry &); | ||
| extern char &SIFixVGPRCopiesID; | ||
|
|
||
| void initializeAMDGPUNextUseAnalysisLegacyPassPass(PassRegistry &); |
| //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| private: | ||
| unsigned sizeOf(const MachineInstr &MI) const { | ||
| // When !Cfg.CountPhis, PHIs do not contribute to distances/sizes since they |
There was a problem hiding this comment.
This probably should be isTransient
| void calcInstrIds(const MachineBasicBlock *BB, | ||
| InstrToIdMap &MutableInstrToId) const { | ||
| InstrIdTy Id = 0; | ||
| for (auto &MI : BB->instrs()) { |
| // TODO: Renumber from MI onwards. | ||
| auto &MutableInstrToId = const_cast<InstrToIdMap &>(InstrToId); | ||
| calcInstrIds(MI->getParent(), MutableInstrToId); | ||
| return InstrToId.find(MI)->second; |
| // basic block. | ||
| InstrIdTy getHeadLen(const MachineInstr *MI) const { | ||
| const MachineBasicBlock *MBB = MI->getParent(); | ||
| return getInstrId(MI) + getInstrId(&MBB->instr_front()) + 1; |
There was a problem hiding this comment.
Almost all of this code is using the wrong iterator. You should be using the default begin/end and ranges instead of instrs* to correctly handle bundles
|
|
||
| // Length of the segment from 'From' to 'To' (exclusive). Both instructions | ||
| // must be in the same basic block. | ||
| InstrIdTy getDistance(const MachineInstr *From, |
|
|
||
| enum class EdgeKind { Back = -1, None = 0, Forward = 1 }; | ||
| static constexpr StringRef toString(EdgeKind EK) { | ||
| if (EK == EdgeKind::Back) |
There was a problem hiding this comment.
switch so new entries get a warning
| return "none"; | ||
| } | ||
|
|
||
| struct PathInfo { |
There was a problem hiding this comment.
Fields can be reordered for packing size
arsenm
left a comment
There was a problem hiding this comment.
I still object to having the "compute" and "graphics" modes, and really object to calling them that. They should have algorithmic names
| return NextUseDistance(-Value); | ||
| } | ||
|
|
||
| constexpr NextUseDistance applyLoopWeight() const { |
There was a problem hiding this comment.
This smells like reinventing BlockFrequencies?
| return A -= B; | ||
| } | ||
|
|
||
| constexpr inline NextUseDistance min(NextUseDistance A, NextUseDistance B) { |
There was a problem hiding this comment.
With operator< implemented std::min should work
Based on - llvm#156079 and - llvm#171520 See those PRs for background. Provides a compatibility mode option `--amdgpu-next-use-analysis-compatibility-mode` that produces results that match either PR llvm#156079 (`compute`) or PR llvm#171520 (`graphics`). Co-authored-by: alex-t <atimofee@amd.com> Co-authored-by: Konstantina Mitropoulou <KonstantinaMitropoulou@amd.com> --------- Co-authored-by: Konstantina Mitropoulou <KonstantinaMitropoulou@amd.com>
llvm#178873 (comment) > PassPass llvm#178873 (comment) > This probably should be isTransient llvm#178873 (comment) > No auto llvm#178873 (comment) > switch so new entries get a warning llvm#178873 (comment) > With operator< implemented std::min should work
llvm#178873 (comment) > PassPass llvm#178873 (comment) > This probably should be isTransient llvm#178873 (comment) > No auto llvm#178873 (comment) > switch so new entries get a warning llvm#178873 (comment) > With operator< implemented std::min should work
Based on
See those PRs for background.
Provides a compatibility mode option
--amdgpu-next-use-analysis-compatibility-modethat produces results that match either PR #156079 (compute) or PR #171520 (graphics).Co-authored-by: alex-t atimofee@amd.com
Co-authored-by: Konstantina Mitropoulou KonstantinaMitropoulou@amd.com