[MISched] Use SchedRegion in overrideSchedPolicy and overridePostRASchedPolicy #149297

harrisonGPU · 2025-07-17T12:24:10Z

This patch updates overrideSchedPolicy and overridePostRASchedPolicy to take a
SchedRegion parameter instead of just NumRegionInstrs. This provides access to both the
instruction range and the parent MachineBasicBlock, which enables looking up function-level
attributes.

With this change, targets can select post-RA scheduling direction per function using a function
attribute. For example:

void overridePostRASchedPolicy(MachineSchedPolicy &Policy,
                               const SchedRegion &Region) const {
  const Function &F = Region.RegionBegin->getMF()->getFunction();
  Attribute Attr = F.getFnAttribute("amdgpu-post-ra-direction");
  ...
}

…heduling direction

llvmbot · 2025-07-17T12:24:44Z

@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-backend-powerpc
@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-backend-risc-v

Author: Harrison Hao (harrisonGPU)

Changes

The current overridePostRASchedPolicy method receives NumRegionInstrs,
but this value is not actually useful when determining post-RA scheduling direction.

To support setting the direction via a function attribute, we replace the unused
argument with MachineFunction, allowing access to per-function metadata.

For example:

void overridePostRASchedPolicy(MachineSchedPolicy &amp;Policy,
const MachineFunction &amp;MF) const {
    const Function &amp;F = MF.getFunction();
    Attribute Attr = F.getFnAttribute("function-attribute");
...
}

This avoids the need to encode scheduling direction in target features and enables
finer-grained, per-shader control over post-RA scheduling.

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

4 Files Affected:

(modified) llvm/include/llvm/CodeGen/TargetSubtargetInfo.h (+1-1)
(modified) llvm/lib/CodeGen/MachineScheduler.cpp (+1-1)
(modified) llvm/lib/Target/RISCV/RISCVSubtarget.cpp (+2-2)
(modified) llvm/lib/Target/RISCV/RISCVSubtarget.h (+2-2)

diff --git a/llvm/include/llvm/CodeGen/TargetSubtargetInfo.h b/llvm/include/llvm/CodeGen/TargetSubtargetInfo.h
index 45e67d80629cb..0e6162aed9df8 100644
--- a/llvm/include/llvm/CodeGen/TargetSubtargetInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetSubtargetInfo.h
@@ -241,7 +241,7 @@ class LLVM_ABI TargetSubtargetInfo : public MCSubtargetInfo {
   /// Note that some options like tracking register pressure won't take effect
   /// in post-ra scheduling.
   virtual void overridePostRASchedPolicy(MachineSchedPolicy &Policy,
-                                         unsigned NumRegionInstrs) const {}
+                                         const MachineFunction &MF) const {}
 
   // Perform target-specific adjustments to the latency of a schedule
   // dependency.
