[AMDGPU] Support function attribute to override postRA scheduling direction #147708

harrisonGPU · 2025-07-09T11:46:48Z

This patch adds support for controlling the post-RA machine scheduler direction
(topdown, bottomup, bidirectional) on a per-function basis using the
"amdgpu-post-ra-direction" function attribute.

llvmbot · 2025-07-09T11:47:21Z

@llvm/pr-subscribers-backend-amdgpu

Author: Harrison Hao (harrisonGPU)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/147708.diff

3 Files Affected:

(modified) llvm/include/llvm/CodeGen/MachineScheduler.h (+1)
(modified) llvm/lib/CodeGen/MachineScheduler.cpp (+1-1)
(modified) llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp (+17)

diff --git a/llvm/include/llvm/CodeGen/MachineScheduler.h b/llvm/include/llvm/CodeGen/MachineScheduler.h
index e7a7091acee64..1635eae758590 100644
--- a/llvm/include/llvm/CodeGen/MachineScheduler.h
+++ b/llvm/include/llvm/CodeGen/MachineScheduler.h
@@ -116,6 +116,7 @@ enum Direction {
 } // namespace MISched
 
 LLVM_ABI extern cl::opt<MISched::Direction> PreRADirection;
+LLVM_ABI extern cl::opt<MISched::Direction> PostRADirection;
 LLVM_ABI extern cl::opt<bool> VerifyScheduling;
 #ifndef NDEBUG
 extern cl::opt<bool> ViewMISchedDAGs;
diff --git a/llvm/lib/CodeGen/MachineScheduler.cpp b/llvm/lib/CodeGen/MachineScheduler.cpp
index 76cba2949af60..72e1fce07f33e 100644
--- a/llvm/lib/CodeGen/MachineScheduler.cpp
+++ b/llvm/lib/CodeGen/MachineScheduler.cpp
@@ -190,7 +190,7 @@ cl::opt<MISched::Direction> PreRADirection(
         clEnumValN(MISched::Bidirectional, "bidirectional",
                    "Force bidirectional pre reg-alloc list scheduling")));
 
