Scalarizer: Replace cl::opts with pass parameters (#110645)

Preserve the existing defaults (although load-store defaulting
to false is a really bad one). Also migrate DirectX tests to new PM.

Author: arsenm

llvm/include/llvm/Transforms/Scalar/Scalarizer.h

@@ -27,13 +27,25 @@ class Function;
 class FunctionPass;
 
 struct ScalarizerPassOptions {
-  // These options correspond 1:1 to cl::opt options defined in
-  // Scalarizer.cpp. When the cl::opt are specified, they take precedence.
-  // When the cl::opt are not specified, the present optional values allow to
-  // override the cl::opt's default values.
-  std::optional<bool> ScalarizeVariableInsertExtract;
-  std::optional<bool> ScalarizeLoadStore;
-  std::optional<unsigned> ScalarizeMinBits;
+  /// Instruct the scalarizer pass to attempt to keep values of a minimum number
+  /// of bits.
+
+  /// Split vectors larger than this size into fragments, where each fragment is
+  /// either a vector no larger than this size or a scalar.
+  ///
+  /// Instructions with operands or results of different sizes that would be
+  /// split into a different number of fragments are currently left as-is.
+  unsigned ScalarizeMinBits = 0;
+
+  /// Allow the scalarizer pass to scalarize insertelement/extractelement with
+  /// variable index.
+  bool ScalarizeVariableInsertExtract = true;
+
+  /// Allow the scalarizer pass to scalarize loads and store
+  ///
+  /// This is disabled by default because having separate loads and stores makes
+  /// it more likely that the -combiner-alias-analysis limits will be reached.
+  bool ScalarizeLoadStore = false;
 };
 
 class ScalarizerPass : public PassInfoMixin<ScalarizerPass> {

llvm/lib/Passes/PassBuilder.cpp

@@ -1047,6 +1047,40 @@ Expected<IPSCCPOptions> parseIPSCCPOptions(StringRef Params) {
   return Result;
 }
 
+Expected<ScalarizerPassOptions> parseScalarizerOptions(StringRef Params) {
+  ScalarizerPassOptions Result;
+  while (!Params.empty()) {
+    StringRef ParamName;
+    std::tie(ParamName, Params) = Params.split(';');
+
+    if (ParamName.consume_front("min-bits=")) {
+      if (ParamName.getAsInteger(0, Result.ScalarizeMinBits)) {
+        return make_error<StringError>(
+            formatv("invalid argument to Scalarizer pass min-bits "
+                    "parameter: '{0}' ",
+                    ParamName)
+                .str(),
+            inconvertibleErrorCode());
+      }
+
+      continue;
+    }
+
+    bool Enable = !ParamName.consume_front("no-");
+    if (ParamName == "load-store")
+      Result.ScalarizeLoadStore = Enable;
+    else if (ParamName == "variable-insert-extract")
+      Result.ScalarizeVariableInsertExtract = Enable;
+    else {
+      return make_error<StringError>(
+          formatv("invalid Scalarizer pass parameter '{0}' ", ParamName).str(),
+          inconvertibleErrorCode());
+    }
+  }
+
+  return Result;
+}
+
 Expected<SROAOptions> parseSROAOptions(StringRef Params) {
   if (Params.empty() || Params == "modify-cfg")
     return SROAOptions::ModifyCFG;

llvm/lib/Passes/PassRegistry.def

@@ -456,7 +456,6 @@ FUNCTION_PASS("reg2mem", RegToMemPass())
 FUNCTION_PASS("safe-stack", SafeStackPass(TM))
 FUNCTION_PASS("sandbox-vectorizer", SandboxVectorizerPass())
 FUNCTION_PASS("scalarize-masked-mem-intrin", ScalarizeMaskedMemIntrinPass())
-FUNCTION_PASS("scalarizer", ScalarizerPass())
 FUNCTION_PASS("sccp", SCCPPass())
 FUNCTION_PASS("select-optimize", SelectOptimizePass(TM))
 FUNCTION_PASS("separate-const-offset-from-gep",
@@ -573,6 +572,12 @@ FUNCTION_PASS_WITH_PARAMS(
       return StackLifetimePrinterPass(dbgs(), Type);
     },
     parseStackLifetimeOptions, "may;must")
+FUNCTION_PASS_WITH_PARAMS(
+    "scalarizer", "ScalarizerPass",
+    [](ScalarizerPassOptions Opts) { return ScalarizerPass(Opts); },
+    parseScalarizerOptions,
+    "load-store;no-load-store;variable-insert-extract;"
+    "no-variable-insert-extract;min-bits=N;")
 FUNCTION_PASS_WITH_PARAMS(
     "separate-const-offset-from-gep", "SeparateConstOffsetFromGEPPass",
     [](bool LowerGEP) { return SeparateConstOffsetFromGEPPass(LowerGEP); },

llvm/lib/Target/DirectX/DirectXPassRegistry.def

@@ -23,6 +23,7 @@ MODULE_ANALYSIS("dxil-resource-md", DXILResourceMDAnalysis())
 #ifndef MODULE_PASS
 #define MODULE_PASS(NAME, CREATE_PASS)
 #endif
+MODULE_PASS("dxil-data-scalarization", DXILDataScalarization())
 MODULE_PASS("dxil-intrinsic-expansion", DXILIntrinsicExpansion())
 MODULE_PASS("dxil-op-lower", DXILOpLowering())
 MODULE_PASS("dxil-pretty-printer", DXILPrettyPrinterPass(dbgs()))

llvm/lib/Target/DirectX/DirectXTargetMachine.cpp

@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DirectXTargetMachine.h"
+#include "DXILDataScalarization.h"
 #include "DXILIntrinsicExpansion.h"
 #include "DXILOpLowering.h"
 #include "DXILPrettyPrinter.h"

llvm/lib/Transforms/Scalar/Scalarizer.cpp

@@ -39,7 +39,6 @@
 #include "llvm/IR/Value.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/Casting.h"
-#include "llvm/Support/CommandLine.h"
 #include "llvm/Transforms/Utils/Local.h"
 #include <cassert>
 #include <cstdint>
@@ -51,28 +50,6 @@ using namespace llvm;
 
 #define DEBUG_TYPE "scalarizer"
 
-static cl::opt<bool> ClScalarizeVariableInsertExtract(
-    "scalarize-variable-insert-extract", cl::init(true), cl::Hidden,
-    cl::desc("Allow the scalarizer pass to scalarize "
-             "insertelement/extractelement with variable index"));
-
-// This is disabled by default because having separate loads and stores
-// makes it more likely that the -combiner-alias-analysis limits will be
-// reached.
-static cl::opt<bool> ClScalarizeLoadStore(
-    "scalarize-load-store", cl::init(false), cl::Hidden,
-    cl::desc("Allow the scalarizer pass to scalarize loads and store"));
-
-// Split vectors larger than this size into fragments, where each fragment is
-// either a vector no larger than this size or a scalar.
-//
-// Instructions with operands or results of different sizes that would be split
-// into a different number of fragments are currently left as-is.
-static cl::opt<unsigned> ClScalarizeMinBits(
-    "scalarize-min-bits", cl::init(0), cl::Hidden,
-    cl::desc("Instruct the scalarizer pass to attempt to keep values of a "
-             "minimum number of bits"));
-
 namespace {
 
 BasicBlock::iterator skipPastPhiNodesAndDbg(BasicBlock::iterator Itr) {
@@ -273,24 +250,14 @@ static Value *concatenate(IRBuilder<> &Builder, ArrayRef<Value *> Fragments,
   return Res;
 }
 
-template <typename T>
-T getWithDefaultOverride(const cl::opt<T> &ClOption,
-                         const std::optional<T> &DefaultOverride) {
-  return ClOption.getNumOccurrences() ? ClOption
-                                      : DefaultOverride.value_or(ClOption);
-}
-
 class ScalarizerVisitor : public InstVisitor<ScalarizerVisitor, bool> {
 public:
   ScalarizerVisitor(DominatorTree *DT, const TargetTransformInfo *TTI,
                     ScalarizerPassOptions Options)
-      : DT(DT), TTI(TTI), ScalarizeVariableInsertExtract(getWithDefaultOverride(
-                              ClScalarizeVariableInsertExtract,
-                              Options.ScalarizeVariableInsertExtract)),
-        ScalarizeLoadStore(getWithDefaultOverride(ClScalarizeLoadStore,
-                                                  Options.ScalarizeLoadStore)),
-        ScalarizeMinBits(getWithDefaultOverride(ClScalarizeMinBits,
-                                                Options.ScalarizeMinBits)) {}
+      : DT(DT), TTI(TTI),
+        ScalarizeVariableInsertExtract(Options.ScalarizeVariableInsertExtract),
+        ScalarizeLoadStore(Options.ScalarizeLoadStore),
+        ScalarizeMinBits(Options.ScalarizeMinBits) {}
 
   bool visit(Function &F);
 

llvm/test/CodeGen/DirectX/rsqrt.ll

@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt -S -scalarizer -dxil-op-lower -mtriple=dxil-pc-shadermodel6.3-library %s | FileCheck %s
+; RUN: opt -S -passes='function(scalarizer<load-store>),dxil-op-lower' -mtriple=dxil-pc-shadermodel6.3-library %s | FileCheck %s
 
 ; Make sure dxil operation function calls for rsqrt are generated for float and half.
 

llvm/test/CodeGen/DirectX/scalar-data.ll

@@ -1,4 +1,4 @@
-; RUN: opt -S -dxil-data-scalarization -scalarizer -scalarize-load-store -dxil-op-lower -mtriple=dxil-pc-shadermodel6.3-library %s | FileCheck %s
+; RUN: opt -S -passes='dxil-data-scalarization,function(scalarizer<load-store>),dxil-op-lower' -mtriple=dxil-pc-shadermodel6.3-library %s | FileCheck %s
 ; RUN: llc %s -mtriple=dxil-pc-shadermodel6.3-library --filetype=asm -o - | FileCheck %s
 
 ; Make sure we don't touch arrays without vectors and that can recurse multiple-dimension arrays of vectors

llvm/test/CodeGen/DirectX/scalar-load.ll

@@ -1,4 +1,4 @@
-; RUN: opt -S -dxil-data-scalarization -scalarizer -scalarize-load-store -dxil-op-lower -mtriple=dxil-pc-shadermodel6.3-library %s | FileCheck %s
+; RUN: opt -S -passes='dxil-data-scalarization,function(scalarizer<load-store>),dxil-op-lower' -mtriple=dxil-pc-shadermodel6.3-library %s | FileCheck %s
 ; RUN: llc %s -mtriple=dxil-pc-shadermodel6.3-library --filetype=asm -o - | FileCheck %s
 
 ; Make sure we can load groupshared, static vectors and arrays of vectors

llvm/test/CodeGen/DirectX/scalar-store.ll

@@ -1,4 +1,4 @@
-; RUN: opt -S -dxil-data-scalarization -scalarizer -scalarize-load-store -dxil-op-lower -mtriple=dxil-pc-shadermodel6.3-library %s | FileCheck %s
+; RUN: opt -S -passes='dxil-data-scalarization,scalarizer<load-store>,dxil-op-lower' -mtriple=dxil-pc-shadermodel6.3-library %s | FileCheck %s
 ; RUN: llc %s -mtriple=dxil-pc-shadermodel6.3-library --filetype=asm -o - | FileCheck %s
 
 ; Make sure we can store groupshared, static vectors and arrays of vectors