[llvm] [AMDGPU][Scheduler] Simplify scheduling revert logic (PR #177203)
Lucas Ramirez via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 30 10:02:16 PST 2026
https://github.com/lucas-rami updated https://github.com/llvm/llvm-project/pull/177203
>From baf1d4609e8bd8a00623a7424af2a829aed5828c Mon Sep 17 00:00:00 2001
From: Lucas Ramirez <lucas.rami at proton.me>
Date: Fri, 30 Jan 2026 16:48:59 +0000
Subject: [PATCH 1/3] Squashed changes
---
llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp | 93 +++++++++----------
llvm/lib/Target/AMDGPU/GCNSchedStrategy.h | 8 +-
.../CodeGen/AMDGPU/debug-value-scheduler.mir | 4 +-
.../CodeGen/AMDGPU/sema-v-unsched-bundle.ll | 2 +-
4 files changed, 52 insertions(+), 55 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
index 09ba89c1eec58..5a4623e850020 100644
--- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
@@ -1655,10 +1655,12 @@ void GCNSchedStage::checkScheduling() {
// Revert if this region's schedule would cause a drop in occupancy or
// spilling.
- if (shouldRevertScheduling(WavesAfter))
- revertScheduling();
- else
+ if (shouldRevertScheduling(WavesAfter)) {
+ modifyRegionSchedule(RegionIdx, DAG.BB, Unsched);
+ std::tie(DAG.RegionBegin, DAG.RegionEnd) = DAG.Regions[RegionIdx];
+ } else {
DAG.Pressure[RegionIdx] = PressureAfter;
+ }
}
unsigned
@@ -1886,73 +1888,64 @@ bool GCNSchedStage::mayCauseSpilling(unsigned WavesAfter) {
return false;
}
-void GCNSchedStage::revertScheduling() {
- LLVM_DEBUG(dbgs() << "Attempting to revert scheduling.\n");
- DAG.RegionEnd = DAG.RegionBegin;
- int SkippedDebugInstr = 0;
- for (MachineInstr *MI : Unsched) {
- if (MI->isDebugInstr()) {
- ++SkippedDebugInstr;
- continue;
- }
+void GCNSchedStage::modifyRegionSchedule(unsigned RegionIdx,
+ MachineBasicBlock *MBB,
+ ArrayRef<MachineInstr *> MIOrder) {
+ assert(std::distance(DAG.Regions[RegionIdx].first,
+ DAG.Regions[RegionIdx].second) ==
+ static_cast<long>(MIOrder.size()) &&
+ "instruction number mismatch");
+ if (MIOrder.empty())
+ return;
+
+ LLVM_DEBUG(dbgs() << "Reverting scheduling for region " << RegionIdx << '\n');
+ // Reconstruct MI sequence by moving instructions in desired order before
+ // the current region's start.
+ MachineBasicBlock::iterator RegionEnd = DAG.Regions[RegionIdx].first;
+ for (MachineInstr *MI : MIOrder) {
+ // Either move the next MI in order before the end of the region or move the
+ // region end past the MI if it is at the correct position.
MachineBasicBlock::iterator MII = MI->getIterator();
- if (MII != DAG.RegionEnd) {
+ if (MII != RegionEnd) {
// Will subsequent splice move MI up past a non-debug instruction?
bool NonDebugReordered =
+ !MI->isDebugInstr() &&
skipDebugInstructionsForward(DAG.RegionEnd, MII) != MII;
- DAG.BB->splice(DAG.RegionEnd, DAG.BB, MI);
+ MBB->splice(RegionEnd, MBB, MI);
// Only update LiveIntervals information if non-debug instructions are
// reordered. Otherwise debug instructions could cause code generation to
// change.
if (NonDebugReordered)
DAG.LIS->handleMove(*MI, true);
+ } else {
+ ++RegionEnd;
+ }
+ if (MI->isDebugInstr()) {
+ LLVM_DEBUG(dbgs() << "Scheduling " << *MI);
+ continue;
}
// Reset read-undef flags and update them later.
- for (auto &Op : MI->all_defs())
+ for (MachineOperand &Op : MI->all_defs())
Op.setIsUndef(false);
RegisterOperands RegOpers;
RegOpers.collect(*MI, *DAG.TRI, DAG.MRI, DAG.ShouldTrackLaneMasks, false);
- if (!MI->isDebugInstr()) {
- if (DAG.ShouldTrackLaneMasks) {
- // Adjust liveness and add missing dead+read-undef flags.
- SlotIndex SlotIdx = DAG.LIS->getInstructionIndex(*MI).getRegSlot();
- RegOpers.adjustLaneLiveness(*DAG.LIS, DAG.MRI, SlotIdx, MI);
- } else {
- // Adjust for missing dead-def flags.
- RegOpers.detectDeadDefs(*MI, *DAG.LIS);
- }
+ if (DAG.ShouldTrackLaneMasks) {
+ // Adjust liveness and add missing dead+read-undef flags.
+ SlotIndex SlotIdx = DAG.LIS->getInstructionIndex(*MI).getRegSlot();
+ RegOpers.adjustLaneLiveness(*DAG.LIS, DAG.MRI, SlotIdx, MI);
+ } else {
+ // Adjust for missing dead-def flags.
+ RegOpers.detectDeadDefs(*MI, *DAG.LIS);
}
- DAG.RegionEnd = MI->getIterator();
- ++DAG.RegionEnd;
LLVM_DEBUG(dbgs() << "Scheduling " << *MI);
}
- // After reverting schedule, debug instrs will now be at the end of the block
- // and RegionEnd will point to the first debug instr. Increment RegionEnd
- // pass debug instrs to the actual end of the scheduling region.
- while (SkippedDebugInstr-- > 0)
- ++DAG.RegionEnd;
-
- // If Unsched.front() instruction is a debug instruction, this will actually
- // shrink the region since we moved all debug instructions to the end of the
- // block. Find the first instruction that is not a debug instruction.
- DAG.RegionBegin = Unsched.front()->getIterator();
- if (DAG.RegionBegin->isDebugInstr()) {
- for (MachineInstr *MI : Unsched) {
- if (MI->isDebugInstr())
- continue;
- DAG.RegionBegin = MI->getIterator();
- break;
- }
- }
-
- // Then move the debug instructions back into their correct place and set
- // RegionBegin and RegionEnd if needed.
- DAG.placeDebugValues();
-
- DAG.Regions[RegionIdx] = std::pair(DAG.RegionBegin, DAG.RegionEnd);
+ // The region end doesn't change throughout scheduling since it itself is
+ // outside the region (whether that is a MBB end or a terminator MI).
+ assert(RegionEnd == DAG.Regions[RegionIdx].second && "region end mismatch");
+ DAG.Regions[RegionIdx].first = MIOrder.front();
}
bool RewriteMFMAFormStage::isRewriteCandidate(MachineInstr *MI) const {
diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.h b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.h
index 07c972517598b..dc85fef958c9a 100644
--- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.h
+++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.h
@@ -402,8 +402,12 @@ class GCNSchedStage {
// Returns true if the new schedule may result in more spilling.
bool mayCauseSpilling(unsigned WavesAfter);
- // Attempt to revert scheduling for this region.
- void revertScheduling();
+ /// Sets the schedule of region \p RegionIdx in block \p MBB to \p MIOrder.
+ /// The MIs in \p MIOrder must be exactly the same as the ones currently
+ /// existing inside the region, only in a different order that honors def-use
+ /// chains.
+ void modifyRegionSchedule(unsigned RegionIdx, MachineBasicBlock *MBB,
+ ArrayRef<MachineInstr *> MIOrder);
void advanceRegion() { RegionIdx++; }
diff --git a/llvm/test/CodeGen/AMDGPU/debug-value-scheduler.mir b/llvm/test/CodeGen/AMDGPU/debug-value-scheduler.mir
index 170672dc4af64..c02b38ebcfdce 100644
--- a/llvm/test/CodeGen/AMDGPU/debug-value-scheduler.mir
+++ b/llvm/test/CodeGen/AMDGPU/debug-value-scheduler.mir
@@ -8,13 +8,13 @@
# CHECK-NEXT: From: DBG_VALUE %17:vgpr_32, 0, 0
# CHECK-NEXT: To: S_ENDPGM 0, implicit %69:vgpr_32, implicit %70:vgpr_32
# CHECK-NEXT: RegionInstrs: 46
-# CHECK: Attempting to revert scheduling.
+# CHECK: Reverting scheduling for region 1
# CHECK: ********** MI Scheduling **********
# CHECK: test_same_num_instrs:%bb.2
# CHECK-NEXT: From: DBG_VALUE %17:vgpr_32, 0, 0
# CHECK-NEXT: To: S_ENDPGM 0, implicit %69:vgpr_32, implicit %70:vgpr_32
# CHECK-NEXT: RegionInstrs: 46
-# CHECK: Attempting to revert scheduling.
+# CHECK: Reverting scheduling for region 1
---
name: test_same_num_instrs
diff --git a/llvm/test/CodeGen/AMDGPU/sema-v-unsched-bundle.ll b/llvm/test/CodeGen/AMDGPU/sema-v-unsched-bundle.ll
index 5ff2f24d294dc..295075266271a 100644
--- a/llvm/test/CodeGen/AMDGPU/sema-v-unsched-bundle.ll
+++ b/llvm/test/CodeGen/AMDGPU/sema-v-unsched-bundle.ll
@@ -1,7 +1,7 @@
; REQUIRES: asserts
; RUN: llc -mtriple=amdgcn -O1 -mcpu=gfx90a -debug-only=machine-scheduler -filetype=null < %s 2>&1 | FileCheck --check-prefix=DEBUG %s
-; DEBUG: Attempting to revert scheduling.
+; DEBUG: Reverting scheduling for region 0
@G = global <32 x i8> splat (i8 1)
@G.1 = global <32 x i8> splat (i8 127)
>From 494079a4c22992449109f021aab9b7a07ff95d58 Mon Sep 17 00:00:00 2001
From: Lucas Ramirez <lucas.rami at proton.me>
Date: Fri, 30 Jan 2026 16:49:38 +0000
Subject: [PATCH 2/3] Cast to wider type
---
llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
index 5a4623e850020..949ea6ad661bc 100644
--- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
@@ -1891,9 +1891,9 @@ bool GCNSchedStage::mayCauseSpilling(unsigned WavesAfter) {
void GCNSchedStage::modifyRegionSchedule(unsigned RegionIdx,
MachineBasicBlock *MBB,
ArrayRef<MachineInstr *> MIOrder) {
- assert(std::distance(DAG.Regions[RegionIdx].first,
- DAG.Regions[RegionIdx].second) ==
- static_cast<long>(MIOrder.size()) &&
+ assert(static_cast<size_t>(std::distance(DAG.Regions[RegionIdx].first,
+ DAG.Regions[RegionIdx].second)) ==
+ MIOrder.size() &&
"instruction number mismatch");
if (MIOrder.empty())
return;
>From fb1c7459575f9f6f826963052c5746834e646827 Mon Sep 17 00:00:00 2001
From: Lucas Ramirez <lucas.rami at proton.me>
Date: Fri, 30 Jan 2026 18:02:01 +0000
Subject: [PATCH 3/3] Fix typo in code coming from main
---
llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
index 949ea6ad661bc..8d8a1e67a5956 100644
--- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
@@ -1911,7 +1911,7 @@ void GCNSchedStage::modifyRegionSchedule(unsigned RegionIdx,
// Will subsequent splice move MI up past a non-debug instruction?
bool NonDebugReordered =
!MI->isDebugInstr() &&
- skipDebugInstructionsForward(DAG.RegionEnd, MII) != MII;
+ skipDebugInstructionsForward(RegionEnd, MII) != MII;
MBB->splice(RegionEnd, MBB, MI);
// Only update LiveIntervals information if non-debug instructions are
// reordered. Otherwise debug instructions could cause code generation to
More information about the llvm-commits
mailing list