diff --git a/llvm/lib/CodeGen/MachineScheduler.cpp b/llvm/lib/CodeGen/MachineScheduler.cpp
index 76cba2949af60..dfea60a84ad45 100644
--- a/llvm/lib/CodeGen/MachineScheduler.cpp
+++ b/llvm/lib/CodeGen/MachineScheduler.cpp
@@ -4338,7 +4338,7 @@ void PostGenericScheduler::initPolicy(MachineBasicBlock::iterator Begin,
   RegionPolicy.OnlyBottomUp = false;
 
   // Allow the subtarget to override default policy.
-  MF.getSubtarget().overridePostRASchedPolicy(RegionPolicy, NumRegionInstrs);
+  MF.getSubtarget().overridePostRASchedPolicy(RegionPolicy, MF);
 
   // After subtarget overrides, apply command line options.
   if (PostRADirection == MISched::TopDown) {
diff --git a/llvm/lib/Target/RISCV/RISCVSubtarget.cpp b/llvm/lib/Target/RISCV/RISCVSubtarget.cpp
index c754de45db7fd..628d82b16b7fe 100644
--- a/llvm/lib/Target/RISCV/RISCVSubtarget.cpp
+++ b/llvm/lib/Target/RISCV/RISCVSubtarget.cpp
@@ -231,8 +231,8 @@ void RISCVSubtarget::overrideSchedPolicy(MachineSchedPolicy &Policy,
   Policy.ShouldTrackPressure = true;
 }
 
-void RISCVSubtarget::overridePostRASchedPolicy(MachineSchedPolicy &Policy,
-                                               unsigned NumRegionInstrs) const {
+void RISCVSubtarget::overridePostRASchedPolicy(
+    MachineSchedPolicy &Policy, const MachineFunction &MF) const {
   MISched::Direction PostRASchedDirection = getPostRASchedDirection();
   if (PostRASchedDirection == MISched::TopDown) {
     Policy.OnlyTopDown = true;
diff --git a/llvm/lib/Target/RISCV/RISCVSubtarget.h b/llvm/lib/Target/RISCV/RISCVSubtarget.h
index 4f560cca22dff..55f44ac9da1d1 100644
--- a/llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ b/llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -398,8 +398,8 @@ class RISCVSubtarget : public RISCVGenSubtargetInfo {
                            unsigned NumRegionInstrs) const override;
 
   void overridePostRASchedPolicy(MachineSchedPolicy &Policy,
-                                 unsigned NumRegionInstrs) const override;
+                                 const MachineFunction &MF) const override;
 };
-} // End llvm namespace
+} // namespace llvm
 
 #endif

wangpc-pp

I have two questions here:

Should we change overrideSchedPolicy as well?
Subtarget should be per-function? Why can't we just use it? Edit: I see, you want attributes.

mshockwave

though I'm not sure how many people is using it (downstream), but I don't think NumRegionInstrs can be easily calculated from MachineFunction. Plus, fundamentally this API is called per region rather than per function, so I'm wondering instead of passing in only the MachineFunction, could we pass in SchedRegion instead, which has both the MachineInstr iterators and number of instructions in the region (you have to make SchedRegion public, of course)

mshockwave · 2025-07-17T16:37:26Z

Should we change overrideSchedPolicy as well?

I also think we should apply this change on pre-RA scheduler in this patch.

jroelofs · 2025-07-17T16:53:12Z

Plus, fundamentally this API is called per region rather than per function

I was thinking the same thing. Alternatively, you could pass the MachineBasicBlock, since that's what we have available in initPolicy.

arsenm · 2025-07-18T00:19:17Z

Does overridePostRASchedPolicy really need direct access to the attribute? I'd expect the scheduler pass to directly query the attribute to select the mode

harrisonGPU · 2025-07-18T07:18:17Z

@wangpc-pp @mshockwave @jroelofs Thanks a lot for the guidance!

I've now updated both overridePostRASchedPolicy and overrideSchedPolicy. In the new version, I'm passing MachineBasicBlock instead of MachineFunction.

For graphics shader compilers, we have different shader stages (e.g., vertex shader, fragment shader), which means each stage corresponds to a separate function within the same pipeline. We want to control the post-RA scheduling direction per function using function attributes. Although a function may have multiple scheduling regions, we believe the post-RA scheduling direction should remain consistent across all regions within the same function.

In a previous patch, I implemented this using a target feature, but we think a function attribute makes more sense for finer-grained control. So, I propose to extend the overridePostRASchedPolicy interface by adding MachineBasicBlock as an argument:

#147708

With this change, the backend can access the function attribute and select the post-RA scheduling direction accordingly. Here's the updated hook:

void GCNSubtarget::overridePostRASchedPolicy(MachineSchedPolicy &Policy,
                                             const MachineBasicBlock &MBB,
                                             unsigned NumRegionInstr) const {
  const Function &F = MBB.getParent()->getFunction();
  Attribute PostRADirectionAttr = F.getFnAttribute("amdgpu-post-ra-direction");
  if (!PostRADirectionAttr.isValid())
    return;

  StringRef PostRADirectionStr = PostRADirectionAttr.getValueAsString();
  if (PostRADirectionStr == "topdown") {
    Policy.OnlyTopDown = true;
    Policy.OnlyBottomUp = false;
  } else if (PostRADirectionStr == "bottomup") {
    Policy.OnlyTopDown = false;
    Policy.OnlyBottomUp = true;
  } else if (PostRADirectionStr == "bidirectional") {
    Policy.OnlyTopDown = false;
    Policy.OnlyBottomUp = false;
  } else {
    DiagnosticInfoOptimizationFailure Diag(
        F, F.getSubprogram(),
        Twine("invalid value for post-ra direction attribute: '") +
            PostRADirectionStr + "'");
    F.getContext().diagnose(Diag);
  }
}

harrisonGPU · 2025-07-18T07:30:29Z

Does overridePostRASchedPolicy really need direct access to the attribute? I'd expect the scheduler pass to directly query the attribute to select the mode

I believe overridePostRASchedPolicy is designed to allow backend-specific customization, so it's a reasonable place to access function attributes and implement target-specific scheduling strategies.

llvm/include/llvm/CodeGen/TargetSubtargetInfo.h

llvm/lib/Target/AMDGPU/GCNSubtarget.h

llvm/lib/Target/PowerPC/PPCSubtarget.cpp

llvm/lib/Target/RISCV/RISCVSubtarget.h

mshockwave · 2025-07-18T16:47:03Z

Does overridePostRASchedPolicy really need direct access to the attribute? I'd expect the scheduler pass to directly query the attribute to select the mode

I guess you meant making this attribute a generic one so that MachineScheduler would be the one selecting the policy instead of the subtarget?

jroelofs

LGTM

harrisonGPU · 2025-07-21T02:46:50Z

Anyone have additional feedback or suggestions?

wangpc-pp · 2025-07-21T08:25:12Z

though I'm not sure how many people is using it (downstream), but I don't think NumRegionInstrs can be easily calculated from MachineFunction. Plus, fundamentally this API is called per region rather than per function, so I'm wondering instead of passing in only the MachineFunction, could we pass in SchedRegion instead, which has both the MachineInstr iterators and number of instructions in the region (you have to make SchedRegion public, of course)

I would prefer to pass SchedRegion here. @harrisonGPU

harrisonGPU · 2025-07-22T02:31:33Z

though I'm not sure how many people is using it (downstream), but I don't think NumRegionInstrs can be easily calculated from MachineFunction. Plus, fundamentally this API is called per region rather than per function, so I'm wondering instead of passing in only the MachineFunction, could we pass in SchedRegion instead, which has both the MachineInstr iterators and number of instructions in the region (you have to make SchedRegion public, of course)

I would prefer to pass SchedRegion here. @harrisonGPU

I don’t think it’s worth making the interface more complex. As far as I know, only the RISC-V backend currently overrides overridePostRASchedPolicy, and even there it only uses the Policy argument. The NumRegionInstrs argument has been unused for a long time, and overrideSchedPolicy also typically only uses Policy across most backends.

In AMDGPU’s case, we only want to use a function attribute to control the post-RA direction, and we believe this direction should remain consistent across all regions within the same function.

In fact, I think this is how most backends work — they don’t select different post-RA or pre-RA scheduler directions per region. Even though the machine scheduler splits a function into multiple regions, the policy is usually function-wide.

Introducing SchedRegion here would complicate the API, and I’m not sure the added complexity is justified.

What do you think?

wangpc-pp · 2025-07-22T03:02:20Z

I still think we should pass SchedRegion. The signature will be:

void overridePostRASchedPolicy(MachineSchedPolicy &Policy,
                               const SchedRegion &Region) const

The parameters are less and it is more straightforward since the scheduler is region-based, MachineBasicBlock != SchedRegion conceptively.

harrisonGPU · 2025-07-22T04:18:02Z

I still think we should pass SchedRegion. The signature will be:
void overridePostRASchedPolicy(MachineSchedPolicy &Policy,
                               const SchedRegion &Region) const
The parameters are less and it is more straightforward since the scheduler is region-based, MachineBasicBlock != SchedRegion conceptively.

Thanks, I need to change overrideSchedPolicy as well?

harrisonGPU · 2025-07-22T04:51:59Z

@wangpc-pp , I have already updated it, what do you think about?

wangpc-pp

LGTM.

(We can also change enterRegion but this can be a follow-up)

harrisonGPU · 2025-07-22T07:54:56Z

Thanks :-)

