[llvm] Scalarizer: Replace cl::opts with pass parameters (PR #110645)

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 1 11:57:54 PDT 2024


https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/110645

>From c80ce0abca84a45c93d262fd3599e266e1f972ce Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Tue, 1 Oct 2024 13:40:22 +0400
Subject: [PATCH 1/2] Scalarizer: Replace cl::opts with pass parameters

Preserve the existing defaults (although load-store defaulting
to false is a really bad one). Also migrate DirectX tests to new PM.
---
 .../llvm/Transforms/Scalar/Scalarizer.h       | 26 ++++++++----
 llvm/lib/Passes/PassBuilder.cpp               | 31 ++++++++++++++
 llvm/lib/Passes/PassRegistry.def              |  7 +++-
 .../Target/DirectX/DirectXPassRegistry.def    |  1 +
 .../Target/DirectX/DirectXTargetMachine.cpp   |  1 +
 llvm/lib/Transforms/Scalar/Scalarizer.cpp     | 41 ++-----------------
 llvm/test/CodeGen/DirectX/rsqrt.ll            |  2 +-
 llvm/test/CodeGen/DirectX/scalar-data.ll      |  2 +-
 llvm/test/CodeGen/DirectX/scalar-load.ll      |  2 +-
 llvm/test/CodeGen/DirectX/scalar-store.ll     |  2 +-
 .../Scalarizer/basic-inseltpoison.ll          |  2 +-
 llvm/test/Transforms/Scalarizer/basic.ll      |  2 +-
 .../Scalarizer/constant-extractelement.ll     |  2 +-
 .../Scalarizer/constant-insertelement.ll      |  2 +-
 llvm/test/Transforms/Scalarizer/dbginfo.ll    |  2 +-
 llvm/test/Transforms/Scalarizer/min-bits.ll   |  4 +-
 .../Transforms/Scalarizer/opaque-ptr-bug.ll   |  2 +-
 .../Scalarizer/pass-param-parse-errors.ll     |  8 ++++
 .../Transforms/Scalarizer/scatter-order.ll    |  2 +-
 llvm/test/Transforms/Scalarizer/store-bug.ll  |  2 +-
 .../Scalarizer/variable-extractelement.ll     |  4 +-
 .../Scalarizer/variable-insertelement.ll      |  4 +-
 .../Scalarizer/vector-of-pointer-to-vector.ll |  2 +-
 23 files changed, 89 insertions(+), 64 deletions(-)
 create mode 100644 llvm/test/Transforms/Scalarizer/pass-param-parse-errors.ll

diff --git a/llvm/include/llvm/Transforms/Scalar/Scalarizer.h b/llvm/include/llvm/Transforms/Scalar/Scalarizer.h
index 4d2a1a2f889a3c..401ca4d995348c 100644
--- a/llvm/include/llvm/Transforms/Scalar/Scalarizer.h
+++ b/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> {
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 402b2340e5c35f..01655962ec81bc 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -1047,6 +1047,37 @@ 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(';');
+
+    bool Enable = !ParamName.consume_front("no-");
+
+    if (ParamName == "load-store")
+      Result.ScalarizeLoadStore = Enable;
+    else if (ParamName == "variable-insert-extract")
+      Result.ScalarizeVariableInsertExtract = Enable;
+    else if (Enable && 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());
+      }
+    } 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;
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 3dc7f185f330c5..a5cc2ac5fc8e8e 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/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;no-min-bits=N;min-bits=N;")
 FUNCTION_PASS_WITH_PARAMS(
     "separate-const-offset-from-gep", "SeparateConstOffsetFromGEPPass",
     [](bool LowerGEP) { return SeparateConstOffsetFromGEPPass(LowerGEP); },
diff --git a/llvm/lib/Target/DirectX/DirectXPassRegistry.def b/llvm/lib/Target/DirectX/DirectXPassRegistry.def
index a3e051b173d890..ae729a1082b867 100644
--- a/llvm/lib/Target/DirectX/DirectXPassRegistry.def
+++ b/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()))
diff --git a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
index f358215ecf3735..18251ea3bd01d3 100644
--- a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
+++ b/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"
diff --git a/llvm/lib/Transforms/Scalar/Scalarizer.cpp b/llvm/lib/Transforms/Scalar/Scalarizer.cpp
index da9b804e94a74a..ee86e2e6c9751e 100644
--- a/llvm/lib/Transforms/Scalar/Scalarizer.cpp
+++ b/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);
 
