IRNormalizer: Replace cl::opts with pass parameters (#133874)

arsenm · web-flow · commit 7e25b2407314 · 2025-04-01T23:27:20.000+07:00
Not sure why the "fold-all" option naming didn't match the
variable "FoldPreOutputs", but I've preserved the difference.

More annoyingly, the pass name "normalize" does not match the pass
name IRNormalizer and should probably be fixed one way or the other.

Also the existing test coverage for the flags is lacking. I've added
a test that shows they parse, but we should have tests that they
do something.
diff --git a/llvm/include/llvm/Transforms/Utils/IRNormalizer.h b/llvm/include/llvm/Transforms/Utils/IRNormalizer.h
@@ -5,8 +5,29 @@
 
 namespace llvm {
 
+struct IRNormalizerOptions {
+  /// Preserves original instruction order.
+  bool PreserveOrder = false;
+
+  /// Renames all instructions (including user-named)
+  bool RenameAll = true;
+
+  /// Folds all regular instructions (including pre-outputs)
+  bool FoldPreOutputs = true;
+
+  /// Sorts and reorders operands in commutative instructions
+  bool ReorderOperands = true;
+};
+
 /// IRNormalizer aims to transform LLVM IR into normal form.
 struct IRNormalizerPass : public PassInfoMixin<IRNormalizerPass> {
+private:
+  const IRNormalizerOptions Options;
+
+public:
+  IRNormalizerPass(IRNormalizerOptions Options = IRNormalizerOptions())
+      : Options(Options) {}
+
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM) const;
 };
 
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
@@ -793,6 +793,31 @@ Expected<bool> parseLowerMatrixIntrinsicsPassOptions(StringRef Params) {
                                             "LowerMatrixIntrinsics");
 }
 