-static cl::opt<MISched::Direction> PostRADirection(
+cl::opt<MISched::Direction> PostRADirection(
     "misched-postra-direction", cl::Hidden,
     cl::desc("Post reg-alloc list scheduling direction"),
     cl::init(MISched::Unspecified),
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
index c3536113e9bef..4c46e703b4604 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -1144,6 +1144,23 @@ GCNTargetMachine::createMachineScheduler(MachineSchedContext *C) const {
 
 ScheduleDAGInstrs *
 GCNTargetMachine::createPostMachineScheduler(MachineSchedContext *C) const {
+  Attribute PostRADirectionAttr =
+      C->MF->getFunction().getFnAttribute("misched-postra-direction");
+
+  if (PostRADirectionAttr.isValid()) {
+    StringRef PostRADirectionStr = PostRADirectionAttr.getValueAsString();
+    if (PostRADirectionStr == "topdown")
+      PostRADirection = MISched::TopDown;
+    else if (PostRADirectionStr == "bottomup")
+      PostRADirection = MISched::BottomUp;
+    else if (PostRADirectionStr == "bidirectional")
+      PostRADirection = MISched::Bidirectional;
+    else
+      report_fatal_error(
+          Twine("invalid value for 'misched-postra-direction' attribute: ") +
+          PostRADirectionStr);
+  }
+
   ScheduleDAGMI *DAG =
       new GCNPostScheduleDAGMILive(C, std::make_unique<PostGenericScheduler>(C),
                                    /*RemoveKillFlags=*/true);

arsenm

Missing tests and I thought there already was an attribute for this

llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

shiltian · 2025-07-09T13:39:03Z

I thought there already was an attribute for this

I wonder what the reason of having a function attribute for this

harrisonGPU · 2025-07-09T14:11:40Z

I wonder what the reason of having a function attribute for this

We use function attributes here because in graphics shader compiler, performance tuning often needs to be done per shader stage—not across the whole pipeline.

harrisonGPU · 2025-07-09T14:14:47Z

Missing tests and I thought there already was an attribute for this

No, we don’t have this attribute. I’ve run into this issue, the existing option is a static one, so it applies to the entire pipeline, not to individual shader stages. Also, I’m not sure how to write a proper lit test for this , what exactly should the test check?

perlfu · 2025-07-11T02:53:56Z

llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

Function attributes should not override anything actually set on the command line, i.e. if command line sets an option it should override function attributes.

This mechanism as implemented is too global.
It will change direction of all PostRA schedulers.
Can you revise this change to add some way of setting a direction for specific schedulers directly?

Thanks Carl, I understand your point. So I created a new option called amdgpu-post-ra-direction. The function attribute will no longer override the command-line option.

It's similar to how amdgpu-sched-strategy works:

llvm-project/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

Lines 1122 to 1144 in 43a9ec2

Attribute SchedStrategyAttr =

C->MF->getFunction().getFnAttribute("amdgpu-sched-strategy");

StringRef SchedStrategy = SchedStrategyAttr.isValid()

? SchedStrategyAttr.getValueAsString()

: AMDGPUSchedStrategy;

if (SchedStrategy == "max-ilp")

return createGCNMaxILPMachineScheduler(C);

if (SchedStrategy == "max-memory-clause")

return createGCNMaxMemoryClauseMachineScheduler(C);

if (SchedStrategy == "iterative-ilp")

return createIterativeILPMachineScheduler(C);

if (SchedStrategy == "iterative-minreg")

return createMinRegScheduler(C);

if (SchedStrategy == "iterative-maxocc")

return createIterativeGCNMaxOccupancyMachineScheduler(C);

return createGCNMaxOccupancyMachineScheduler(C);

}

Code changes do not seem to solve either of the two issues I mentioned.

New option AMDGPUPostRADirection is not connected, and would not solve the issue of function attribute overriding the direction of all machine schedulers.
To fix this issue, the direction needs to be passed at scheduler construction or otherwise stored/updated in the scheduler object.
i.e. there needs to be a per-scheduler way of setting direction that doesn't currently exist.

To avoid a function attribute overriding command line options, you should check for presence of command line option before parsing function attribute.
Then ignore the function attribute if a command line option is already present/set.
Again you don't need to do this part if the command line override applies to a per-scheduler direction as implemented above -- this detail can be encapsulated within the MachineScheduler code, then AMDGPU code only need set the desired direction.

Thanks! I noticed that LLVM provides the overridePostRASchedPolicy hook, so I’ve switched to using a target feature to control the post-RA direction. This avoids overriding the direction for all machine schedulers and no longer conflicts with command-line overrides. What do you think about it?

arsenm

Needs tests

arsenm · 2025-07-11T03:25:03Z

llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

Better to emit an LLVMContext warning and ignore the value, plus this needs testing

Thanks Matt. I've already added a DiagnosticInfoOptimizationFailure to emit a warning. I also thought about how to add a lit test for this patch, so I added a dbgs() print for the PostRA direction and checked it using FileCheck. What do you think?

arsenm · 2025-07-13T22:51:07Z

llvm/test/CodeGen/AMDGPU/postra-sched-attribute.ll

Don't need the other attributes

Do you mean we don't need the amdgpu-post-ra-direction attribute?

Other attributes = alwaysinline nounwind memory(readwrite).
You only need the attribute under test.

Thanks, I have updated it. :-)

arsenm

This should absolutely not go into subtarget features. This made more sense as an attribute

perlfu · 2025-07-16T07:03:50Z

If you modify overridePostRASchedPolicy to pass the MachineFunction then this can work using function attribute approach.

harrisonGPU · 2025-07-22T09:17:36Z

I have already used function attribute approach, what do think about?

harrisonGPU · 2025-07-28T09:02:51Z

Ping.

shiltian · 2025-07-31T02:52:20Z

llvm/lib/Target/AMDGPU/GCNSubtarget.cpp

Thanks, I have removed it.

…ection

shiltian

LGTM

shiltian · 2025-07-31T16:25:54Z

llvm/lib/Target/AMDGPU/GCNSubtarget.cpp

Suggested change

Twine("invalid value for postRa direction attribute: '") +

Twine("invalid value for postRA direction attribute: '") +

shiltian · 2025-07-31T16:26:08Z

llvm/test/CodeGen/AMDGPU/postra-sched-attribute.ll

This check misses the actual value of the attribute

I have removed actual value of the attribute. :-)

harrisonGPU requested review from arsenm, jayfoad, nhaehnle, perlfu and shiltian July 9, 2025 11:46

llvmbot added backend:AMDGPU llvm:codegen labels Jul 9, 2025

arsenm reviewed Jul 9, 2025

View reviewed changes

llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp Outdated Show resolved Hide resolved

harrisonGPU requested a review from arsenm July 10, 2025 14:43

perlfu reviewed Jul 11, 2025

View reviewed changes

arsenm reviewed Jul 11, 2025

View reviewed changes

harrisonGPU force-pushed the amdgpu/post-ra-attr branch from 4bcd9a6 to 220345b Compare July 13, 2025 12:55

arsenm reviewed Jul 13, 2025

View reviewed changes

harrisonGPU force-pushed the amdgpu/post-ra-attr branch from 99be971 to db514b4 Compare July 15, 2025 12:01

harrisonGPU changed the title ~~[AMDGPU] Support function attribute to override postRA scheduling direction~~ [AMDGPU] Add post-RA scheduling direction control via target features Jul 15, 2025

arsenm requested changes Jul 15, 2025

View reviewed changes

harrisonGPU mentioned this pull request Jul 18, 2025

[MISched] Use SchedRegion in overrideSchedPolicy and overridePostRASchedPolicy #149297

Merged

harrisonGPU force-pushed the amdgpu/post-ra-attr branch from db514b4 to bb7a6eb Compare July 22, 2025 09:13

harrisonGPU changed the title ~~[AMDGPU] Add post-RA scheduling direction control via target features~~ [AMDGPU] Support function attribute to override postRA scheduling direction Jul 22, 2025

harrisonGPU requested review from arsenm and perlfu July 22, 2025 09:17

shiltian reviewed Jul 31, 2025

View reviewed changes

[AMDGPU] Support function attribute to override postRA scheduling dir…

1757d5e

…ection

harrisonGPU added 8 commits July 31, 2025 12:26

[AMDGPU] Add a new attr and lit test.

81edefe

[AMDGPU] Remove other att.

7058279

[AMDGPU]

d3b7844

[AMDGPU] Add post-RA scheduling direction control via target features

ec3de57

[AMDGPU]

967bce6

[AMDGPU] Use function attribute for post-ra

4c37d70

[AMDGPU] Update

d3a2b63

[AMDGPU] Remove option.

90bcf6b

shiltian approved these changes Jul 31, 2025

View reviewed changes

[AMDGPU] Update for comments.

195ce35

harrisonGPU force-pushed the amdgpu/post-ra-attr branch from bfb5deb to 195ce35 Compare August 1, 2025 02:06

harrisonGPU merged commit f9b258c into llvm:main Aug 1, 2025
13 of 15 checks passed

harrisonGPU deleted the amdgpu/post-ra-attr branch August 1, 2025 08:07

	Attribute SchedStrategyAttr =
	C->MF->getFunction().getFnAttribute("amdgpu-sched-strategy");
	StringRef SchedStrategy = SchedStrategyAttr.isValid()
	? SchedStrategyAttr.getValueAsString()
	: AMDGPUSchedStrategy;

	if (SchedStrategy == "max-ilp")
	return createGCNMaxILPMachineScheduler(C);

	if (SchedStrategy == "max-memory-clause")
	return createGCNMaxMemoryClauseMachineScheduler(C);

	if (SchedStrategy == "iterative-ilp")
	return createIterativeILPMachineScheduler(C);

	if (SchedStrategy == "iterative-minreg")
	return createMinRegScheduler(C);

	if (SchedStrategy == "iterative-maxocc")
	return createIterativeGCNMaxOccupancyMachineScheduler(C);

	return createGCNMaxOccupancyMachineScheduler(C);
	}

	Twine("invalid value for postRa direction attribute: '") +
	Twine("invalid value for postRA direction attribute: '") +

[AMDGPU] Support function attribute to override postRA scheduling direction #147708

[AMDGPU] Support function attribute to override postRA scheduling direction #147708

Uh oh!

Conversation

harrisonGPU commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 9, 2025

Uh oh!

arsenm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shiltian commented Jul 9, 2025

Uh oh!

harrisonGPU commented Jul 9, 2025

Uh oh!

harrisonGPU commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arsenm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harrisonGPU Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arsenm left a comment

Choose a reason for hiding this comment

Uh oh!

perlfu commented Jul 16, 2025

Uh oh!

harrisonGPU commented Jul 22, 2025

Uh oh!

harrisonGPU commented Jul 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shiltian left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

harrisonGPU commented Jul 9, 2025 •

edited

Loading

harrisonGPU commented Jul 9, 2025 •

edited

Loading

harrisonGPU Jul 14, 2025 •

edited

Loading