diff --git a/llvm/test/CodeGen/DirectX/rsqrt.ll b/llvm/test/CodeGen/DirectX/rsqrt.ll
index 26b22e19635af2..612b6222e7594e 100644
--- a/llvm/test/CodeGen/DirectX/rsqrt.ll
+++ b/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.
 
diff --git a/llvm/test/CodeGen/DirectX/scalar-data.ll b/llvm/test/CodeGen/DirectX/scalar-data.ll
index 4438604a3a8797..c436f1eae4425d 100644
--- a/llvm/test/CodeGen/DirectX/scalar-data.ll
+++ b/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
diff --git a/llvm/test/CodeGen/DirectX/scalar-load.ll b/llvm/test/CodeGen/DirectX/scalar-load.ll
index 11678f48a5e015..612e5646f5a081 100644
--- a/llvm/test/CodeGen/DirectX/scalar-load.ll
+++ b/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
diff --git a/llvm/test/CodeGen/DirectX/scalar-store.ll b/llvm/test/CodeGen/DirectX/scalar-store.ll
index 08d8a2c57c6c33..7734d32bca58cd 100644
--- a/llvm/test/CodeGen/DirectX/scalar-store.ll
+++ b/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
diff --git a/llvm/test/Transforms/Scalarizer/basic-inseltpoison.ll b/llvm/test/Transforms/Scalarizer/basic-inseltpoison.ll
index bbcdcb6f586742..6cb94e8f561bcf 100644
--- a/llvm/test/Transforms/Scalarizer/basic-inseltpoison.ll
+++ b/llvm/test/Transforms/Scalarizer/basic-inseltpoison.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-load-store -S | FileCheck %s
+; RUN: opt %s -passes='function(scalarizer<load-store>,dce)' -S | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 declare <4 x float> @ext(<4 x float>)
diff --git a/llvm/test/Transforms/Scalarizer/basic.ll b/llvm/test/Transforms/Scalarizer/basic.ll
index 28d5933ab9b87c..190e8a089a5f62 100644
--- a/llvm/test/Transforms/Scalarizer/basic.ll
+++ b/llvm/test/Transforms/Scalarizer/basic.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-load-store -S | FileCheck %s
+; RUN: opt %s -passes='function(scalarizer<load-store>,dce)' -S | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 declare <4 x float> @ext(<4 x float>)
diff --git a/llvm/test/Transforms/Scalarizer/constant-extractelement.ll b/llvm/test/Transforms/Scalarizer/constant-extractelement.ll
index 50b7c3904999c8..ad1561e8bae83a 100644
--- a/llvm/test/Transforms/Scalarizer/constant-extractelement.ll
+++ b/llvm/test/Transforms/Scalarizer/constant-extractelement.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-load-store -S | FileCheck --check-prefixes=ALL %s
+; RUN: opt %s -passes='function(scalarizer<load-store>,dce)' -S | FileCheck --check-prefixes=ALL %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
diff --git a/llvm/test/Transforms/Scalarizer/constant-insertelement.ll b/llvm/test/Transforms/Scalarizer/constant-insertelement.ll
index afb6d6a49296bc..c1871ac92d255c 100644
--- a/llvm/test/Transforms/Scalarizer/constant-insertelement.ll
+++ b/llvm/test/Transforms/Scalarizer/constant-insertelement.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-load-store -S | FileCheck --check-prefixes=ALL %s
+; RUN: opt %s -passes='function(scalarizer<load-store>,dce)' -S | FileCheck --check-prefixes=ALL %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
diff --git a/llvm/test/Transforms/Scalarizer/dbginfo.ll b/llvm/test/Transforms/Scalarizer/dbginfo.ll
index 310b5aae02cf45..c388805fa6a2ea 100644
--- a/llvm/test/Transforms/Scalarizer/dbginfo.ll
+++ b/llvm/test/Transforms/Scalarizer/dbginfo.ll
@@ -1,4 +1,4 @@
-; RUN: opt %s -passes='function(scalarizer)' -scalarize-load-store -S --experimental-debuginfo-iterators=false | FileCheck %s
+; RUN: opt %s -passes='function(scalarizer<load-store>)' -S --experimental-debuginfo-iterators=false | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 ; FIXME: the test output here changes if we use the RemoveDIs non-intrinsic
 ; debug-info format for the test. Specifically, the intrinsics no longer
