[llvm] [MachineScheduler][AMDGPU] Allow scheduling of single-MI regions (PR #128739)
Lucas Ramirez via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 27 02:21:19 PST 2025
https://github.com/lucas-rami updated https://github.com/llvm/llvm-project/pull/128739
>From aeed954998e9c3c727794c3d715c3b18fb886a9b Mon Sep 17 00:00:00 2001
From: Lucas Ramirez <lucas.rami at proton.me>
Date: Tue, 25 Feb 2025 16:18:44 +0000
Subject: [PATCH 1/5] Allow scheduling of regions with single MI
---
llvm/include/llvm/CodeGen/ScheduleDAGInstrs.h | 8 ++++++++
llvm/lib/CodeGen/MachineScheduler.cpp | 3 ++-
llvm/lib/CodeGen/ScheduleDAGInstrs.cpp | 5 +++--
llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp | 5 ++++-
.../test/CodeGen/AMDGPU/debug-value-scheduler-liveins.mir | 2 ++
5 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/ScheduleDAGInstrs.h b/llvm/include/llvm/CodeGen/ScheduleDAGInstrs.h
index aaa10e684687c..82240745c2772 100644
--- a/llvm/include/llvm/CodeGen/ScheduleDAGInstrs.h
+++ b/llvm/include/llvm/CodeGen/ScheduleDAGInstrs.h
@@ -124,6 +124,9 @@ namespace llvm {
/// rescheduling).
bool RemoveKillFlags;
+ /// True if regions with a single MI should be scheduled.
+ bool ScheduleSingleMIRegions = false;
+
/// The standard DAG builder does not normally include terminators as DAG
/// nodes because it does not create the necessary dependencies to prevent
/// reordering. A specialized scheduler can override
@@ -288,6 +291,11 @@ namespace llvm {
return Topo.IsReachable(SU, TargetSU);
}
+ /// Whether regions with a single MI should be scheduled.
+ bool shouldScheduleSingleMIRegions() const {
+ return ScheduleSingleMIRegions;
+ }
+
/// Returns an iterator to the top of the current scheduling region.
MachineBasicBlock::iterator begin() const { return RegionBegin; }
diff --git a/llvm/lib/CodeGen/MachineScheduler.cpp b/llvm/lib/CodeGen/MachineScheduler.cpp
index 0da7535031a7d..b9903ee832d31 100644
--- a/llvm/lib/CodeGen/MachineScheduler.cpp
+++ b/llvm/lib/CodeGen/MachineScheduler.cpp
@@ -770,6 +770,7 @@ void MachineSchedulerBase::scheduleRegions(ScheduleDAGInstrs &Scheduler,
MBBRegionsVector MBBRegions;
getSchedRegions(&*MBB, MBBRegions, Scheduler.doMBBSchedRegionsTopDown());
+ bool ScheduleSingleMI = Scheduler.shouldScheduleSingleMIRegions();
for (const SchedRegion &R : MBBRegions) {
MachineBasicBlock::iterator I = R.RegionBegin;
MachineBasicBlock::iterator RegionEnd = R.RegionEnd;
@@ -780,7 +781,7 @@ void MachineSchedulerBase::scheduleRegions(ScheduleDAGInstrs &Scheduler,
Scheduler.enterRegion(&*MBB, I, RegionEnd, NumRegionInstrs);
// Skip empty scheduling regions (0 or 1 schedulable instructions).
- if (I == RegionEnd || I == std::prev(RegionEnd)) {
+ if (I == RegionEnd || (!ScheduleSingleMI && I == std::prev(RegionEnd))) {
// Close the current region. Bundle the terminator if needed.
// This invalidates 'RegionEnd' and 'I'.
Scheduler.exitRegion();
diff --git a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
index a26804707dd1f..cd652659dfdef 100644
--- a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
+++ b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
@@ -116,8 +116,9 @@ ScheduleDAGInstrs::ScheduleDAGInstrs(MachineFunction &mf,
bool RemoveKillFlags)
: ScheduleDAG(mf), MLI(mli), MFI(mf.getFrameInfo()),
RemoveKillFlags(RemoveKillFlags),
- UnknownValue(UndefValue::get(
- Type::getVoidTy(mf.getFunction().getContext()))), Topo(SUnits, &ExitSU) {
+ UnknownValue(
+ UndefValue::get(Type::getVoidTy(mf.getFunction().getContext()))),
+ Topo(SUnits, &ExitSU) {
DbgValues.clear();
const TargetSubtargetInfo &ST = mf.getSubtarget();
diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
index 176586e3fbbb6..dbab18b7ae46f 100644
--- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
@@ -759,7 +759,10 @@ GCNScheduleDAGMILive::GCNScheduleDAGMILive(
MFI(*MF.getInfo<SIMachineFunctionInfo>()),
StartingOccupancy(MFI.getOccupancy()), MinOccupancy(StartingOccupancy),
RegionLiveOuts(this, /*IsLiveOut=*/true) {
-
+ // We want regions with a single MI to be scheduled so that we can reason on
+ // them correctlt during scheduling stages that move MIs between regions (e.g.
+ // rematerialization).
+ ScheduleSingleMIRegions = true;
LLVM_DEBUG(dbgs() << "Starting occupancy is " << StartingOccupancy << ".\n");
if (RelaxedOcc) {
MinOccupancy = std::min(MFI.getMinAllowedOccupancy(), StartingOccupancy);
diff --git a/llvm/test/CodeGen/AMDGPU/debug-value-scheduler-liveins.mir b/llvm/test/CodeGen/AMDGPU/debug-value-scheduler-liveins.mir
index d415346b49b28..2a08c52e447ba 100644
--- a/llvm/test/CodeGen/AMDGPU/debug-value-scheduler-liveins.mir
+++ b/llvm/test/CodeGen/AMDGPU/debug-value-scheduler-liveins.mir
@@ -2,6 +2,8 @@
# RUN: llc -mtriple=amdgcn -mcpu=gfx908 -passes=machine-scheduler %s -o - -debug-only=machine-scheduler 2>&1 | FileCheck %s
# REQUIRES: asserts
+# CHECK: ********** MI Scheduling **********
+# CHECK-NEXT: test_get_liveins:%bb.0
# CHECK: ********** MI Scheduling **********
# CHECK-NEXT: test_get_liveins:%bb.1
# CHECK: Region live-in pressure: VGPRs: 1 AGPRs: 0, SGPRs: 0, LVGPR WT: 0, LSGPR WT: 0
>From 1d43cbf8c167fbf7bac7cf1d6c4dbe720dfa633f Mon Sep 17 00:00:00 2001
From: Lucas Ramirez <lucas.rami at proton.me>
Date: Tue, 25 Feb 2025 16:38:54 +0000
Subject: [PATCH 2/5] Revert spurious formatting change
---
llvm/lib/CodeGen/ScheduleDAGInstrs.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
index cd652659dfdef..a26804707dd1f 100644
--- a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
+++ b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp
@@ -116,9 +116,8 @@ ScheduleDAGInstrs::ScheduleDAGInstrs(MachineFunction &mf,
bool RemoveKillFlags)
: ScheduleDAG(mf), MLI(mli), MFI(mf.getFrameInfo()),
RemoveKillFlags(RemoveKillFlags),
- UnknownValue(
- UndefValue::get(Type::getVoidTy(mf.getFunction().getContext()))),
- Topo(SUnits, &ExitSU) {
+ UnknownValue(UndefValue::get(
+ Type::getVoidTy(mf.getFunction().getContext()))), Topo(SUnits, &ExitSU) {
DbgValues.clear();
const TargetSubtargetInfo &ST = mf.getSubtarget();
>From 252bce60e4733b090eb9816425e42e7d6ce7f168 Mon Sep 17 00:00:00 2001
From: Lucas Ramirez <11032120+lucas-rami at users.noreply.github.com>
Date: Tue, 25 Feb 2025 17:41:32 +0100
Subject: [PATCH 3/5] Fix comment typo
Co-authored-by: Jay Foad <jay.foad at gmail.com>
---
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 dbab18b7ae46f..0bd3e8728b2dc 100644
--- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
@@ -760,7 +760,7 @@ GCNScheduleDAGMILive::GCNScheduleDAGMILive(
StartingOccupancy(MFI.getOccupancy()), MinOccupancy(StartingOccupancy),
RegionLiveOuts(this, /*IsLiveOut=*/true) {
// We want regions with a single MI to be scheduled so that we can reason on
- // them correctlt during scheduling stages that move MIs between regions (e.g.
+ // them correctly during scheduling stages that move MIs between regions (e.g.
// rematerialization).
ScheduleSingleMIRegions = true;
LLVM_DEBUG(dbgs() << "Starting occupancy is " << StartingOccupancy << ".\n");
>From db36039c1cde8aab464c5eb72ccf59e010937863 Mon Sep 17 00:00:00 2001
From: Lucas Ramirez <lucas.rami at proton.me>
Date: Wed, 26 Feb 2025 17:42:05 +0000
Subject: [PATCH 4/5] Remove flag; this is now the default scheduler behavior
---
llvm/include/llvm/CodeGen/ScheduleDAGInstrs.h | 8 --------
llvm/lib/CodeGen/MachineScheduler.cpp | 3 +--
llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp | 5 +----
llvm/test/CodeGen/ARM/misched-branch-targets.mir | 11 ++++++-----
llvm/test/CodeGen/PowerPC/pr47155-47156.ll | 2 ++
llvm/test/CodeGen/X86/fake-use-scheduler.mir | 6 ++++++
6 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/ScheduleDAGInstrs.h b/llvm/include/llvm/CodeGen/ScheduleDAGInstrs.h
index 82240745c2772..aaa10e684687c 100644
--- a/llvm/include/llvm/CodeGen/ScheduleDAGInstrs.h
+++ b/llvm/include/llvm/CodeGen/ScheduleDAGInstrs.h
@@ -124,9 +124,6 @@ namespace llvm {
/// rescheduling).
bool RemoveKillFlags;
- /// True if regions with a single MI should be scheduled.
- bool ScheduleSingleMIRegions = false;
-
/// The standard DAG builder does not normally include terminators as DAG
/// nodes because it does not create the necessary dependencies to prevent
/// reordering. A specialized scheduler can override
@@ -291,11 +288,6 @@ namespace llvm {
return Topo.IsReachable(SU, TargetSU);
}
- /// Whether regions with a single MI should be scheduled.
- bool shouldScheduleSingleMIRegions() const {
- return ScheduleSingleMIRegions;
- }
-
/// Returns an iterator to the top of the current scheduling region.
MachineBasicBlock::iterator begin() const { return RegionBegin; }
diff --git a/llvm/lib/CodeGen/MachineScheduler.cpp b/llvm/lib/CodeGen/MachineScheduler.cpp
index b9903ee832d31..6f3a65fe795a9 100644
--- a/llvm/lib/CodeGen/MachineScheduler.cpp
+++ b/llvm/lib/CodeGen/MachineScheduler.cpp
@@ -770,7 +770,6 @@ void MachineSchedulerBase::scheduleRegions(ScheduleDAGInstrs &Scheduler,
MBBRegionsVector MBBRegions;
getSchedRegions(&*MBB, MBBRegions, Scheduler.doMBBSchedRegionsTopDown());
- bool ScheduleSingleMI = Scheduler.shouldScheduleSingleMIRegions();
for (const SchedRegion &R : MBBRegions) {
MachineBasicBlock::iterator I = R.RegionBegin;
MachineBasicBlock::iterator RegionEnd = R.RegionEnd;
@@ -781,7 +780,7 @@ void MachineSchedulerBase::scheduleRegions(ScheduleDAGInstrs &Scheduler,
Scheduler.enterRegion(&*MBB, I, RegionEnd, NumRegionInstrs);
// Skip empty scheduling regions (0 or 1 schedulable instructions).
- if (I == RegionEnd || (!ScheduleSingleMI && I == std::prev(RegionEnd))) {
+ if (I == RegionEnd) {
// Close the current region. Bundle the terminator if needed.
// This invalidates 'RegionEnd' and 'I'.
Scheduler.exitRegion();
diff --git a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
index 0bd3e8728b2dc..176586e3fbbb6 100644
--- a/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp
@@ -759,10 +759,7 @@ GCNScheduleDAGMILive::GCNScheduleDAGMILive(
MFI(*MF.getInfo<SIMachineFunctionInfo>()),
StartingOccupancy(MFI.getOccupancy()), MinOccupancy(StartingOccupancy),
RegionLiveOuts(this, /*IsLiveOut=*/true) {
- // We want regions with a single MI to be scheduled so that we can reason on
- // them correctly during scheduling stages that move MIs between regions (e.g.
- // rematerialization).
- ScheduleSingleMIRegions = true;
+
LLVM_DEBUG(dbgs() << "Starting occupancy is " << StartingOccupancy << ".\n");
if (RelaxedOcc) {
MinOccupancy = std::min(MFI.getMinAllowedOccupancy(), StartingOccupancy);
diff --git a/llvm/test/CodeGen/ARM/misched-branch-targets.mir b/llvm/test/CodeGen/ARM/misched-branch-targets.mir
index 610344f844001..4b27ed93119c9 100644
--- a/llvm/test/CodeGen/ARM/misched-branch-targets.mir
+++ b/llvm/test/CodeGen/ARM/misched-branch-targets.mir
@@ -1,7 +1,7 @@
-# RUN: llc -o - -run-pass=machine-scheduler -misched=shuffle %s | FileCheck %s
-# RUN: llc -o - -passes=machine-scheduler -misched=shuffle %s | FileCheck %s
-# RUN: llc -o - -run-pass=postmisched %s | FileCheck %s
-# RUN: llc -o - -passes=postmisched %s | FileCheck %s
+# RUN: llc -o - -run-pass=machine-scheduler -misched=shuffle %s | FileCheck -check-prefixes=CHECK,CHECK-MISCHED %s
+# RUN: llc -o - -passes=machine-scheduler -misched=shuffle %s | FileCheck -check-prefixes=CHECK,CHECK-MISCHED %s
+# RUN: llc -o - -run-pass=postmisched %s | FileCheck -check-prefixes=CHECK,CHECK-POSTMISCHED %s
+# RUN: llc -o - -passes=postmisched %s | FileCheck -check-prefixes=CHECK,CHECK-POSTMISCHED %s
# REQUIRES: asserts
# -misched=shuffle is only available with assertions enabled
@@ -147,7 +147,8 @@ body: |
# CHECK-LABEL: name: foo_setjmp
# CHECK: body:
-# CHECK: tBL 14 /* CC::al */, $noreg, @setjmp, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit-def $sp, implicit-def $r0
+# CHECK-MISCHED: tBL 14 /* CC::al */, $noreg, @setjmp, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit $r0, implicit-def $sp, implicit-def $r0
+# CHECK-POSTMISCHED: tBL 14 /* CC::al */, $noreg, @setjmp, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit-def $sp, implicit-def $r0
# CHECK-NEXT: t2BTI
---
diff --git a/llvm/test/CodeGen/PowerPC/pr47155-47156.ll b/llvm/test/CodeGen/PowerPC/pr47155-47156.ll
index 02f287634578a..1fb3607eb05c3 100644
--- a/llvm/test/CodeGen/PowerPC/pr47155-47156.ll
+++ b/llvm/test/CodeGen/PowerPC/pr47155-47156.ll
@@ -4,6 +4,7 @@
; RUN: -stop-after=postmisched -debug-only=machine-scheduler 2>&1 >/dev/null | FileCheck %s
define void @pr47155() {
+; CHECK-LABEL: Machine code for function pr47155
; CHECK: *** Final schedule for %bb.0 ***
; CHECK: ********** MI Scheduling **********
; CHECK-NEXT: pr47155:%bb.0 entry
@@ -23,6 +24,7 @@ entry:
}
define void @pr47156(ptr %fn) {
+; CHECK-LABEL: Machine code for function pr47156
; CHECK: *** Final schedule for %bb.0 ***
; CHECK: ********** MI Scheduling **********
; CHECK-NEXT: pr47156:%bb.0 entry
diff --git a/llvm/test/CodeGen/X86/fake-use-scheduler.mir b/llvm/test/CodeGen/X86/fake-use-scheduler.mir
index 8b82c4ed2485d..20c9ffec61383 100644
--- a/llvm/test/CodeGen/X86/fake-use-scheduler.mir
+++ b/llvm/test/CodeGen/X86/fake-use-scheduler.mir
@@ -9,6 +9,12 @@
#
# CHECK: ********** MI Scheduling **********
# CHECK-NEXT: foo:%bb.0 entry
+# CHECK-NEXT: From: $rax = COPY %5:gr64
+# CHECK-NEXT: To: RET 0, killed $rax
+# CHECK-NEXT: RegionInstrs: 1
+#
+# CHECK: ********** MI Scheduling **********
+# CHECK-NEXT: foo:%bb.0 entry
# CHECK-NEXT: From: %0:gr64 = COPY $rdi
# CHECK-NEXT: To: FAKE_USE %5:gr64
# CHECK-NEXT: RegionInstrs: 7
>From 0f41fe8f360507c52b4624020c8f0172defd9574 Mon Sep 17 00:00:00 2001
From: Lucas Ramirez <lucas.rami at proton.me>
Date: Thu, 27 Feb 2025 10:20:51 +0000
Subject: [PATCH 5/5] Update comment in machine scheduler
---
llvm/lib/CodeGen/MachineScheduler.cpp | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/MachineScheduler.cpp b/llvm/lib/CodeGen/MachineScheduler.cpp
index 6f3a65fe795a9..b199dbb78fbb9 100644
--- a/llvm/lib/CodeGen/MachineScheduler.cpp
+++ b/llvm/lib/CodeGen/MachineScheduler.cpp
@@ -779,7 +779,10 @@ void MachineSchedulerBase::scheduleRegions(ScheduleDAGInstrs &Scheduler,
// it. Perhaps it still needs to be bundled.
Scheduler.enterRegion(&*MBB, I, RegionEnd, NumRegionInstrs);
- // Skip empty scheduling regions (0 or 1 schedulable instructions).
+ // Skip empty scheduling regions but include single-MI regions; we want
+ // those to be scheduled so that backends which move MIs across regions
+ // during scheduling can reason about and schedule those regions
+ // correctly.
if (I == RegionEnd) {
// Close the current region. Bundle the terminator if needed.
// This invalidates 'RegionEnd' and 'I'.
More information about the llvm-commits
mailing list