+Expected<IRNormalizerOptions> parseIRNormalizerPassOptions(StringRef Params) {
+  IRNormalizerOptions Result;
+  while (!Params.empty()) {
+    StringRef ParamName;
+    std::tie(ParamName, Params) = Params.split(';');
+
+    bool Enable = !ParamName.consume_front("no-");
+    if (ParamName == "preserve-order")
+      Result.PreserveOrder = Enable;
+    else if (ParamName == "rename-all")
+      Result.RenameAll = Enable;
+    else if (ParamName == "fold-all") // FIXME: Name mismatch
+      Result.FoldPreOutputs = Enable;
+    else if (ParamName == "reorder-operands")
+      Result.ReorderOperands = Enable;
+    else {
+      return make_error<StringError>(
+          formatv("invalid normalize pass parameter '{0}' ", ParamName).str(),
+          inconvertibleErrorCode());
+    }
+  }
+
+  return Result;
+}
+
 Expected<AddressSanitizerOptions> parseASanPassOptions(StringRef Params) {
   AddressSanitizerOptions Result;
   while (!Params.empty()) {
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
@@ -422,7 +422,6 @@ FUNCTION_PASS("move-auto-init", MoveAutoInitPass())
 FUNCTION_PASS("nary-reassociate", NaryReassociatePass())
 FUNCTION_PASS("newgvn", NewGVNPass())
 FUNCTION_PASS("no-op-function", NoOpFunctionPass())
-FUNCTION_PASS("normalize", IRNormalizerPass())
 FUNCTION_PASS("objc-arc", ObjCARCOptPass())
 FUNCTION_PASS("objc-arc-contract", ObjCARCContractPass())
 FUNCTION_PASS("objc-arc-expand", ObjCARCExpandPass())
@@ -568,6 +567,11 @@ FUNCTION_PASS_WITH_PARAMS(
     "lower-matrix-intrinsics", "LowerMatrixIntrinsicsPass",
     [](bool Minimal) { return LowerMatrixIntrinsicsPass(Minimal); },
     parseLowerMatrixIntrinsicsPassOptions, "minimal")
+FUNCTION_PASS_WITH_PARAMS(
+    "normalize", "IRNormalizerPass",
+    [](IRNormalizerOptions Options) { return IRNormalizerPass(Options); },
+    parseIRNormalizerPassOptions,
+    "no-preserve-order;preserve-order;no-rename-all;rename-all;no-fold-all;fold-all;no-reorder-operands;reorder-operands")
 FUNCTION_PASS_WITH_PARAMS(
     "mldst-motion", "MergedLoadStoreMotionPass",
     [](MergedLoadStoreMotionOptions Opts) {
diff --git a/llvm/lib/Transforms/Utils/IRNormalizer.cpp b/llvm/lib/Transforms/Utils/IRNormalizer.cpp
@@ -40,21 +40,13 @@ namespace {
 /// IRNormalizer aims to transform LLVM IR into normal form.
 class IRNormalizer {
 public:
-  /// \name Normalizer flags.
-  /// @{
-  /// Preserves original order of instructions.
-  static cl::opt<bool> PreserveOrder;
-  /// Renames all instructions (including user-named).
-  static cl::opt<bool> RenameAll; // TODO: Don't rename on empty name
-  /// Folds all regular instructions (including pre-outputs).
-  static cl::opt<bool> FoldPreOutputs;
-  /// Sorts and reorders operands in commutative instructions.
-  static cl::opt<bool> ReorderOperands;
-  /// @}
-
   bool runOnFunction(Function &F);
 
+  IRNormalizer(IRNormalizerOptions Options) : Options(Options) {}
+
 private:
+  const IRNormalizerOptions Options;
+
   // Random constant for hashing, so the state isn't zero.
   const uint64_t MagicHashConstant = 0x6acaa36bef8325c5ULL;
   DenseSet<const Instruction *> NamedInstructions;
@@ -96,19 +88,6 @@ class IRNormalizer {
 };
 } // namespace
 
-cl::opt<bool> IRNormalizer::PreserveOrder(
-    "norm-preserve-order", cl::Hidden, cl::init(false),
-    cl::desc("Preserves original instruction order"));
-cl::opt<bool> IRNormalizer::RenameAll(
-    "norm-rename-all", cl::Hidden, cl::init(true),
-    cl::desc("Renames all instructions (including user-named)"));
-cl::opt<bool> IRNormalizer::FoldPreOutputs(
-    "norm-fold-all", cl::Hidden, cl::init(true),
-    cl::desc("Folds all regular instructions (including pre-outputs)"));
-cl::opt<bool> IRNormalizer::ReorderOperands(
-    "norm-reorder-operands", cl::Hidden, cl::init(true),
-    cl::desc("Sorts and reorders operands in commutative instructions"));
-
 /// Entry method to the IRNormalizer.
 ///
 /// \param F Function to normalize.
@@ -118,7 +97,7 @@ bool IRNormalizer::runOnFunction(Function &F) {
 
   Outputs = collectOutputInstructions(F);
 
-  if (!PreserveOrder)
+  if (!Options.PreserveOrder)
     reorderInstructions(F);
 
   // TODO: Reorder basic blocks via a topological sort.
@@ -127,8 +106,8 @@ bool IRNormalizer::runOnFunction(Function &F) {
     nameInstruction(I);
 
   for (auto &I : instructions(F)) {
-    if (!PreserveOrder) {
-      if (ReorderOperands)
+    if (!Options.PreserveOrder) {
+      if (Options.ReorderOperands)
         reorderInstructionOperandsByNames(&I);
 
       if (auto *Phi = dyn_cast<PHINode>(&I))
@@ -146,7 +125,7 @@ bool IRNormalizer::runOnFunction(Function &F) {
 void IRNormalizer::nameFunctionArguments(Function &F) const {
   int ArgumentCounter = 0;
   for (auto &A : F.args()) {
-    if (RenameAll || A.getName().empty()) {
+    if (Options.RenameAll || A.getName().empty()) {
       A.setName("a" + Twine(ArgumentCounter));
       ArgumentCounter += 1;
     }
@@ -167,7 +146,7 @@ void IRNormalizer::nameBasicBlocks(Function &F) const {
       if (isOutput(&I))
         Hash = hashing::detail::hash_16_bytes(Hash, I.getOpcode());
 
-    if (RenameAll || B.getName().empty()) {
+    if (Options.RenameAll || B.getName().empty()) {
       // Name basic block. Substring hash to make diffs more readable.
       B.setName("bb" + std::to_string(Hash).substr(0, 5));
     }
@@ -219,7 +198,7 @@ void IRNormalizer::sortCommutativeOperands(Instruction *I, T &Operands) const {
 void IRNormalizer::nameAsInitialInstruction(Instruction *I) const {
   if (I->getType()->isVoidTy())
     return;
-  if (!(I->getName().empty() || RenameAll))
+  if (!(I->getName().empty() || Options.RenameAll))
     return;
   LLVM_DEBUG(dbgs() << "Naming initial instruction: " << *I << "\n");
 
@@ -359,7 +338,7 @@ void IRNormalizer::nameAsRegularInstruction(Instruction *I) {
   }
   Name.append(")");
 
-  if ((I->getName().empty() || RenameAll) && !I->getType()->isVoidTy())
+  if ((I->getName().empty() || Options.RenameAll) && !I->getType()->isVoidTy())
     I->setName(Name);
 }
 
@@ -379,7 +358,7 @@ void IRNormalizer::nameAsRegularInstruction(Instruction *I) {
 void IRNormalizer::foldInstructionName(Instruction *I) const {
   // If this flag is raised, fold all regular
   // instructions (including pre-outputs).
-  if (!FoldPreOutputs) {
+  if (!Options.FoldPreOutputs) {
     // Don't fold if one of the users is an output instruction.
     for (auto *U : I->users())
       if (auto *IU = dyn_cast<Instruction>(U))
@@ -690,7 +669,7 @@ SetVector<int> IRNormalizer::getOutputFootprint(
 
 PreservedAnalyses IRNormalizerPass::run(Function &F,
                                         FunctionAnalysisManager &AM) const {
-  IRNormalizer{}.runOnFunction(F);
+  IRNormalizer(Options).runOnFunction(F);
   PreservedAnalyses PA;
   PA.preserveSet<CFGAnalyses>();
   return PA;
diff --git a/llvm/test/Transforms/IRNormalizer/pass-parameters.ll b/llvm/test/Transforms/IRNormalizer/pass-parameters.ll
@@ -0,0 +1,21 @@
+; RUN: not opt -S -passes='normalize<invalid>' %s 2>&1 | FileCheck -check-prefix=ERR %s
+; RUN: opt -S -passes=normalize < %s | FileCheck %s
+; RUN: opt -S -passes='normalize<>' < %s | FileCheck %s
+; RUN: opt -S -passes='normalize<preserve-order;rename-all;fold-all;reorder-operands>' < %s | FileCheck %s
+; RUN: opt -S -passes='normalize<no-preserve-order;no-rename-all;no-fold-all;no-reorder-operands>' < %s | FileCheck %s
+
+; FIXME: This verifies all the pass parameter names parse, but not
+; that they work as expected.
+
+; ERR: invalid normalize pass parameter 'invalid'
+
+; CHECK: define i32 @0(i32 %a0, i32 %a1) {
+; CHECK-NEXT: bb17254:
+; CHECK-NEXT: %"vl12603(%a0, %a1)" = add i32 %a0, %a1
+; CHECK-NEXT: ret i32 %"vl12603(%a0, %a1)"
+; CHECK-NEXT: }
+define i32 @0(i32, i32) {
+  %3 = add i32 %0, %1
+  ret i32 %3
+}
+
diff --git a/llvm/test/Transforms/IRNormalizer/reordering.ll b/llvm/test/Transforms/IRNormalizer/reordering.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
-; RUN: opt -S -passes=normalize -verify-each -norm-rename-all=false < %s | FileCheck %s
+; RUN: opt -S -passes='normalize<no-rename-all>' -verify-each < %s | FileCheck %s
 
 define void @foo() {
 ; CHECK-LABEL: define void @foo() {
