[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -320,12 +325,72 @@ class GCNRPTracker {
protected:
const LiveIntervals &LIS;
+
LiveRegSet VirtLiveRegs;
+
+ // Physical register liveness: Units provides O(1) unit-level alias checks,
+ // Regs tracks which register names contributed to pressure for cheap
+ // reconstruction. Both must be kept in sync.
+ struct PhysicalRegLiveness {
+LiveRegUnits Units;
+SmallDenseSet Regs;
+
+void init(const TargetRegisterInfo &TRI) {
+ Units.init(TRI);
+ Regs.clear();
+}
+void clear() {
+ Units.clear();
+ Regs.clear();
+}
+const BitVector &getBitVector() const { return Units.getBitVector(); }
+
+void add(Register Reg) {
+ Units.addReg(Reg);
+ Regs.insert(Reg.asMCReg());
+}
+void remove(Register Reg) {
+ Units.removeReg(Reg);
+ Regs.erase(Reg.asMCReg());
+}
+void remove(const BitVector &KilledUnits, MCRegister Reg) {
+ Units.removeUnits(KilledUnits);
+ Regs.erase(Reg);
+}
+ };
+ PhysicalRegLiveness PhysLiveRegs;
+
GCNRegPressure CurPressure, MaxPressure;
+
+ // Flag to control whether physical register tracking is active.
+ // Set to true when GCNTrackers are enabled, false otherwise.
+ bool TrackPhysRegs = false;
dhruvachak wrote:
With the latest commit, the idea of setPhysRegTracking() and querying the
cached TrackPhysRegs is gone.
Physical register tracking is enabled in this PR whenever a GCNRPTracker is
used. This functionality can be turned OFF by using
-amdgpu-track-physregs-in-gcn-trackers=false.
This simplifies the design and makes more sense than depending on GCNTrackers
being enabled since there are code paths that use GCNRPTracker without checking
for GCNTrackers.
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
dhruvachak wrote: Rebased. Since the old base was quite old, there were many conflicts. The rebase was done after squashing all the individual commits. Since https://github.com/llvm/llvm-project/pull/169616 introduced multi-level controls for enabling useGCNTrackers, I think it is better to enable physical register tracking whenever GCNRPTracker is used. I think it no more makes sense to have physical register tracking depend on the global overriding amdgpu-use-amdgpu-trackers option. This change has now been made in this PR. https://github.com/llvm/llvm-project/pull/184275 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
https://github.com/dhruvachak edited https://github.com/llvm/llvm-project/pull/184275 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
https://github.com/dhruvachak edited https://github.com/llvm/llvm-project/pull/184275 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -596,11 +709,33 @@ void GCNUpwardRPTracker::recede(const MachineInstr &MI) {
CurPressure.inc(U.VRegOrUnit.asVirtualReg(), PrevMask, LiveMask, *MRI);
}
+ if (TrackPhysRegs) {
+// Physical register handling needs the register directly to avoid
aliasing,
+// so we need to iterate over all uses.
+for (const MachineOperand &MO : MI.all_uses()) {
+ if (!MO.isReg() || !MO.getReg().isPhysical() || !MO.readsReg())
dhruvachak wrote:
Done.
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -320,12 +325,72 @@ class GCNRPTracker {
protected:
const LiveIntervals &LIS;
+
LiveRegSet VirtLiveRegs;
+
+ // Physical register liveness: Units provides O(1) unit-level alias checks,
+ // Regs tracks which register names contributed to pressure for cheap
+ // reconstruction. Both must be kept in sync.
arsenm wrote:
> Since this kind of overlap should be rare in practice, to keep things
> efficient, I am leaning towards not making this change. Instead, I will
> document this potential issue.
The liveness needs to be accurately modeled, at any cost. The scheduler cannot
afford to oversubscribe the allocator
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
https://github.com/dhruvachak edited https://github.com/llvm/llvm-project/pull/184275 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -320,12 +325,72 @@ class GCNRPTracker {
protected:
const LiveIntervals &LIS;
+
LiveRegSet VirtLiveRegs;
+
+ // Physical register liveness: Units provides O(1) unit-level alias checks,
+ // Regs tracks which register names contributed to pressure for cheap
+ // reconstruction. Both must be kept in sync.
+ struct PhysicalRegLiveness {
+LiveRegUnits Units;
+SmallDenseSet Regs;
+
+void init(const TargetRegisterInfo &TRI) {
+ Units.init(TRI);
+ Regs.clear();
+}
+void clear() {
+ Units.clear();
+ Regs.clear();
+}
+const BitVector &getBitVector() const { return Units.getBitVector(); }
+
+void add(Register Reg) {
dhruvachak wrote:
Fixed.
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -320,12 +325,72 @@ class GCNRPTracker {
protected:
const LiveIntervals &LIS;
+
LiveRegSet VirtLiveRegs;
+
+ // Physical register liveness: Units provides O(1) unit-level alias checks,
+ // Regs tracks which register names contributed to pressure for cheap
+ // reconstruction. Both must be kept in sync.
dhruvachak wrote:
Good point. Yes, when the UpwardTracker uses recede(), it can indeed lead to
the above described scenario, leading to under-counting of register pressure.
In order to be accurate, we would need a more expensive operation for every
def. Specifically, we would need to check which entries in Regs have their
units fully covered by the def, remove those entries from Regs, and decrement
register pressure for each of them. Since this kind of overlap should be rare
in practice, to keep things efficient, I am leaning towards not making this
change. Instead, I will document this potential issue.
There are a couple of other cases where under/over-counting of register
pressure because of physical registers may occur. The current implementation of
physical register tracking does not consider live-ins/live-outs, so that may
lead to accuracy issues. Computing live-ins/live-outs may not be cheap, so I
wanted to leave them out of the first version. I will document that as well.
While thinking more about your example, I found a potential inconsistency
because of which validation of register pressure may fail. The method recede()
validates the total pressure and then there is the isValid() method on the
tracker. I will push up a fix to that problem.
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
https://github.com/dhruvachak edited https://github.com/llvm/llvm-project/pull/184275 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -555,29 +660,37 @@ void GCNUpwardRPTracker::recede(const MachineInstr &MI) {
GCNRegPressure DefPressure, ECDefPressure;
bool HasECDefs = false;
for (const MachineOperand &MO : MI.all_defs()) {
-if (!MO.getReg().isVirtual())
- continue;
-
Register Reg = MO.getReg();
-LaneBitmask DefMask = getDefRegMask(MO, *MRI);
-// Treat a def as fully live at the moment of definition: keep a record.
macurtis-amd wrote:
This comment got dropped. Was that on purpose?
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -596,11 +709,33 @@ void GCNUpwardRPTracker::recede(const MachineInstr &MI) {
CurPressure.inc(U.VRegOrUnit.asVirtualReg(), PrevMask, LiveMask, *MRI);
}
+ if (TrackPhysRegs) {
+// Physical register handling needs the register directly to avoid
aliasing,
+// so we need to iterate over all uses.
+for (const MachineOperand &MO : MI.all_uses()) {
+ if (!MO.isReg() || !MO.getReg().isPhysical() || !MO.readsReg())
macurtis-amd wrote:
`all_uses()` only returns register operands so MO.isReg() is not needed.
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -320,12 +325,72 @@ class GCNRPTracker {
protected:
const LiveIntervals &LIS;
+
LiveRegSet VirtLiveRegs;
+
+ // Physical register liveness: Units provides O(1) unit-level alias checks,
+ // Regs tracks which register names contributed to pressure for cheap
+ // reconstruction. Both must be kept in sync.
+ struct PhysicalRegLiveness {
+LiveRegUnits Units;
+SmallDenseSet Regs;
+
+void init(const TargetRegisterInfo &TRI) {
+ Units.init(TRI);
+ Regs.clear();
+}
+void clear() {
+ Units.clear();
+ Regs.clear();
+}
+const BitVector &getBitVector() const { return Units.getBitVector(); }
+
+void add(Register Reg) {
+ Units.addReg(Reg);
+ Regs.insert(Reg.asMCReg());
+}
+void remove(Register Reg) {
+ Units.removeReg(Reg);
+ Regs.erase(Reg.asMCReg());
+}
+void remove(const BitVector &KilledUnits, MCRegister Reg) {
+ Units.removeUnits(KilledUnits);
+ Regs.erase(Reg);
+}
+ };
+ PhysicalRegLiveness PhysLiveRegs;
+
GCNRegPressure CurPressure, MaxPressure;
+
+ // Flag to control whether physical register tracking is active.
+ // Set to true when GCNTrackers are enabled, false otherwise.
+ bool TrackPhysRegs = false;
+
const MachineInstr *LastTrackedMI = nullptr;
mutable const MachineRegisterInfo *MRI = nullptr;
+ const SIRegisterInfo *SRI = nullptr;
+
+ GCNRPTracker(const LiveIntervals &LIS, const MachineRegisterInfo &MRI)
+ : LIS(LIS), MRI(&MRI),
+SRI(static_cast(MRI.getTargetRegisterInfo())) {
+setPhysRegTracking();
+if (TrackPhysRegs)
+ PhysLiveRegs.init(*SRI);
macurtis-amd wrote:
Add `PhysicalRegLiveness::PhysicalRegLiveness((const TargetRegisterInfo &TRI)`?
I don't think there is benefit in conditionally initializing `PhysLiveRegs`.
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -130,6 +131,10 @@ struct GCNRegPressure {
LaneBitmask NewMask,
const MachineRegisterInfo &MRI);
+ /// Update pressure for a physical register (add or remove). Used when
+ /// tracking physical registers.
+ void inc(MCRegister Reg, bool IsAdd, const MachineRegisterInfo &MRI);
macurtis-amd wrote:
nit: I think it would be more readable at the call sites if this was:
```
void inc(MCRegister Reg, const MachineRegisterInfo &MRI, bool IsAdd = true);
void dec(MCRegister Reg, const MachineRegisterInfo &MRI) { inc(Reg, MRI,
false); }
```
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -596,11 +709,33 @@ void GCNUpwardRPTracker::recede(const MachineInstr &MI) {
CurPressure.inc(U.VRegOrUnit.asVirtualReg(), PrevMask, LiveMask, *MRI);
}
+ if (TrackPhysRegs) {
+// Physical register handling needs the register directly to avoid
aliasing,
+// so we need to iterate over all uses.
+for (const MachineOperand &MO : MI.all_uses()) {
+ if (!MO.isReg() || !MO.getReg().isPhysical() || !MO.readsReg())
+continue;
+ Register Reg = MO.getReg();
+ if (!MRI->isAllocatable(Reg))
+continue;
+ bool NewlyLive = insertIfNotLive(Reg.asMCReg());
+ if (NewlyLive)
+CurPressure.inc(Reg.asMCReg(), /*IsAdd=*/true, *MRI);
+}
+ }
+
// Update MaxPressure with uses plus early-clobber defs pressure.
MaxPressure = HasECDefs ? max(CurPressure + ECDefPressure, MaxPressure)
: max(CurPressure, MaxPressure);
- assert(CurPressure == getRegPressure(*MRI, VirtLiveRegs));
+ auto VirtPressure = getRegPressure(*MRI, VirtLiveRegs);
+ auto PhysPressure = constructPhysRegPressure();
macurtis-amd wrote:
These produce `set but not used` warnings when assertions are off (`NDEBUG`
defined).
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -320,12 +325,72 @@ class GCNRPTracker {
protected:
const LiveIntervals &LIS;
+
LiveRegSet VirtLiveRegs;
+
+ // Physical register liveness: Units provides O(1) unit-level alias checks,
+ // Regs tracks which register names contributed to pressure for cheap
+ // reconstruction. Both must be kept in sync.
+ struct PhysicalRegLiveness {
+LiveRegUnits Units;
+SmallDenseSet Regs;
+
+void init(const TargetRegisterInfo &TRI) {
+ Units.init(TRI);
+ Regs.clear();
+}
+void clear() {
+ Units.clear();
+ Regs.clear();
+}
+const BitVector &getBitVector() const { return Units.getBitVector(); }
+
+void add(Register Reg) {
+ Units.addReg(Reg);
+ Regs.insert(Reg.asMCReg());
+}
+void remove(Register Reg) {
+ Units.removeReg(Reg);
+ Regs.erase(Reg.asMCReg());
+}
+void remove(const BitVector &KilledUnits, MCRegister Reg) {
+ Units.removeUnits(KilledUnits);
+ Regs.erase(Reg);
+}
+ };
+ PhysicalRegLiveness PhysLiveRegs;
+
GCNRegPressure CurPressure, MaxPressure;
+
+ // Flag to control whether physical register tracking is active.
+ // Set to true when GCNTrackers are enabled, false otherwise.
+ bool TrackPhysRegs = false;
+
const MachineInstr *LastTrackedMI = nullptr;
mutable const MachineRegisterInfo *MRI = nullptr;
+ const SIRegisterInfo *SRI = nullptr;
+
+ GCNRPTracker(const LiveIntervals &LIS, const MachineRegisterInfo &MRI)
+ : LIS(LIS), MRI(&MRI),
+SRI(static_cast(MRI.getTargetRegisterInfo())) {
+setPhysRegTracking();
+if (TrackPhysRegs)
+ PhysLiveRegs.init(*SRI);
+ }
- GCNRPTracker(const LiveIntervals &LIS_) : LIS(LIS_) {}
+ // Copy constructor - PhysLiveRegs.Units must be initialized then copied.
+ GCNRPTracker(const GCNRPTracker &Other)
+ : LIS(Other.LIS), VirtLiveRegs(Other.VirtLiveRegs),
+CurPressure(Other.CurPressure), MaxPressure(Other.MaxPressure),
+TrackPhysRegs(Other.TrackPhysRegs), LastTrackedMI(Other.LastTrackedMI),
+MRI(Other.MRI), SRI(Other.SRI) {
+if (TrackPhysRegs) {
+ assert(SRI && "SRI not initialized");
+ PhysLiveRegs.init(*SRI);
+ PhysLiveRegs.Units.addUnits(Other.PhysLiveRegs.getBitVector());
+ PhysLiveRegs.Regs = Other.PhysLiveRegs.Regs;
+}
+ }
macurtis-amd wrote:
I think the default copy constructor would actually achieve the same result.
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -320,12 +325,72 @@ class GCNRPTracker {
protected:
const LiveIntervals &LIS;
+
LiveRegSet VirtLiveRegs;
+
+ // Physical register liveness: Units provides O(1) unit-level alias checks,
+ // Regs tracks which register names contributed to pressure for cheap
+ // reconstruction. Both must be kept in sync.
+ struct PhysicalRegLiveness {
+LiveRegUnits Units;
+SmallDenseSet Regs;
+
+void init(const TargetRegisterInfo &TRI) {
+ Units.init(TRI);
+ Regs.clear();
+}
+void clear() {
+ Units.clear();
+ Regs.clear();
+}
+const BitVector &getBitVector() const { return Units.getBitVector(); }
+
+void add(Register Reg) {
+ Units.addReg(Reg);
+ Regs.insert(Reg.asMCReg());
+}
+void remove(Register Reg) {
+ Units.removeReg(Reg);
+ Regs.erase(Reg.asMCReg());
+}
+void remove(const BitVector &KilledUnits, MCRegister Reg) {
+ Units.removeUnits(KilledUnits);
+ Regs.erase(Reg);
+}
+ };
+ PhysicalRegLiveness PhysLiveRegs;
+
GCNRegPressure CurPressure, MaxPressure;
+
+ // Flag to control whether physical register tracking is active.
+ // Set to true when GCNTrackers are enabled, false otherwise.
+ bool TrackPhysRegs = false;
macurtis-amd wrote:
I think it would be better to have a static method that returns whether phys
reg tracking is on or off.
This would make `setPhysRegTracking()` unnecessary.
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -555,29 +660,37 @@ void GCNUpwardRPTracker::recede(const MachineInstr &MI) {
GCNRegPressure DefPressure, ECDefPressure;
bool HasECDefs = false;
for (const MachineOperand &MO : MI.all_defs()) {
-if (!MO.getReg().isVirtual())
- continue;
-
Register Reg = MO.getReg();
-LaneBitmask DefMask = getDefRegMask(MO, *MRI);
-// Treat a def as fully live at the moment of definition: keep a record.
-if (MO.isEarlyClobber()) {
- ECDefPressure.inc(Reg, LaneBitmask::getNone(), DefMask, *MRI);
- HasECDefs = true;
-} else
- DefPressure.inc(Reg, LaneBitmask::getNone(), DefMask, *MRI);
+if (Reg.isVirtual()) {
+ LaneBitmask DefMask = getDefRegMask(MO, *MRI);
-auto I = VirtLiveRegs.find(Reg);
-if (I == VirtLiveRegs.end())
- continue;
+ if (MO.isEarlyClobber()) {
+ECDefPressure.inc(Reg, LaneBitmask::getNone(), DefMask, *MRI);
+HasECDefs = true;
+ } else
+DefPressure.inc(Reg, LaneBitmask::getNone(), DefMask, *MRI);
+
+ auto I = VirtLiveRegs.find(Reg);
+ if (I == VirtLiveRegs.end())
+continue;
+
+ LaneBitmask &LiveMask = I->second;
+ LaneBitmask PrevMask = LiveMask;
+ LiveMask &= ~DefMask;
+ CurPressure.inc(Reg, PrevMask, LiveMask, *MRI);
+ if (LiveMask.none())
+VirtLiveRegs.erase(I);
+} else if (TrackPhysRegs && Reg.isPhysical() && MRI->isAllocatable(Reg)) {
macurtis-amd wrote:
nit: This conditional is repeated a few times. Might be nice to create a helper
for it.
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -471,6 +493,73 @@ LaneBitmask llvm::getLiveLaneMask(unsigned Reg, SlotIndex
SI,
return getLiveLaneMask(LIS.getInterval(Reg), SI, MRI, LaneMaskFilter);
}
+bool GCNRPTracker::isUnitLiveAt(MCRegUnit Unit, SlotIndex SI) const {
+ const LiveRange *LR = LIS.getCachedRegUnit(Unit);
+ // If LIS has no reg-unit live range, be conservative and assume it is live.
+ return !LR || LR->liveAt(SI);
+}
+
+bool GCNRPTracker::isAnyRegUnitNotLive(MCRegister Reg) const {
+ assert(SRI && "SRI not initialized");
+ const BitVector &Units = PhysLiveRegs.getBitVector();
+ return llvm::any_of(SRI->regunits(Reg), [&](MCRegUnit Unit) {
+return !Units.test(static_cast(Unit));
+ });
+}
+
+bool GCNRPTracker::checkRegKilled(MCRegister Reg, SlotIndex SI) const {
+ assert(SRI && "SRI not initialized");
+ const BitVector &Units = PhysLiveRegs.getBitVector();
+ return llvm::any_of(SRI->regunits(Reg), [&](MCRegUnit Unit) {
+return Units.test(static_cast(Unit)) && !isUnitLiveAt(Unit, SI);
+ });
+}
+
+bool GCNRPTracker::eraseKilledUnits(MCRegister Reg, SlotIndex SI) {
+ assert(SRI && "SRI not initialized");
+ // Due to aliasing, a physical register may not be present in
+ // PhysLiveRegs.Regs, but one of its regunits may show up as killed. Return
+ // early in this case.
+ if (!PhysLiveRegs.Regs.contains(Reg))
+return false;
+ BitVector KilledUnits(PhysLiveRegs.getBitVector().size(), false);
+ for (MCRegUnit Unit : SRI->regunits(Reg)) {
+unsigned U = static_cast(Unit);
+if (PhysLiveRegs.getBitVector().test(U) && !isUnitLiveAt(Unit, SI))
+ KilledUnits.set(U);
+ }
+ if (KilledUnits.none())
+return false;
+ PhysLiveRegs.remove(KilledUnits, Reg);
+ return true;
+}
+
+bool GCNRPTracker::eraseAllLiveUnits(MCRegister Reg) {
+ assert(SRI && "SRI not initialized");
+ if (!PhysLiveRegs.Regs.contains(Reg))
+return false;
+ PhysLiveRegs.remove(Reg);
+ return true;
+}
+
+bool GCNRPTracker::insertIfNotLive(MCRegister Reg) {
+ assert(SRI && "SRI not initialized");
+ const BitVector &Units = PhysLiveRegs.getBitVector();
+ bool NewlyLive = llvm::any_of(SRI->regunits(Reg), [&](MCRegUnit Unit) {
+return !Units.test(static_cast(Unit));
+ });
macurtis-amd wrote:
replace with `bool NewlyLive = isAnyRegUnitNotLive(Reg);`?
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
@@ -335,12 +400,41 @@ class GCNRPTracker {
LaneBitmask getLastUsedLanes(Register Reg, SlotIndex Pos) const;
+ // Check if a register unit is live at a given slot index per LIS.
+ bool isUnitLiveAt(MCRegUnit Unit, SlotIndex SI) const;
+
+ // Check if any register unit of Reg is not currently live in PhysLiveRegs.
+ bool isAnyRegUnitNotLive(MCRegister Reg) const;
+
+ // Reconstruct physical register pressure from PhysLiveRegs.Regs.
+ GCNRegPressure constructPhysRegPressure() const;
+
+ // Check if Reg has any killed units at the given slot index.
+ bool checkRegKilled(MCRegister Reg, SlotIndex SI) const;
+
+ // Check if Reg has any killed units and erase them from PhysLiveRegs.
+ bool eraseKilledUnits(MCRegister Reg, SlotIndex SI);
+
+ // Erase all live units of Reg from PhysLiveRegs.
+ // Returns true if any unit was live (and thus erased).
+ bool eraseAllLiveUnits(MCRegister Reg);
+
+ // Insert units of Reg into PhysLiveRegs if not already live.
+ // Returns true if any unit was newly inserted.
+ bool insertIfNotLive(MCRegister Reg);
+
public:
+ // Enable physical register tracking only if both GCNTrackers and
+ // TrackPhysRegInTrackers are true.
+ void setPhysRegTracking();
+
// reset tracker and set live register set to the specified value.
void reset(const MachineRegisterInfo &MRInfo,
const LiveRegSet &VirtLiveRegsSet);
+
// live regs for the current state
const decltype(VirtLiveRegs) &getLiveRegs() const { return VirtLiveRegs; }
+ const decltype(VirtLiveRegs) &getVirtLiveRegs() const { return VirtLiveRegs;
}
macurtis-amd wrote:
Unused / unneeded?
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
dhruvachak wrote: ping. @arsenm The parallel pressure tracking system is gone in the latest update. https://github.com/llvm/llvm-project/pull/184275 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
dhruvachak wrote: As per discussion, the GCNPressure objects have been unified, tracking both virtual and physical registers. The live register representation has been extended to track both regunits and registers, so that accurate pressure can be reconstructed. The reconstruction capability is required for validation purposes. The live-reg-sets for virtual and physical remain distinct. I explored unifying them but it does not look like there is anything to gain. The representations are different, so the lower level APIs will remain distinct. Even if we have a wrapper struct, it will only increase the indirection level for every access to the live-reg object. https://github.com/llvm/llvm-project/pull/184275 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff origin/main HEAD --extensions cpp,h --
llvm/lib/Target/AMDGPU/GCNIterativeScheduler.cpp
llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
llvm/lib/Target/AMDGPU/GCNRegPressure.h
llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp --diff_from_common_commit
``
:warning:
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing `origin/main` to the base branch/commit you want to compare against.
:warning:
View the diff from clang-format here.
``diff
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
index 95baeb1e0..39b7cd972 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
@@ -732,10 +732,9 @@ void GCNUpwardRPTracker::recede(const MachineInstr &MI) {
auto PhysPressure = constructPhysRegPressure();
assert(CurPressure == VirtPressure + PhysPressure ||
(dbgs() << "Pressure mismatch in recede()\nMI: " << MI
- << "Tracked: " << print(CurPressure)
- << "Expected: " << print(VirtPressure + PhysPressure)
- << "Virt: " << print(VirtPressure)
- << "Phys: " << print(PhysPressure),
+ << "Tracked: " << print(CurPressure) << "Expected: "
+ << print(VirtPressure + PhysPressure) << "Virt: "
+ << print(VirtPressure) << "Phys: " << print(PhysPressure),
false));
}
``
https://github.com/llvm/llvm-project/pull/184275
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
https://github.com/dhruvachak edited https://github.com/llvm/llvm-project/pull/184275 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)
https://github.com/dhruvachak edited https://github.com/llvm/llvm-project/pull/184275 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
