Skip to content

Commit 66b5f16

Browse files
michaelmaitlandmichaelmaitland
authored
[RISCV] Do not check PostRAScheduler in enablePostRAScheduler (#92781)
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 `RISCVSubtarget::enablePostRAMachineScheduler` decides whether the pass should be enabled or not. `TargetSubtargetInfo::enablePostRAScheduler` and `TargetSubtargetInfo::enablePostRAMachineScheduler` who check the SchedModel value are not called by RISC-V backend. `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 scheduler 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 controlled 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.
1 parent 098c6df commit 66b5f16

File tree

6 files changed

+11
-12
lines changed

6 files changed

+11
-12
lines changed

‎llvm/docs/ReleaseNotes.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ Changes to the RISC-V Backend
134134
match GNU objdump. The bytes within the groups are in big endian order.
135135
* Added smstateen extension to -march. CSR names for smstateen were already supported.
136136
* Zaamo and Zalrsc are no longer experimental.
137+
* 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.
137138

138139
Changes to the WebAssembly Backend
139140
----------------------------------

‎llvm/lib/Target/RISCV/RISCVProcessors.td

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ def ROCKET : RISCVTuneProcessorModel<"rocket",
8585

8686
def SIFIVE_7 : RISCVTuneProcessorModel<"sifive-7-series",
8787
SiFive7Model,
88-
[TuneSiFive7]>;
88+
[TuneSiFive7, FeaturePostRAScheduler]>;
8989

9090
def SIFIVE_E20 : RISCVProcessorModel<"sifive-e20",
9191
RocketModel,
@@ -145,7 +145,7 @@ def SIFIVE_E76 : RISCVProcessorModel<"sifive-e76",
145145
FeatureStdExtA,
146146
FeatureStdExtF,
147147
FeatureStdExtC],
148-
[TuneSiFive7]>;
148+
[TuneSiFive7, FeaturePostRAScheduler]>;
149149

150150
def SIFIVE_S21 : RISCVProcessorModel<"sifive-s21",
151151
RocketModel,
@@ -189,7 +189,7 @@ def SIFIVE_S76 : RISCVProcessorModel<"sifive-s76",
189189
FeatureStdExtD,
190190
FeatureStdExtC,
191191
FeatureStdExtZihintpause],
192-
[TuneSiFive7]>;
192+
[TuneSiFive7, FeaturePostRAScheduler]>;
193193

194194
def SIFIVE_U54 : RISCVProcessorModel<"sifive-u54",
195195
RocketModel,
@@ -212,7 +212,7 @@ def SIFIVE_U74 : RISCVProcessorModel<"sifive-u74",
212212
FeatureStdExtF,
213213
FeatureStdExtD,
214214
FeatureStdExtC],
215-
[TuneSiFive7]>;
215+
[TuneSiFive7, FeaturePostRAScheduler]>;
216216

217217
def SIFIVE_X280 : RISCVProcessorModel<"sifive-x280", SiFive7Model,
218218
[Feature64Bit,
@@ -230,6 +230,7 @@ def SIFIVE_X280 : RISCVProcessorModel<"sifive-x280", SiFive7Model,
230230
FeatureStdExtZba,
231231
FeatureStdExtZbb],
232232
[TuneSiFive7,
233+
FeaturePostRAScheduler,
233234
TuneDLenFactor2]>;
234235

235236
def SIFIVE_P450 : RISCVProcessorModel<"sifive-p450", SiFiveP400Model,
@@ -262,7 +263,8 @@ def SIFIVE_P450 : RISCVProcessorModel<"sifive-p450", SiFiveP400Model,
262263
[TuneNoDefaultUnroll,
263264
TuneConditionalCompressedMoveFusion,
264265
TuneLUIADDIFusion,
265-
TuneAUIPCADDIFusion]>;
266+
TuneAUIPCADDIFusion,
267+
FeaturePostRAScheduler]>;
266268

267269
def SIFIVE_P670 : RISCVProcessorModel<"sifive-p670", SiFiveP600Model,
268270
[Feature64Bit,
@@ -302,7 +304,8 @@ def SIFIVE_P670 : RISCVProcessorModel<"sifive-p670", SiFiveP600Model,
302304
TuneConditionalCompressedMoveFusion,
303305
TuneLUIADDIFusion,
304306
TuneAUIPCADDIFusion,
305-
TuneNoSinkSplatOperands]>;
307+
TuneNoSinkSplatOperands,
308+
FeaturePostRAScheduler]>;
306309

307310
def SYNTACORE_SCR1_BASE : RISCVProcessorModel<"syntacore-scr1-base",
308311
SyntacoreSCR1Model,

‎llvm/lib/Target/RISCV/RISCVSchedSiFive7.td

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,6 @@ def SiFive7Model : SchedMachineModel {
199199
let LoadLatency = 3;
200200
let MispredictPenalty = 3;
201201
let CompleteModel = 0;
202-
let PostRAScheduler = true;
203202
let EnableIntervals = true;
204203
let UnsupportedFeatures = [HasStdExtZbkb, HasStdExtZbkc, HasStdExtZbkx,
205204
HasStdExtZcmt, HasStdExtZknd, HasStdExtZkne,

‎llvm/lib/Target/RISCV/RISCVSchedSiFiveP400.td

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ def SiFiveP400Model : SchedMachineModel {
1313
let MicroOpBufferSize = 56; // Max micro-ops that can be buffered.
1414
let LoadLatency = 4; // Cycles for loads to access the cache.
1515
let MispredictPenalty = 9; // Extra cycles for a mispredicted branch.
16-
let PostRAScheduler = true;
1716
let UnsupportedFeatures = [HasStdExtZbkb, HasStdExtZbkc, HasStdExtZbkx,
1817
HasStdExtZcmt, HasStdExtZknd, HasStdExtZkne,
1918
HasStdExtZknh, HasStdExtZksed, HasStdExtZksh,

‎llvm/lib/Target/RISCV/RISCVSchedSiFiveP600.td

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ def SiFiveP600Model : SchedMachineModel {
5656
let MicroOpBufferSize = 160; // Max micro-ops that can be buffered.
5757
let LoadLatency = 4; // Cycles for loads to access the cache.
5858
let MispredictPenalty = 9; // Extra cycles for a mispredicted branch.
59-
let PostRAScheduler = true;
6059
let UnsupportedFeatures = [HasStdExtZbkb, HasStdExtZbkc, HasStdExtZbkx,
6160
HasStdExtZknd, HasStdExtZkne, HasStdExtZknh,
6261
HasStdExtZksed, HasStdExtZksh, HasStdExtZkr,

‎llvm/lib/Target/RISCV/RISCVSubtarget.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,7 @@ class RISCVSubtarget : public RISCVGenSubtargetInfo {
121121
}
122122
bool enableMachineScheduler() const override { return true; }
123123

124-
bool enablePostRAScheduler() const override {
125-
return getSchedModel().PostRAScheduler || UsePostRAScheduler;
126-
}
124+
bool enablePostRAScheduler() const override { return UsePostRAScheduler; }
127125

128126
Align getPrefFunctionAlignment() const {
129127
return Align(TuneInfo->PrefFunctionAlignment);

0 commit comments

Comments
 (0)