[llvm] [AMDGPU] Fix speculative register pressure queries (PR #208574)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 14 09:55:21 PDT 2026
https://github.com/ilia-cher updated https://github.com/llvm/llvm-project/pull/208574
>From be75e4ccd41655ccb8af5cca927e1cddb60f1c20 Mon Sep 17 00:00:00 2001
From: Ilya Chernyavsky <ichernia at amd.com>
Date: Thu, 9 Jul 2026 22:07:08 +0000
Subject: [PATCH 1/7] [AMDGPU] Fix speculative register pressure queries
There are two issues with the way we currently speculate register pressure:
1. GCNDownwardRPTracker::advance(with UseInternalIterator=false), which is called by the scheduler in schedNode, resets LastTrackedMI, so the tracker (bumpDownwardPressure) doesn't know where the last scheduled instruction is and falls back to the beginning of the basic block. As a result, when we estimate RP impact for a given MI, we tend to find uses that are often already scheduled and should be skipped.
2. When looking for the remaining uses between LastTrackedMI and the candidate MI we should skip already scheduled instruction.
Test:
```
ninja AMDGPUTests
./unittests/Target/AMDGPU/AMDGPUTests --gtest_filter='*AMDGPU*:*BumpDownward*'
```
---
llvm/lib/Target/AMDGPU/GCNRegPressure.cpp | 10 +++-
.../Target/AMDGPU/GCNRegPressureTest.cpp | 57 +++++++++++++++++++
2 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
index 42a430da0acb8..d7d343d5b4264 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
@@ -767,8 +767,11 @@ bool GCNDownwardRPTracker::advance(MachineInstr *MI, bool UseInternalIterator) {
advanceBeforeNext(MI, UseInternalIterator);
advanceToNext(MI, UseInternalIterator);
if (!UseInternalIterator) {
+ auto *SavedLastTrackedMI = LastTrackedMI;
// We must remove any dead def lanes from the current RP
advanceBeforeNext(MI, true);
+ // Restore LastTrackedMI set by advanceToNext
+ LastTrackedMI = SavedLastTrackedMI;
}
return true;
}
@@ -841,8 +844,11 @@ GCNDownwardRPTracker::bumpDownwardPressure(const MachineInstr *MI,
// to be bottom-scheduled to avoid searching uses at each query.
SlotIndex CurrIdx;
const MachineBasicBlock *MBB = MI->getParent();
- MachineBasicBlock::const_iterator IdxPos = skipDebugInstructionsForward(
- LastTrackedMI ? LastTrackedMI : MBB->begin(), MBB->end());
+ MachineBasicBlock::const_iterator StartPos =
+ LastTrackedMI ? std::next(MachineBasicBlock::const_iterator(LastTrackedMI))
+ : MBB->begin();
+ MachineBasicBlock::const_iterator IdxPos =
+ skipDebugInstructionsForward(StartPos, MBB->end());
if (IdxPos == MBB->end()) {
CurrIdx = LIS.getMBBEndIdx(MBB);
} else {
diff --git a/llvm/unittests/Target/AMDGPU/GCNRegPressureTest.cpp b/llvm/unittests/Target/AMDGPU/GCNRegPressureTest.cpp
index d907ee269a448..d536a5dead50e 100644
--- a/llvm/unittests/Target/AMDGPU/GCNRegPressureTest.cpp
+++ b/llvm/unittests/Target/AMDGPU/GCNRegPressureTest.cpp
@@ -8,6 +8,8 @@
#include "GCNRegPressure.h"
#include "AMDGPUUnitTests.h"
+#include "GCNSubtarget.h"
+#include "SIRegisterInfo.h"
#include "llvm/CodeGen/LiveIntervals.h"
#include "llvm/CodeGen/MIRParser/MIRParser.h"
#include "llvm/CodeGen/MachineFunctionAnalysis.h"
@@ -145,3 +147,58 @@ body: |
EXPECT_EQ(RPTracker.moveMaxPressure().getVGPRNum(false), 1U);
EXPECT_EQ(RPTrackerNoLiveIns.moveMaxPressure().getVGPRNum(false), 1U);
}
+
+// Tests the correct handling of multiple uses of the same virtual register
+// in bumpDownwardPressure (speculative estimate of register pressure).
+TEST_F(GCNRegPressureTest, BumpDownwardPressureLastUseAfterCommit) {
+ StringRef MIR = R"(
+name: BumpDownwardPressureLastUseAfterCommit
+tracksRegLiveness: true
+machineFunctionInfo:
+ isEntryFunction: true
+body: |
+ bb.0:
+ %0:vgpr_32 = IMPLICIT_DEF
+ %1:vreg_256_align2 = IMPLICIT_DEF
+ S_NOP 0, implicit %1
+ S_NOP 0, implicit %1
+ S_NOP 0, implicit %0
+ S_ENDPGM 0
+...
+)";
+ EXPECT_TRUE(parseMIR(MIR));
+ MachineFunction &MF = getMF("BumpDownwardPressureLastUseAfterCommit");
+ const LiveIntervals &LIS = MFAM.getResult<LiveIntervalsAnalysis>(MF);
+ const MachineRegisterInfo &MRI = MF.getRegInfo();
+ const SIRegisterInfo *TRI = MF.getSubtarget<GCNSubtarget>().getRegisterInfo();
+
+ MachineBasicBlock &MBB = *MF.getBlockNumbered(0);
+
+ SmallVector<MachineInstr *, 8> Instrs;
+ for (MachineInstr &MI : MBB)
+ Instrs.push_back(&MI);
+ // 0: def %0, 1: def %1, 2: U1 (use %1), 3: U2 (last use %1),
+ // 4: use %0, 5: S_ENDPGM
+ MachineInstr *DefV0 = Instrs[0];
+ MachineInstr *DefV1 = Instrs[1];
+ MachineInstr *U1 = Instrs[2];
+ MachineInstr *U2 = Instrs[3];
+
+ GCNDownwardRPTracker RPTracker(LIS);
+ GCNRPTracker::LiveRegSet Empty;
+ RPTracker.reset(MRI, Empty);
+
+ // Commit the defs and the first use of %1 via the externally-managed
+ // iterator (same as while scheduling).
+ RPTracker.advance(DefV0, /*UseInternalIterator=*/false);
+ RPTracker.advance(DefV1, /*UseInternalIterator=*/false);
+ RPTracker.advance(U1, /*UseInternalIterator=*/false);
+
+ // After committing U1, both %0 (1 VGPR) and %1 (vreg_256 = 8 VGPRs) are live.
+ EXPECT_EQ(RPTracker.getPressure().getArchVGPRNum(), 9U);
+
+ // Speculate the last use of %1. %1 must die here, dropping its 8 VGPRs and
+ // leaving only %0 live.
+ GCNRegPressure P = RPTracker.bumpDownwardPressure(U2, TRI);
+ EXPECT_EQ(P.getArchVGPRNum(), 1U);
+}
>From 799e857d08fc9badb3a7f3be3162a383d0cdbac4 Mon Sep 17 00:00:00 2001
From: Ilya Chernyavsky <ichernia at amd.com>
Date: Fri, 10 Jul 2026 00:03:23 +0000
Subject: [PATCH 2/7] (clang-format)
---
llvm/lib/Target/AMDGPU/GCNRegPressure.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
index d7d343d5b4264..ea9334fc1c67b 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
@@ -845,8 +845,9 @@ GCNDownwardRPTracker::bumpDownwardPressure(const MachineInstr *MI,
SlotIndex CurrIdx;
const MachineBasicBlock *MBB = MI->getParent();
MachineBasicBlock::const_iterator StartPos =
- LastTrackedMI ? std::next(MachineBasicBlock::const_iterator(LastTrackedMI))
- : MBB->begin();
+ LastTrackedMI
+ ? std::next(MachineBasicBlock::const_iterator(LastTrackedMI))
+ : MBB->begin();
MachineBasicBlock::const_iterator IdxPos =
skipDebugInstructionsForward(StartPos, MBB->end());
if (IdxPos == MBB->end()) {
>From e60f93f920dd9d371977b40d0e4d7701aadc501f Mon Sep 17 00:00:00 2001
From: Ilya Chernyavsky <ichernia at amd.com>
Date: Fri, 10 Jul 2026 16:22:48 +0000
Subject: [PATCH 3/7] (nit)
---
llvm/lib/Target/AMDGPU/GCNRegPressure.cpp | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
index ea9334fc1c67b..311839b3abdba 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
@@ -767,10 +767,13 @@ bool GCNDownwardRPTracker::advance(MachineInstr *MI, bool UseInternalIterator) {
advanceBeforeNext(MI, UseInternalIterator);
advanceToNext(MI, UseInternalIterator);
if (!UseInternalIterator) {
- auto *SavedLastTrackedMI = LastTrackedMI;
+ const MachineInstr *SavedLastTrackedMI = LastTrackedMI;
// We must remove any dead def lanes from the current RP
advanceBeforeNext(MI, true);
- // Restore LastTrackedMI set by advanceToNext
+ // Restore LastTrackedMI set by advanceToNext, otherwise
+ // speculative queries (bumpDownwardPressure) don't
+ // know the last scheduled instruction and fail to
+ // correctly estimate pressure change.
LastTrackedMI = SavedLastTrackedMI;
}
return true;
>From a5fccf4f0112c5e890fe79e0ec1a781475845653 Mon Sep 17 00:00:00 2001
From: Ilya Chernyavsky <ichernia at amd.com>
Date: Fri, 10 Jul 2026 18:48:41 +0000
Subject: [PATCH 4/7] (upd test)
---
llvm/unittests/Target/AMDGPU/GCNRegPressureTest.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/llvm/unittests/Target/AMDGPU/GCNRegPressureTest.cpp b/llvm/unittests/Target/AMDGPU/GCNRegPressureTest.cpp
index d536a5dead50e..4a8ad2f44fbb4 100644
--- a/llvm/unittests/Target/AMDGPU/GCNRegPressureTest.cpp
+++ b/llvm/unittests/Target/AMDGPU/GCNRegPressureTest.cpp
@@ -154,8 +154,6 @@ TEST_F(GCNRegPressureTest, BumpDownwardPressureLastUseAfterCommit) {
StringRef MIR = R"(
name: BumpDownwardPressureLastUseAfterCommit
tracksRegLiveness: true
-machineFunctionInfo:
- isEntryFunction: true
body: |
bb.0:
%0:vgpr_32 = IMPLICIT_DEF
@@ -166,7 +164,7 @@ body: |
S_ENDPGM 0
...
)";
- EXPECT_TRUE(parseMIR(MIR));
+ ASSERT_TRUE(parseMIR(MIR));
MachineFunction &MF = getMF("BumpDownwardPressureLastUseAfterCommit");
const LiveIntervals &LIS = MFAM.getResult<LiveIntervalsAnalysis>(MF);
const MachineRegisterInfo &MRI = MF.getRegInfo();
>From dea971a4e786fe99f1f2326909a43c0149ff5f92 Mon Sep 17 00:00:00 2001
From: Ilya Chernyavsky <ichernia at amd.com>
Date: Mon, 13 Jul 2026 17:45:58 +0000
Subject: [PATCH 5/7] (use getIterator)
---
llvm/lib/Target/AMDGPU/GCNRegPressure.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
index 311839b3abdba..c77a6e37e933f 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
@@ -849,7 +849,7 @@ GCNDownwardRPTracker::bumpDownwardPressure(const MachineInstr *MI,
const MachineBasicBlock *MBB = MI->getParent();
MachineBasicBlock::const_iterator StartPos =
LastTrackedMI
- ? std::next(MachineBasicBlock::const_iterator(LastTrackedMI))
+ ? std::next(LastTrackedMI->getIterator())
: MBB->begin();
MachineBasicBlock::const_iterator IdxPos =
skipDebugInstructionsForward(StartPos, MBB->end());
>From bcdeffc41cf201b7bb05a2f65b6da244c1dc0499 Mon Sep 17 00:00:00 2001
From: Ilya Chernyavsky <ichernia at amd.com>
Date: Mon, 13 Jul 2026 20:49:41 +0000
Subject: [PATCH 6/7] (clang-format)
---
llvm/lib/Target/AMDGPU/GCNRegPressure.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
index c77a6e37e933f..a717af3c6527f 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
@@ -848,9 +848,7 @@ GCNDownwardRPTracker::bumpDownwardPressure(const MachineInstr *MI,
SlotIndex CurrIdx;
const MachineBasicBlock *MBB = MI->getParent();
MachineBasicBlock::const_iterator StartPos =
- LastTrackedMI
- ? std::next(LastTrackedMI->getIterator())
- : MBB->begin();
+ LastTrackedMI ? std::next(LastTrackedMI->getIterator()) : MBB->begin();
MachineBasicBlock::const_iterator IdxPos =
skipDebugInstructionsForward(StartPos, MBB->end());
if (IdxPos == MBB->end()) {
>From 853c756c3890105493683ec4f293828e3bb9d12a Mon Sep 17 00:00:00 2001
From: Ilya Chernyavsky <ichernia at amd.com>
Date: Tue, 14 Jul 2026 16:49:17 +0000
Subject: [PATCH 7/7] (hoist code)
---
llvm/lib/Target/AMDGPU/GCNRegPressure.cpp | 24 +++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
index a717af3c6527f..f4f03a7f1430b 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.cpp
@@ -827,6 +827,18 @@ GCNDownwardRPTracker::bumpDownwardPressure(const MachineInstr *MI,
SlotIndex SlotIdx;
SlotIdx = LIS.getInstructionIndex(*MI).getRegSlot();
+ SlotIndex CurrIdx;
+ const MachineBasicBlock *MBB = MI->getParent();
+ MachineBasicBlock::const_iterator StartPos =
+ LastTrackedMI ? std::next(LastTrackedMI->getIterator()) : MBB->begin();
+ MachineBasicBlock::const_iterator IdxPos =
+ skipDebugInstructionsForward(StartPos, MBB->end());
+ if (IdxPos == MBB->end()) {
+ CurrIdx = LIS.getMBBEndIdx(MBB);
+ } else {
+ CurrIdx = LIS.getInstructionIndex(*IdxPos).getRegSlot();
+ }
+
// Account for register pressure similar to RegPressureTracker::recede().
RegisterOperands RegOpers;
RegOpers.collect(*MI, *TRI, *MRI, true, /*IgnoreDead=*/false);
@@ -845,18 +857,6 @@ GCNDownwardRPTracker::bumpDownwardPressure(const MachineInstr *MI,
// last uses for the current position.
// FIXME: allow the caller to pass in the list of vreg uses that remain
// to be bottom-scheduled to avoid searching uses at each query.
- SlotIndex CurrIdx;
- const MachineBasicBlock *MBB = MI->getParent();
- MachineBasicBlock::const_iterator StartPos =
- LastTrackedMI ? std::next(LastTrackedMI->getIterator()) : MBB->begin();
- MachineBasicBlock::const_iterator IdxPos =
- skipDebugInstructionsForward(StartPos, MBB->end());
- if (IdxPos == MBB->end()) {
- CurrIdx = LIS.getMBBEndIdx(MBB);
- } else {
- CurrIdx = LIS.getInstructionIndex(*IdxPos).getRegSlot();
- }
-
LastUseMask =
findUseBetween(Reg, LastUseMask, CurrIdx, SlotIdx, *MRI, TRI, &LIS);
if (LastUseMask.none())
More information about the llvm-commits
mailing list