[llvm] [RISCV] Do not check UsePostRAScheduler in enablePostRAScheduler (PR #92781)

[Michael Maitland via llvm-commits]

https://github.com/michaelmaitland created https://github.com/llvm/llvm-project/pull/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 `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.

>From 32d8ff5f1a2636e26cc29fe8afeadcc359f773da 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] [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);