diff --git a/llvm/test/Transforms/Scalarizer/min-bits.ll b/llvm/test/Transforms/Scalarizer/min-bits.ll
index be901d4990592c..97cc71626e2084 100644
--- a/llvm/test/Transforms/Scalarizer/min-bits.ll
+++ b/llvm/test/Transforms/Scalarizer/min-bits.ll
@@ -1,6 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-load-store -scalarize-min-bits=16 -S | FileCheck %s --check-prefixes=CHECK,MIN16
-; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-load-store -scalarize-min-bits=32 -S | FileCheck %s --check-prefixes=CHECK,MIN32
+; RUN: opt %s -passes='function(scalarizer<load-store;min-bits=16>,dce)' -S | FileCheck %s --check-prefixes=CHECK,MIN16
+; RUN: opt %s -passes='function(scalarizer<load-store;min-bits=32>,dce)' -S | FileCheck %s --check-prefixes=CHECK,MIN32
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 define void @load_add_store_v2i16(ptr %pa, ptr %pb) {
diff --git a/llvm/test/Transforms/Scalarizer/opaque-ptr-bug.ll b/llvm/test/Transforms/Scalarizer/opaque-ptr-bug.ll
index 7d0ba0aa45f05b..7d187dc33db881 100644
--- a/llvm/test/Transforms/Scalarizer/opaque-ptr-bug.ll
+++ b/llvm/test/Transforms/Scalarizer/opaque-ptr-bug.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt %s -passes='scalarizer,dce' -S -scalarize-load-store -o - | FileCheck %s
+; RUN: opt %s -passes='scalarizer<load-store>,dce' -S -o - | FileCheck %s
 
 ; This used to crash because the same (pointer) value was scattered by
 ; different amounts.
diff --git a/llvm/test/Transforms/Scalarizer/pass-param-parse-errors.ll b/llvm/test/Transforms/Scalarizer/pass-param-parse-errors.ll
new file mode 100644
index 00000000000000..5ca138be22a12e
--- /dev/null
+++ b/llvm/test/Transforms/Scalarizer/pass-param-parse-errors.ll
@@ -0,0 +1,8 @@
+; RUN: not opt -passes='scalarizer<unknown>' -disable-output %s 2>&1 | FileCheck -check-prefix=UNKNOWNERR %s
+; RUN: not opt -passes='scalarizer<;>' -disable-output %s 2>&1 | FileCheck -check-prefix=UNKNOWNERR %s
+; RUN: not opt -passes='scalarizer<min-bits=>' -disable-output %s 2>&1 | FileCheck -check-prefix=MINBITS-EMPTY-ERR %s
+; RUN: not opt -passes='scalarizer<min-bits=x>' -disable-output %s 2>&1 | FileCheck -check-prefix=MINBITS-NOTINT-ERR %s
+
+; UNKNOWNERR: invalid Scalarizer pass parameter '{{.*}}'
+; MINBITS-EMPTY-ERR: invalid argument to Scalarizer pass min-bits parameter: ''
+; MINBITS-NOTINT-ERR: invalid argument to Scalarizer pass min-bits parameter: 'x'
diff --git a/llvm/test/Transforms/Scalarizer/scatter-order.ll b/llvm/test/Transforms/Scalarizer/scatter-order.ll
index 1861b0f38446e6..a98a2198b1ed28 100644
--- a/llvm/test/Transforms/Scalarizer/scatter-order.ll
+++ b/llvm/test/Transforms/Scalarizer/scatter-order.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt %s -passes='function(scalarizer)' -scalarize-load-store -S | FileCheck %s
+; RUN: opt %s -passes='function(scalarizer<load-store>)' -S | FileCheck %s
 
 ; This verifies that the order of extract element instructions is
 ; deterministic. In the past we could end up with different results depending
diff --git a/llvm/test/Transforms/Scalarizer/store-bug.ll b/llvm/test/Transforms/Scalarizer/store-bug.ll
index 1fccdc624e4930..6d9cd46db53905 100644
--- a/llvm/test/Transforms/Scalarizer/store-bug.ll
+++ b/llvm/test/Transforms/Scalarizer/store-bug.ll
@@ -1,4 +1,4 @@
-; RUN: opt -passes='function(scalarizer)' -scalarize-load-store -S < %s | FileCheck %s
+; RUN: opt -passes='function(scalarizer<load-store>)' -S < %s | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 ; This input caused the scalarizer not to clear cached results
diff --git a/llvm/test/Transforms/Scalarizer/variable-extractelement.ll b/llvm/test/Transforms/Scalarizer/variable-extractelement.ll
index 85361cdd6942d3..9211ff91809c92 100644
--- a/llvm/test/Transforms/Scalarizer/variable-extractelement.ll
+++ b/llvm/test/Transforms/Scalarizer/variable-extractelement.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt %s -passes='function(scalarizer,dce)' -S | FileCheck --check-prefix=DEFAULT %s
-; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-variable-insert-extract=false -S | FileCheck --check-prefix=OFF %s
-; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-variable-insert-extract=true -S | FileCheck --check-prefix=DEFAULT %s
+; RUN: opt %s -passes='function(scalarizer<no-variable-insert-extract>,dce)' -S | FileCheck --check-prefix=OFF %s
+; RUN: opt %s -passes='function(scalarizer<variable-insert-extract>,dce)' -S | FileCheck --check-prefix=DEFAULT %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
diff --git a/llvm/test/Transforms/Scalarizer/variable-insertelement.ll b/llvm/test/Transforms/Scalarizer/variable-insertelement.ll
index 452a94f3017587..13122657932a05 100644
--- a/llvm/test/Transforms/Scalarizer/variable-insertelement.ll
+++ b/llvm/test/Transforms/Scalarizer/variable-insertelement.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt %s -passes='function(scalarizer,dce)' -S | FileCheck --check-prefix=DEFAULT %s
-; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-variable-insert-extract=false -S | FileCheck --check-prefix=OFF %s
-; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-variable-insert-extract=true -S | FileCheck --check-prefix=DEFAULT %s
+; RUN: opt %s -passes='function(scalarizer<no-variable-insert-extract>,dce)' -S | FileCheck --check-prefix=OFF %s
+; RUN: opt %s -passes='function(scalarizer<variable-insert-extract>,dce)' -S | FileCheck --check-prefix=DEFAULT %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
diff --git a/llvm/test/Transforms/Scalarizer/vector-of-pointer-to-vector.ll b/llvm/test/Transforms/Scalarizer/vector-of-pointer-to-vector.ll
index 89ae34193257a9..b1b71ecc801f48 100644
--- a/llvm/test/Transforms/Scalarizer/vector-of-pointer-to-vector.ll
+++ b/llvm/test/Transforms/Scalarizer/vector-of-pointer-to-vector.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-load-store -S | FileCheck %s
+; RUN: opt %s -passes='function(scalarizer<load-store>,dce)' -S | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 define <1 x i32> @f1(<1 x ptr> %src, i32 %index) {

>From d3a3cf2895881d0ec2fb32bf14265fde1b4ada57 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Tue, 1 Oct 2024 22:56:25 +0400
Subject: [PATCH 2/2] Don't try to handle no-min-bits

---
 llvm/lib/Passes/PassBuilder.cpp                | 18 +++++++++++-------
 llvm/lib/Passes/PassRegistry.def               |  2 +-
 .../Scalarizer/pass-param-parse-errors.ll      |  1 +
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 01655962ec81bc..c3429c8d6841b8 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -1053,13 +1053,7 @@ Expected<ScalarizerPassOptions> parseScalarizerOptions(StringRef Params) {
     StringRef ParamName;
     std::tie(ParamName, Params) = Params.split(';');
 
-    bool Enable = !ParamName.consume_front("no-");
-
-    if (ParamName == "load-store")
-      Result.ScalarizeLoadStore = Enable;
-    else if (ParamName == "variable-insert-extract")
-      Result.ScalarizeVariableInsertExtract = Enable;
-    else if (Enable && ParamName.consume_front("min-bits=")) {
+    if (ParamName.consume_front("min-bits=")) {
       if (ParamName.getAsInteger(0, Result.ScalarizeMinBits)) {
         return make_error<StringError>(
             formatv("invalid argument to Scalarizer pass min-bits "
@@ -1068,6 +1062,16 @@ Expected<ScalarizerPassOptions> parseScalarizerOptions(StringRef Params) {
                 .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
     } else {
       return make_error<StringError>(
           formatv("invalid Scalarizer pass parameter '{0}' ", ParamName).str(),
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index a5cc2ac5fc8e8e..90859c18c4f499 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -577,7 +577,7 @@ FUNCTION_PASS_WITH_PARAMS(
     [](ScalarizerPassOptions Opts) { return ScalarizerPass(Opts); },
     parseScalarizerOptions,
     "load-store;no-load-store;variable-insert-extract;"
-    "no-variable-insert-extract;no-min-bits=N;min-bits=N;")
+    "no-variable-insert-extract;min-bits=N;")
 FUNCTION_PASS_WITH_PARAMS(
     "separate-const-offset-from-gep", "SeparateConstOffsetFromGEPPass",
     [](bool LowerGEP) { return SeparateConstOffsetFromGEPPass(LowerGEP); },
diff --git a/llvm/test/Transforms/Scalarizer/pass-param-parse-errors.ll b/llvm/test/Transforms/Scalarizer/pass-param-parse-errors.ll
index 5ca138be22a12e..7a241dac6ada13 100644
--- a/llvm/test/Transforms/Scalarizer/pass-param-parse-errors.ll
+++ b/llvm/test/Transforms/Scalarizer/pass-param-parse-errors.ll
@@ -2,6 +2,7 @@
 ; RUN: not opt -passes='scalarizer<;>' -disable-output %s 2>&1 | FileCheck -check-prefix=UNKNOWNERR %s
 ; RUN: not opt -passes='scalarizer<min-bits=>' -disable-output %s 2>&1 | FileCheck -check-prefix=MINBITS-EMPTY-ERR %s
 ; RUN: not opt -passes='scalarizer<min-bits=x>' -disable-output %s 2>&1 | FileCheck -check-prefix=MINBITS-NOTINT-ERR %s
+; RUN: not opt -passes='scalarizer<no-min-bits=10>' -disable-output %s 2>&1 | FileCheck -check-prefix=UNKNOWNERR %s
 
 ; UNKNOWNERR: invalid Scalarizer pass parameter '{{.*}}'
 ; MINBITS-EMPTY-ERR: invalid argument to Scalarizer pass min-bits parameter: ''



More information about the llvm-commits mailing list