From 7274ae970d3d627eb12c3cfde64f9d323d6edda3 Mon Sep 17 00:00:00 2001 From: Lucas Ramirez <11032120+lucas-rami@users.noreply.github.com> Date: Sun, 1 Feb 2026 17:55:12 +0100 Subject: [PATCH] [AMDGPU][Scheduler] Simplify scheduling revert logic (#177203) When scheduling must be reverted for a region, the current implementation re-orders non-debug instructions and debug instructions separately; the former in a first pass and the latter in a second pass handled by a generic machine scheduler helper whose state is tied to the current region being scheduled, in turns limiting the revert logic to only work on the active scheduling region. This makes the revert logic work in a single pass for all MIs, and removes the restriction that it works exclusively on the active scheduling region. The latter enables future use cases such as reverting scheduling of multiple regions at once. --- llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp | 95 +++++++++---------- 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, 53 insertions(+), 56 deletions(-) diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp index 09ba89c1eec5..8d8a1e67a595 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 MIOrder) { + assert(static_cast(std::distance(DAG.Regions[RegionIdx].first, + DAG.Regions[RegionIdx].second)) == + 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 = - skipDebugInstructionsForward(DAG.RegionEnd, MII) != MII; - DAG.BB->splice(DAG.RegionEnd, DAG.BB, MI); + !MI->isDebugInstr() && + 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 // 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 07c972517598..dc85fef958c9 100644 --- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.h +++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.h @@ -402,8 +402,12 @@ public: // 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 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 170672dc4af6..c02b38ebcfdc 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 5ff2f24d294d..295075266271 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)