[CodeGen] Move EnableSinkAndFold to TargetOptions

[optimisan]

EnableSinkAndFold was in TargetPassConfig unavailable in the new pass infrastructure.

[optimisan]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• [CodeGen] Move EnableSinkAndFold to TargetOptions #114746 👈 (View in Graphite)
• [CodeGen][NewPM] Port MachineCycleInfo to NPM #114745 : 2 other dependent PRs (#115434 , #116166 )
• [NFC][CodeGen] Clang format MachineSink.cpp #114027
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-backend-aarch64

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

Author: Akshat Oke (optimisan)

Changes

---

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

5 Files Affected:

• (modified) llvm/include/llvm/CodeGen/TargetPassConfig.h (-8)
• (modified) llvm/include/llvm/Target/TargetOptions.h (+7-1)
• (modified) llvm/lib/CodeGen/MachineSink.cpp (+3-1)
• (modified) llvm/lib/Target/AArch64/AArch64TargetMachine.cpp (+1-1)
• (modified) llvm/lib/Target/RISCV/RISCVTargetMachine.cpp (+1-1)

diff --git a/llvm/include/llvm/CodeGen/TargetPassConfig.h b/llvm/include/llvm/CodeGen/TargetPassConfig.h
index 2f5951e3ec3bce..b395774b14c441 100644
--- a/llvm/include/llvm/CodeGen/TargetPassConfig.h
+++ b/llvm/include/llvm/CodeGen/TargetPassConfig.h
@@ -131,11 +131,6 @@ class TargetPassConfig : public ImmutablePass {
   /// Default setting for -enable-tail-merge on this target.
   bool EnableTailMerge = true;
 
-  /// Enable sinking of instructions in MachineSink where a computation can be
-  /// folded into the addressing mode of a memory load/store instruction or
-  /// replace a copy.
-  bool EnableSinkAndFold = false;
-
   /// Require processing of functions such that callees are generated before
   /// callers.
   bool RequireCodeGenSCCOrder = false;
@@ -198,9 +193,6 @@ class TargetPassConfig : public ImmutablePass {
   bool getEnableTailMerge() const { return EnableTailMerge; }
   void setEnableTailMerge(bool Enable) { setOpt(EnableTailMerge, Enable); }
 
-  bool getEnableSinkAndFold() const { return EnableSinkAndFold; }
-  void setEnableSinkAndFold(bool Enable) { setOpt(EnableSinkAndFold, Enable); }
-
   bool requiresCodeGenSCCOrder() const { return RequireCodeGenSCCOrder; }
   void setRequiresCodeGenSCCOrder(bool Enable = true) {
     setOpt(RequireCodeGenSCCOrder, Enable);
diff --git a/llvm/include/llvm/Target/TargetOptions.h b/llvm/include/llvm/Target/TargetOptions.h
index 88f253805ca99c..b16ad5b69ff05a 100644
--- a/llvm/include/llvm/Target/TargetOptions.h
+++ b/llvm/include/llvm/Target/TargetOptions.h
@@ -137,7 +137,8 @@ namespace llvm {
           ApproxFuncFPMath(false), EnableAIXExtendedAltivecABI(false),
           HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
           GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
-          EnableFastISel(false), EnableGlobalISel(false), UseInitArray(false),
+          EnableSinkAndFold(false), EnableFastISel(false),
+          EnableGlobalISel(false), UseInitArray(false),
           DisableIntegratedAS(false), FunctionSections(false),
           DataSections(false), IgnoreXCOFFVisibility(false),
           XCOFFTracebackTable(true), UniqueSectionNames(true),
@@ -239,6 +240,11 @@ namespace llvm {
     /// they were generated. Default is true.
     unsigned StackSymbolOrdering : 1;
 
+    /// EnableSinkAndFold - Enable sinking of instructions in MachineSink where
+    /// a computation can be folded into the addressing mode of a memory
+    /// load/store instruction or replace a copy.
+    unsigned EnableSinkAndFold : 1;
+
     /// EnableFastISel - This flag enables fast-path instruction selection
     /// which trades away generated code quality in favor of reducing
     /// compile time.
diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp
index a0e09398602e9e..6849a3f12d8cfd 100644
--- a/llvm/lib/CodeGen/MachineSink.cpp
+++ b/llvm/lib/CodeGen/MachineSink.cpp
@@ -54,6 +54,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetMachine.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
@@ -729,7 +730,8 @@ bool MachineSinking::runOnMachineFunction(MachineFunction &MF) {
   AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
   RegClassInfo.runOnMachineFunction(MF);
   TargetPassConfig *PassConfig = &getAnalysis<TargetPassConfig>();
-  EnableSinkAndFold = PassConfig->getEnableSinkAndFold();
+  auto &TM = PassConfig->getTM<TargetMachine>();
+  EnableSinkAndFold = TM.Options.EnableSinkAndFold;
 
   bool EverMadeChange = false;
 
diff --git a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
index c7bd0390b65620..ee8aae4ee8bcc8 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
@@ -505,7 +505,7 @@ class AArch64PassConfig : public TargetPassConfig {
       : TargetPassConfig(TM, PM) {
     if (TM.getOptLevel() != CodeGenOptLevel::None)
       substitutePass(&PostRASchedulerID, &PostMachineSchedulerID);
-    setEnableSinkAndFold(EnableSinkFold);
+    TM.Options.EnableSinkAndFold = EnableSinkFold;
   }
 
   AArch64TargetMachine &getAArch64TargetMachine() const {
diff --git a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
index 72d74d2d79b1d5..00653ca348476e 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -337,7 +337,7 @@ class RISCVPassConfig : public TargetPassConfig {
       : TargetPassConfig(TM, PM) {
     if (TM.getOptLevel() != CodeGenOptLevel::None)
       substitutePass(&PostRASchedulerID, &PostMachineSchedulerID);
-    setEnableSinkAndFold(EnableSinkFold);
+    TM.Options.EnableSinkAndFold = EnableSinkFold;
     EnableLoopTermFold = true;
   }
 

[arsenm]

We want to moving things out of TargetOptions, not into. Can we turn this into a pass parameter, and just let the targets set it when they add the pass to the pipeline?

Alternatively it seems to be just one debug flag, could move it directly into the pass instead of making targets create aliased cl::opts

[optimisan]

The default value is false, but two targets are setting it to true.

Currently MachineSink is added by generic TargetPassConfig. Can add the option to CGPassBuilderOptions so targets can set it there instead.