…hedPolicy (llvm#149297) This patch updates `overrideSchedPolicy` and `overridePostRASchedPolicy` to take a `SchedRegion` parameter instead of just `NumRegionInstrs`. This provides access to both the instruction range and the parent `MachineBasicBlock`, which enables looking up function-level attributes. With this change, targets can select post-RA scheduling direction per function using a function attribute. For example: ```cpp void overridePostRASchedPolicy(MachineSchedPolicy &Policy, const SchedRegion &Region) const { const Function &F = Region.RegionBegin->getMF()->getFunction(); Attribute Attr = F.getFnAttribute("amdgpu-post-ra-direction"); ... }

[MISched] Extend overridePostRASchedPolicy to support per-function sc…

e9d38f6

…heduling direction

harrisonGPU requested review from arsenm, preames, mshockwave, jroelofs, michaelmaitland and wangpc-pp July 17, 2025 12:24

llvmbot added backend:RISC-V llvm:codegen labels Jul 17, 2025

wangpc-pp reviewed Jul 17, 2025

View reviewed changes

mshockwave reviewed Jul 17, 2025

View reviewed changes

[MISched] Add MachineBasicBlock.

cf2392f

llvmbot added backend:AArch64 backend:AMDGPU backend:PowerPC labels Jul 18, 2025

jroelofs approved these changes Jul 18, 2025

View reviewed changes

llvm/include/llvm/CodeGen/TargetSubtargetInfo.h Outdated Show resolved Hide resolved

mshockwave reviewed Jul 18, 2025

View reviewed changes

llvm/lib/Target/AMDGPU/GCNSubtarget.h Outdated Show resolved Hide resolved

llvm/lib/Target/PowerPC/PPCSubtarget.cpp Outdated Show resolved Hide resolved

llvm/lib/Target/RISCV/RISCVSubtarget.h Show resolved Hide resolved

Update for comments.

ed80350

jroelofs reviewed Jul 19, 2025

View reviewed changes

harrisonGPU added 2 commits July 22, 2025 12:46

[MISched] Use Region to replace MBB

fa4dc0b

[MISched] Update.

acf8622

harrisonGPU changed the title ~~[MISched] Extend overridePostRASchedPolicy to support per-function scheduling direction~~ [MISched] Use SchedRegion in overrideSchedPolicy and overridePostRASchedPolicy Jul 22, 2025

wangpc-pp approved these changes Jul 22, 2025

View reviewed changes

harrisonGPU merged commit 8c14d3f into llvm:main Jul 22, 2025
9 checks passed

harrisonGPU deleted the MISched branch July 22, 2025 08:08

[MISched] Use SchedRegion in overrideSchedPolicy and overridePostRASchedPolicy #149297

[MISched] Use SchedRegion in overrideSchedPolicy and overridePostRASchedPolicy #149297

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

wangpc-pp left a comment • 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!

mshockwave left a comment • 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!

mshockwave commented Jul 17, 2025

Uh oh!

jroelofs commented Jul 17, 2025

Uh oh!

arsenm commented Jul 18, 2025

Uh oh!

harrisonGPU commented Jul 18, 2025

Uh oh!

harrisonGPU commented Jul 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mshockwave commented Jul 18, 2025

Uh oh!

jroelofs left a comment

Choose a reason for hiding this comment

Uh oh!

harrisonGPU commented Jul 21, 2025

Uh oh!

wangpc-pp commented Jul 21, 2025

Uh oh!

harrisonGPU commented Jul 22, 2025

Uh oh!

wangpc-pp commented Jul 22, 2025

Uh oh!

harrisonGPU commented Jul 22, 2025

Uh oh!

harrisonGPU commented Jul 22, 2025

Uh oh!

wangpc-pp left a comment

Choose a reason for hiding this comment

Uh oh!

harrisonGPU commented Jul 22, 2025

Uh oh!

Uh oh!

Uh oh!

harrisonGPU commented Jul 17, 2025 •

edited

Loading

llvmbot commented Jul 17, 2025 •

edited

Loading

wangpc-pp left a comment •

edited

Loading

mshockwave left a comment •

edited

Loading