[llvm] [RISCV] Do not check PostRAScheduler in enablePostRAScheduler (PR #92781)
Michael Maitland via llvm-commits
llvm-commits at lists.llvm.org
Wed May 22 09:01:40 PDT 2024
https://github.com/michaelmaitland updated https://github.com/llvm/llvm-project/pull/92781
>From 400bd8b3c9a73183a400f4e99b3c78ecf0f51384 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Mon, 20 May 2024 08:59:17 -0700
Subject: [PATCH 1/4] [RISCV] Do not check UsePostRAScheduler in
enablePostRAScheduler
On RISC-V, there are a few ways to control whether the PostMachineScheduler is
enabled. If `-enable-post-misched` is passed or passed with a value of true,
then the PostMachineScheduler is enabled. If it is passed with a value of false
then the PostMachineScheduler is disabled. If the option is not passed
at all, then `TargetSubtargetInfo::enablePostRAMachineScheduler` decides
whether the pass should be enabled or not.
`RISCVSubtarget::enablePostRAMachineScheduler` currently checks if the
active scheduler model sets `PostRAScheduler`. If it is set to true by the
scheduler model, then the pass is enabled. If it is not set to true by the
scheduler model, then the value of `UsePostRAScheduler` subtarget feature is
used.
I argue that the RISC-V backend should not use `PostRAScheduler` field of the
scheduler model to control whether the PostMachineScheduler is enabled for the
following reasons:
1. No other targets use this value to control whether PostMachineScheduler
is enabled. They only use it to check whether the legacy PostRASchedulerList
scheduelr is enabled.
2. We can add the `UsePostRAScheduler` feature to the processor definition
in RISCVProcessors.td to tie a processor to whether the pass should be
enabled by default. This makes the feature and the sched model field redundant.
3. Since these options are redundant, we should prefer the feature, since we can
set `+` and `-` on the feature, but the value of the scheduler cannot be
controled on the command line.
4. Keeping both options allows us to set the feature and the scheduler
model value to conflicting values. Although the scheduler model value
will win out, it feels awkward to allow it.
There are no upstream subtargets that use the scheduler model
`PostRAScheduler` field. If this patch lands, all downstream subtargets
should replace `PostRAScheduler` with the `UsePostRAScheduler` feature
in RISCVProcessors.td to acheive the desired functionality.
---
llvm/lib/Target/RISCV/RISCVSubtarget.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVSubtarget.h b/llvm/lib/Target/RISCV/RISCVSubtarget.h
index c880c9e921e0e..347c1bc3c278f 100644
--- a/llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ b/llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -121,9 +121,7 @@ class RISCVSubtarget : public RISCVGenSubtargetInfo {
}
bool enableMachineScheduler() const override { return true; }
- bool enablePostRAScheduler() const override {
- return getSchedModel().PostRAScheduler || UsePostRAScheduler;
- }
+ bool enablePostRAScheduler() const override { return UsePostRAScheduler; }
Align getPrefFunctionAlignment() const {
return Align(TuneInfo->PrefFunctionAlignment);
>From f3545e01a5f5e7f2b57c55c03ba1094121e5ed73 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Tue, 21 May 2024 06:50:04 -0700
Subject: [PATCH 2/4] fixup! update release notes
---
llvm/docs/ReleaseNotes.rst | 1 +
1 file changed, 1 insertion(+)
diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index cba36c7177daa..d148789310775 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -130,6 +130,7 @@ Changes to the RISC-V Backend
match GNU objdump. The bytes within the groups are in big endian order.
* Added smstateen extension to -march. CSR names for smstateen were already supported.
* Zaamo and Zalrsc are no longer experimental.
+* Processors that enable post reg-alloc scheduling (PostMachineScheduler) by default should use the `UsePostRAScheduler` subtarget feature. Setting `PostRAScheduler = 1` in the scheduler model will have no effect on the enabling of the PostMachineScheduler.
Changes to the WebAssembly Backend
----------------------------------
>From a189d7c3463e7b23183f6241c4891b6487e1fcfb Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Tue, 21 May 2024 12:55:58 -0700
Subject: [PATCH 3/4] fixup! remove PostRAScheduler from RISCV scheduler models
---
llvm/lib/Target/RISCV/RISCVProcessors.td | 9 +++++----
llvm/lib/Target/RISCV/RISCVSchedSiFive7.td | 1 -
llvm/lib/Target/RISCV/RISCVSchedSiFiveP400.td | 1 -
llvm/lib/Target/RISCV/RISCVSchedSiFiveP600.td | 1 -
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVProcessors.td b/llvm/lib/Target/RISCV/RISCVProcessors.td
index a4a5d9e96c271..ff3a1c12f0ef8 100644
--- a/llvm/lib/Target/RISCV/RISCVProcessors.td
+++ b/llvm/lib/Target/RISCV/RISCVProcessors.td
@@ -85,7 +85,7 @@ def ROCKET : RISCVTuneProcessorModel<"rocket",
def SIFIVE_7 : RISCVTuneProcessorModel<"sifive-7-series",
SiFive7Model,
- [TuneSiFive7]>;
+ [TuneSiFive7, FeaturePostRAScheduler]>;
def SIFIVE_E20 : RISCVProcessorModel<"sifive-e20",
RocketModel,
@@ -145,7 +145,7 @@ def SIFIVE_E76 : RISCVProcessorModel<"sifive-e76",
FeatureStdExtA,
FeatureStdExtF,
FeatureStdExtC],
- [TuneSiFive7]>;
+ [TuneSiFive7, FeaturePostRAScheduler]>;
def SIFIVE_S21 : RISCVProcessorModel<"sifive-s21",
RocketModel,
@@ -189,7 +189,7 @@ def SIFIVE_S76 : RISCVProcessorModel<"sifive-s76",
FeatureStdExtD,
FeatureStdExtC,
FeatureStdExtZihintpause],
- [TuneSiFive7]>;
+ [TuneSiFive7, FeaturePostRAScheduler]>;
def SIFIVE_U54 : RISCVProcessorModel<"sifive-u54",
RocketModel,
@@ -212,7 +212,7 @@ def SIFIVE_U74 : RISCVProcessorModel<"sifive-u74",
FeatureStdExtF,
FeatureStdExtD,
FeatureStdExtC],
- [TuneSiFive7]>;
+ [TuneSiFive7, FeaturePostRAScheduler]>;
def SIFIVE_X280 : RISCVProcessorModel<"sifive-x280", SiFive7Model,
[Feature64Bit,
@@ -230,6 +230,7 @@ def SIFIVE_X280 : RISCVProcessorModel<"sifive-x280", SiFive7Model,
FeatureStdExtZba,
FeatureStdExtZbb],
[TuneSiFive7,
+ FeaturePostRAScheduler,
TuneDLenFactor2]>;
def SIFIVE_P450 : RISCVProcessorModel<"sifive-p450", SiFiveP400Model,
diff --git a/llvm/lib/Target/RISCV/RISCVSchedSiFive7.td b/llvm/lib/Target/RISCV/RISCVSchedSiFive7.td
index 83fb75727bbe8..a6f776301bdc0 100644
--- a/llvm/lib/Target/RISCV/RISCVSchedSiFive7.td
+++ b/llvm/lib/Target/RISCV/RISCVSchedSiFive7.td
@@ -199,7 +199,6 @@ def SiFive7Model : SchedMachineModel {
let LoadLatency = 3;
let MispredictPenalty = 3;
let CompleteModel = 0;
- let PostRAScheduler = true;
let EnableIntervals = true;
let UnsupportedFeatures = [HasStdExtZbkb, HasStdExtZbkc, HasStdExtZbkx,
HasStdExtZcmt, HasStdExtZknd, HasStdExtZkne,
diff --git a/llvm/lib/Target/RISCV/RISCVSchedSiFiveP400.td b/llvm/lib/Target/RISCV/RISCVSchedSiFiveP400.td
index a37958826e028..80362cae00fcd 100644
--- a/llvm/lib/Target/RISCV/RISCVSchedSiFiveP400.td
+++ b/llvm/lib/Target/RISCV/RISCVSchedSiFiveP400.td
@@ -13,7 +13,6 @@ def SiFiveP400Model : SchedMachineModel {
let MicroOpBufferSize = 56; // Max micro-ops that can be buffered.
let LoadLatency = 4; // Cycles for loads to access the cache.
let MispredictPenalty = 9; // Extra cycles for a mispredicted branch.
- let PostRAScheduler = true;
let UnsupportedFeatures = [HasStdExtZbkb, HasStdExtZbkc, HasStdExtZbkx,
HasStdExtZcmt, HasStdExtZknd, HasStdExtZkne,
HasStdExtZknh, HasStdExtZksed, HasStdExtZksh,
diff --git a/llvm/lib/Target/RISCV/RISCVSchedSiFiveP600.td b/llvm/lib/Target/RISCV/RISCVSchedSiFiveP600.td
index 07d72b61862dd..8291e00878063 100644
--- a/llvm/lib/Target/RISCV/RISCVSchedSiFiveP600.td
+++ b/llvm/lib/Target/RISCV/RISCVSchedSiFiveP600.td
@@ -56,7 +56,6 @@ def SiFiveP600Model : SchedMachineModel {
let MicroOpBufferSize = 160; // Max micro-ops that can be buffered.
let LoadLatency = 4; // Cycles for loads to access the cache.
let MispredictPenalty = 9; // Extra cycles for a mispredicted branch.
- let PostRAScheduler = true;
let UnsupportedFeatures = [HasStdExtZbkb, HasStdExtZbkc, HasStdExtZbkx,
HasStdExtZknd, HasStdExtZkne, HasStdExtZknh,
HasStdExtZksed, HasStdExtZksh, HasStdExtZkr,
>From 33084bb0bdd5d4a4036efc1b2c53b4da98b7d7cf Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Wed, 22 May 2024 09:00:56 -0700
Subject: [PATCH 4/4] fixup! p450 and p670 enable postra
---
llvm/lib/Target/RISCV/RISCVProcessors.td | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVProcessors.td b/llvm/lib/Target/RISCV/RISCVProcessors.td
index ff3a1c12f0ef8..6ebf9f1eb0452 100644
--- a/llvm/lib/Target/RISCV/RISCVProcessors.td
+++ b/llvm/lib/Target/RISCV/RISCVProcessors.td
@@ -263,7 +263,8 @@ def SIFIVE_P450 : RISCVProcessorModel<"sifive-p450", SiFiveP400Model,
[TuneNoDefaultUnroll,
TuneConditionalCompressedMoveFusion,
TuneLUIADDIFusion,
- TuneAUIPCADDIFusion]>;
+ TuneAUIPCADDIFusion,
+ FeaturePostRAScheduler]>;
def SIFIVE_P670 : RISCVProcessorModel<"sifive-p670", SiFiveP600Model,
[Feature64Bit,
@@ -303,7 +304,8 @@ def SIFIVE_P670 : RISCVProcessorModel<"sifive-p670", SiFiveP600Model,
TuneConditionalCompressedMoveFusion,
TuneLUIADDIFusion,
TuneAUIPCADDIFusion,
- TuneNoSinkSplatOperands]>;
+ TuneNoSinkSplatOperands,
+ FeaturePostRAScheduler]>;
def SYNTACORE_SCR1_BASE : RISCVProcessorModel<"syntacore-scr1-base",
SyntacoreSCR1Model,
More information about the llvm-commits
mailing list