[llvm] [AMDGPU] NFC: Provide RPTracker interface for external iterators (PR #93088)
Jeffrey Byrnes via llvm-commits
llvm-commits at lists.llvm.org
Thu May 23 11:46:33 PDT 2024
https://github.com/jrbyrnes updated https://github.com/llvm/llvm-project/pull/93088
>From 8aa83aa035f027d4cdc6211ec945d961abd72c47 Mon Sep 17 00:00:00 2001
From: Jeffrey Byrnes <Jeffrey.Byrnes at amd.com>
Date: Tue, 21 May 2024 13:34:59 -0700
Subject: [PATCH 1/2] [AMDGPU] NFC: Provide RPTracker interface for external
iterators
Change-Id: I79b54722e6e858961486248d94766c3f3c161160
---
llvm/lib/Target/AMDGPU/GCNRegPressure.cpp | 70 +++++++++++++++--------
llvm/lib/Target/AMDGPU/GCNRegPressure.h | 18 +++---
2 files changed, 56 insertions(+), 32 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
index 5c394e6d6296d..f1c4c8b397ddc 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
@@ -343,24 +343,25 @@ void GCNRPTracker::reset(const MachineInstr &MI,
MaxPressure = CurPressure = getRegPressure(*MRI, LiveRegs);
}
-////////////////////////////////////////////////////////////////////////////////
-// GCNUpwardRPTracker
-
-void GCNUpwardRPTracker::reset(const MachineRegisterInfo &MRI_,
- const LiveRegSet &LiveRegs_) {
+void GCNRPTracker::reset(const MachineRegisterInfo &MRI_,
+ const LiveRegSet &LiveRegs_) {
MRI = &MRI_;
LiveRegs = LiveRegs_;
LastTrackedMI = nullptr;
MaxPressure = CurPressure = getRegPressure(MRI_, LiveRegs_);
}
-void GCNUpwardRPTracker::recede(const MachineInstr &MI) {
+////////////////////////////////////////////////////////////////////////////////
+// GCNUpwardRPTracker
+
+bool GCNUpwardRPTracker::recede(const MachineInstr &MI, bool ShouldTrackIt) {
assert(MRI && "call reset first");
- LastTrackedMI = &MI;
+ if (ShouldTrackIt)
+ LastTrackedMI = &MI;
if (MI.isDebugInstr())
- return;
+ return false;
// Kill all defs.
GCNRegPressure DefPressure, ECDefPressure;
@@ -412,6 +413,7 @@ void GCNUpwardRPTracker::recede(const MachineInstr &MI) {
: max(CurPressure, MaxPressure);
assert(CurPressure == getRegPressure(*MRI, LiveRegs));
+ return false;
}
////////////////////////////////////////////////////////////////////////////////
@@ -430,28 +432,44 @@ bool GCNDownwardRPTracker::reset(const MachineInstr &MI,
return true;
}
-bool GCNDownwardRPTracker::advanceBeforeNext() {
+bool GCNDownwardRPTracker::advanceBeforeNext(MachineInstr *MI,
+ bool ShouldTrackIt,
+ LiveIntervals *TheLIS) {
assert(MRI && "call reset first");
- if (!LastTrackedMI)
- return NextMI == MBBEnd;
+ SlotIndex SI;
+ LiveIntervals *CurrLIS;
+ MachineInstr *CurrMI;
+ if (ShouldTrackIt) {
+ if (!LastTrackedMI)
+ return NextMI == MBBEnd;
+
+ assert(NextMI == MBBEnd || !NextMI->isDebugInstr());
+ CurrLIS = const_cast<LiveIntervals *>(&LIS);
+ CurrMI = const_cast<MachineInstr *>(LastTrackedMI);
+
+ SI = NextMI == MBBEnd
+ ? CurrLIS->getInstructionIndex(*LastTrackedMI).getDeadSlot()
+ : CurrLIS->getInstructionIndex(*NextMI).getBaseIndex();
+ }
- assert(NextMI == MBBEnd || !NextMI->isDebugInstr());
+ else { //! ShouldTrackIt
+ CurrLIS = TheLIS;
+ SI = CurrLIS->getInstructionIndex(*MI).getBaseIndex();
+ CurrMI = MI;
+ }
- SlotIndex SI = NextMI == MBBEnd
- ? LIS.getInstructionIndex(*LastTrackedMI).getDeadSlot()
- : LIS.getInstructionIndex(*NextMI).getBaseIndex();
assert(SI.isValid());
// Remove dead registers or mask bits.
SmallSet<Register, 8> SeenRegs;
- for (auto &MO : LastTrackedMI->operands()) {
+ for (auto &MO : CurrMI->operands()) {
if (!MO.isReg() || !MO.getReg().isVirtual())
continue;
if (MO.isUse() && !MO.readsReg())
continue;
if (!SeenRegs.insert(MO.getReg()).second)
continue;
- const LiveInterval &LI = LIS.getInterval(MO.getReg());
+ const LiveInterval &LI = CurrLIS->getInterval(MO.getReg());
if (LI.hasSubRanges()) {
auto It = LiveRegs.end();
for (const auto &S : LI.subranges()) {
@@ -481,15 +499,18 @@ bool GCNDownwardRPTracker::advanceBeforeNext() {
LastTrackedMI = nullptr;
- return NextMI == MBBEnd;
+ return ShouldTrackIt && (NextMI == MBBEnd);
}
-void GCNDownwardRPTracker::advanceToNext() {
+void GCNDownwardRPTracker::advanceToNext(MachineInstr *MI, bool ShouldTrackIt) {
LastTrackedMI = &*NextMI++;
NextMI = skipDebugInstructionsForward(NextMI, MBBEnd);
+ MachineInstr *CurrMI =
+ ShouldTrackIt ? const_cast<MachineInstr *>(LastTrackedMI) : MI;
+
// Add new registers or mask bits.
- for (const auto &MO : LastTrackedMI->all_defs()) {
+ for (const auto &MO : CurrMI->all_defs()) {
Register Reg = MO.getReg();
if (!Reg.isVirtual())
continue;
@@ -502,11 +523,12 @@ void GCNDownwardRPTracker::advanceToNext() {
MaxPressure = max(MaxPressure, CurPressure);
}
-bool GCNDownwardRPTracker::advance() {
- if (NextMI == MBBEnd)
+bool GCNDownwardRPTracker::advance(MachineInstr *MI, bool ShouldTrackIt,
+ LiveIntervals *TheLIS) {
+ if (ShouldTrackIt && NextMI == MBBEnd)
return false;
- advanceBeforeNext();
- advanceToNext();
+ advanceBeforeNext(MI, ShouldTrackIt, TheLIS);
+ advanceToNext(MI, ShouldTrackIt);
return true;
}
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.h b/llvm/lib/Target/AMDGPU/GCNRegPressure.h
index 752f53752fa68..8abbce138cf16 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.h
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.h
@@ -160,6 +160,9 @@ class GCNRPTracker {
bool After);
public:
+ // reset tracker and set live register set to the specified value.
+ void reset(const MachineRegisterInfo &MRI_, const LiveRegSet &LiveRegs_);
+
// live regs for the current state
const decltype(LiveRegs) &getLiveRegs() const { return LiveRegs; }
const MachineInstr *getLastTrackedMI() const { return LastTrackedMI; }
@@ -180,12 +183,9 @@ class GCNUpwardRPTracker : public GCNRPTracker {
public:
GCNUpwardRPTracker(const LiveIntervals &LIS_) : GCNRPTracker(LIS_) {}
- // reset tracker and set live register set to the specified value.
- void reset(const MachineRegisterInfo &MRI_, const LiveRegSet &LiveRegs_);
-
// reset tracker at the specified slot index.
void reset(const MachineRegisterInfo &MRI, SlotIndex SI) {
- reset(MRI, llvm::getLiveRegs(SI, LIS, MRI));
+ GCNRPTracker::reset(MRI, llvm::getLiveRegs(SI, LIS, MRI));
}
// reset tracker to the end of the MBB.
@@ -200,7 +200,7 @@ class GCNUpwardRPTracker : public GCNRPTracker {
}
// move to the state just before the MI (in program order).
- void recede(const MachineInstr &MI);
+ bool recede(const MachineInstr &MI, bool ShouldTrackIt = true);
// checks whether the tracker's state after receding MI corresponds
// to reported by LIS.
@@ -242,13 +242,15 @@ class GCNDownwardRPTracker : public GCNRPTracker {
// Move to the state right before the next MI or after the end of MBB.
// Returns false if reached end of the block.
- bool advanceBeforeNext();
+ bool advanceBeforeNext(MachineInstr *MI = nullptr, bool ShouldTrackIt = true,
+ LiveIntervals *TheLIS = nullptr);
// Move to the state at the MI, advanceBeforeNext has to be called first.
- void advanceToNext();
+ void advanceToNext(MachineInstr *MI = nullptr, bool ShouldTrackIt = true);
// Move to the state at the next MI. Returns false if reached end of block.
- bool advance();
+ bool advance(MachineInstr *MI = nullptr, bool ShouldTrackIt = true,
+ LiveIntervals *TheLIS = nullptr);
// Advance instructions until before End.
bool advance(MachineBasicBlock::const_iterator End);
>From cd91b8f3f6ef6a0a56e4f6fb0a5bfde6c8959c7f Mon Sep 17 00:00:00 2001
From: Jeffrey Byrnes <Jeffrey.Byrnes at amd.com>
Date: Thu, 23 May 2024 11:11:38 -0700
Subject: [PATCH 2/2] Review comments
Change-Id: Ib798a9d6add7d6dd1ccd0e01bea09ac2f4eeb94f
---
llvm/lib/Target/AMDGPU/GCNRegPressure.cpp | 15 +++---
llvm/lib/Target/AMDGPU/GCNRegPressure.h | 64 +++++++++++++++++------
2 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
index f1c4c8b397ddc..6957bf8da6713 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
@@ -354,14 +354,14 @@ void GCNRPTracker::reset(const MachineRegisterInfo &MRI_,
////////////////////////////////////////////////////////////////////////////////
// GCNUpwardRPTracker
-bool GCNUpwardRPTracker::recede(const MachineInstr &MI, bool ShouldTrackIt) {
+void GCNUpwardRPTracker::recede(const MachineInstr &MI, bool ShouldTrackIt) {
assert(MRI && "call reset first");
if (ShouldTrackIt)
LastTrackedMI = &MI;
if (MI.isDebugInstr())
- return false;
+ return;
// Kill all defs.
GCNRegPressure DefPressure, ECDefPressure;
@@ -413,7 +413,6 @@ bool GCNUpwardRPTracker::recede(const MachineInstr &MI, bool ShouldTrackIt) {
: max(CurPressure, MaxPressure);
assert(CurPressure == getRegPressure(*MRI, LiveRegs));
- return false;
}
////////////////////////////////////////////////////////////////////////////////
@@ -450,9 +449,7 @@ bool GCNDownwardRPTracker::advanceBeforeNext(MachineInstr *MI,
SI = NextMI == MBBEnd
? CurrLIS->getInstructionIndex(*LastTrackedMI).getDeadSlot()
: CurrLIS->getInstructionIndex(*NextMI).getBaseIndex();
- }
-
- else { //! ShouldTrackIt
+ } else { //! ShouldTrackIt
CurrLIS = TheLIS;
SI = CurrLIS->getInstructionIndex(*MI).getBaseIndex();
CurrMI = MI;
@@ -503,8 +500,10 @@ bool GCNDownwardRPTracker::advanceBeforeNext(MachineInstr *MI,
}
void GCNDownwardRPTracker::advanceToNext(MachineInstr *MI, bool ShouldTrackIt) {
- LastTrackedMI = &*NextMI++;
- NextMI = skipDebugInstructionsForward(NextMI, MBBEnd);
+ if (ShouldTrackIt) {
+ LastTrackedMI = &*NextMI++;
+ NextMI = skipDebugInstructionsForward(NextMI, MBBEnd);
+ }
MachineInstr *CurrMI =
ShouldTrackIt ? const_cast<MachineInstr *>(LastTrackedMI) : MI;
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.h b/llvm/lib/Target/AMDGPU/GCNRegPressure.h
index 8abbce138cf16..54e320cf08707 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.h
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.h
@@ -143,6 +143,9 @@ inline GCNRegPressure operator-(const GCNRegPressure &P1,
return Diff;
}
+///////////////////////////////////////////////////////////////////////////////
+// GCNRPTracker
+
class GCNRPTracker {
public:
using LiveRegSet = DenseMap<unsigned, LaneBitmask>;
@@ -179,31 +182,36 @@ class GCNRPTracker {
GCNRPTracker::LiveRegSet getLiveRegs(SlotIndex SI, const LiveIntervals &LIS,
const MachineRegisterInfo &MRI);
+////////////////////////////////////////////////////////////////////////////////
+// GCNUpwardRPTracker
+
class GCNUpwardRPTracker : public GCNRPTracker {
public:
GCNUpwardRPTracker(const LiveIntervals &LIS_) : GCNRPTracker(LIS_) {}
- // reset tracker at the specified slot index.
+ /// reset tracker at the specified slot index \p SI.
void reset(const MachineRegisterInfo &MRI, SlotIndex SI) {
GCNRPTracker::reset(MRI, llvm::getLiveRegs(SI, LIS, MRI));
}
- // reset tracker to the end of the MBB.
+ /// reset tracker to the end of the \p MBB.
void reset(const MachineBasicBlock &MBB) {
reset(MBB.getParent()->getRegInfo(),
LIS.getSlotIndexes()->getMBBEndIdx(&MBB));
}
- // reset tracker to the point just after MI (in program order).
+ /// reset tracker to the point just after \p MI (in program order).
void reset(const MachineInstr &MI) {
reset(MI.getMF()->getRegInfo(), LIS.getInstructionIndex(MI).getDeadSlot());
}
- // move to the state just before the MI (in program order).
- bool recede(const MachineInstr &MI, bool ShouldTrackIt = true);
+ /// Move to the state of RP just before the \p MI . If \p ShouldTrackIt is
+ /// set, also update the internal iterators. Setting \p ShouldTrackIt to false
+ /// allows for an externally managed iterator / program order.
+ void recede(const MachineInstr &MI, bool ShouldTrackIt = true);
- // checks whether the tracker's state after receding MI corresponds
- // to reported by LIS.
+ /// \p returns whether the tracker's state after receding MI corresponds
+ /// to reported by LIS.
bool isValid() const;
const GCNRegPressure &getMaxPressure() const { return MaxPressure; }
@@ -217,6 +225,9 @@ class GCNUpwardRPTracker : public GCNRPTracker {
}
};
+////////////////////////////////////////////////////////////////////////////////
+// GCNDownwardRPTracker
+
class GCNDownwardRPTracker : public GCNRPTracker {
// Last position of reset or advanceBeforeNext
MachineBasicBlock::const_iterator NextMI;
@@ -228,34 +239,53 @@ class GCNDownwardRPTracker : public GCNRPTracker {
MachineBasicBlock::const_iterator getNext() const { return NextMI; }
- // Return MaxPressure and clear it.
+ /// \p return MaxPressure and clear it.
GCNRegPressure moveMaxPressure() {
auto Res = MaxPressure;
MaxPressure.clear();
return Res;
}
- // Reset tracker to the point before the MI
- // filling live regs upon this point using LIS.
- // Returns false if block is empty except debug values.
+ /// Reset tracker to the point before the \p MI
+ /// filling \p LiveRegs upon this point using LIS.
+ /// \p returns false if block is empty except debug values.
bool reset(const MachineInstr &MI, const LiveRegSet *LiveRegs = nullptr);
- // Move to the state right before the next MI or after the end of MBB.
- // Returns false if reached end of the block.
+ /// Move to the state right before the next MI or after the end of MBB.
+ /// \p returns false if reached end of the block.
+ /// If \p ShouldTrackIt is true, then internal iterators are used and set to
+ /// process in program order.
+ /// If \p ShouldTrackIt is false, then it is assumed that the tracker is using
+ /// an externally managed iterator, and advance* calls will not update the
+ /// state of the iterator. In such cases, the tracker will move to the state
+ /// right before the provided \p MI and use the provided \p TheLIS for RP
+ /// calculations.
bool advanceBeforeNext(MachineInstr *MI = nullptr, bool ShouldTrackIt = true,
LiveIntervals *TheLIS = nullptr);
- // Move to the state at the MI, advanceBeforeNext has to be called first.
+ /// Move to the state at the MI, advanceBeforeNext has to be called first.
+ /// If \p ShouldTrackIt is true, then internal iterators are used and set to
+ /// process in program order.
+ /// If \p ShouldTrackIt is false, then it is assumed that the tracker is using
+ /// an externally managed iterator, and advance* calls will not update the
+ /// state of the iterator. In such cases, the tracker will move to the state
+ /// at the provided \p MI .
void advanceToNext(MachineInstr *MI = nullptr, bool ShouldTrackIt = true);
- // Move to the state at the next MI. Returns false if reached end of block.
+ /// Move to the state at the next MI. \p returns false if reached end of
+ /// block. If \p ShouldTrackIt is true, then internal iterators are used and
+ /// set to process in program order. If \p ShouldTrackIt is false, then it is
+ /// assumed that the tracker is using an externally managed iterator, and
+ /// advance* calls will not update the state of the iterator. In such cases,
+ /// the tracker will move to the state right before the provided \p MI and use
+ /// the provided \p TheLIS for RP calculations.
bool advance(MachineInstr *MI = nullptr, bool ShouldTrackIt = true,
LiveIntervals *TheLIS = nullptr);
- // Advance instructions until before End.
+ /// Advance instructions until before \p End.
bool advance(MachineBasicBlock::const_iterator End);
- // Reset to Begin and advance to End.
+ /// Reset to \p Begin and advance to \p End.
bool advance(MachineBasicBlock::const_iterator Begin,
MachineBasicBlock::const_iterator End,
const LiveRegSet *LiveRegsCopy = nullptr);
More information about the llvm-commits
mailing list