[llvm] [SandboxVec] Add pass to create Regions from metadata. Generalize SandboxVec pass pipelines. (PR #112288)

Jorge Gorbe Moya via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 15 13:15:57 PDT 2024


https://github.com/slackito updated https://github.com/llvm/llvm-project/pull/112288

>From e8b01e9d8dbdfba3b30a196e40a38e122d113884 Mon Sep 17 00:00:00 2001
From: Jorge Gorbe Moya <jgorbe at google.com>
Date: Mon, 14 Oct 2024 16:02:44 -0700
Subject: [PATCH 1/2] [SandboxVec] Add pass to create Regions from metadata.
 Generalize SandboxVec pass pipelines.

My previous attempt (#111904) hacked creation of Regions from metadata
into the bottom-up vectorizer. I got some feedback that it should be its
own pass. So now we have two SandboxIR function passes (`BottomUpVec`
and `RegionsFromMetadata`) that are interchangeable, and we could have
other SandboxIR function passes doing other kinds of transforms, so this
commit revamps pipeline creation and parsing.

First, `sandboxir::PassManager::setPassPipeline` now accepts pass
arguments in angle brackets. Pass arguments are arbitrary strings that
must be parsed by each pass, the only requirement is that nested angle
bracket pairs must be balanced, to allow for nested pipelines with more
arguments. For example:
```
    bottom-up-vec<region-pass-1,region-pass-2<arg>,region-pass-3>
```
This has complicated the parser a little bit (the loop over pipeline
characters now contains a small state machine), and we now have some new
test cases to exercise the new features.

The main SandboxVectorizerPass now contains a customizable pipeline of
SandboxIR function passes, defined by the `sbvec-passes` flag. Region
passes for the bottom-up vectorizer pass are now in pass arguments (like
in the example above).

Because we have now several classes that can build sub-pass pipelines,
I've moved the logic that interacts with PassRegistry.def into its own
files (PassBuilder.{h,cpp} so it can be easily reused.

Finally, I've added a `RegionsFromMetadata` function pass, which will
allow us to run region passes in isolation from lit tests without
relying on the bottom-up vectorizer, and a new lit test that does
exactly this.

Note that the new pipeline parser now allows empty pipelines. This is
useful for testing. For example, if we use
```
  -sbvec-passes="bottom-up-vec<>"
```
SandboxVectorizer converts LLVM IR to SandboxIR and runs the bottom-up
vectorizer, but no region passes afterwards.
```
  -sbvec-passes=""
```
SandboxVectorizer converts LLVM IR to SandboxIR and runs no passes on
it. This is useful to exercise SandboxIR conversion on its own.
---
 llvm/include/llvm/SandboxIR/Pass.h            |   2 +-
 llvm/include/llvm/SandboxIR/PassManager.h     | 122 +++++++++++++++---
 .../SandboxVectorizer/Passes/BottomUpVec.h    |  10 +-
 .../Passes/PrintInstructionCount.h            |  23 ++++
 .../Passes/RegionsFromMetadata.h              |  34 +++++
 .../SandboxVectorizer/SandboxVectorizer.h     |   6 +-
 .../SandboxVectorizerPassBuilder.h            |  32 +++++
 llvm/lib/Transforms/Vectorize/CMakeLists.txt  |   2 +
 .../SandboxVectorizer/Passes/BottomUpVec.cpp  |  40 +-----
 .../SandboxVectorizer/Passes/PassRegistry.def |  14 +-
 .../Passes/RegionsFromMetadata.cpp            |  30 +++++
 .../SandboxVectorizer/SandboxVectorizer.cpp   |  36 +++++-
 .../SandboxVectorizerPassBuilder.cpp          |  32 +++++
 .../default_pass_pipeline.ll                  |   1 +
 .../regions-from-metadata.ll                  |  15 +++
 .../SandboxVectorizer/user_pass_pipeline.ll   |   4 +-
 llvm/unittests/SandboxIR/PassTest.cpp         |  41 ++++--
 17 files changed, 369 insertions(+), 75 deletions(-)
 create mode 100644 llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h
 create mode 100644 llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h
 create mode 100644 llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h
 create mode 100644 llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
 create mode 100644 llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.cpp
 create mode 100644 llvm/test/Transforms/SandboxVectorizer/regions-from-metadata.ll

diff --git a/llvm/include/llvm/SandboxIR/Pass.h b/llvm/include/llvm/SandboxIR/Pass.h
index 211f10f5d57c5c..5ed9d7442ee70c 100644
--- a/llvm/include/llvm/SandboxIR/Pass.h
+++ b/llvm/include/llvm/SandboxIR/Pass.h
@@ -43,7 +43,7 @@ class Pass {
   LLVM_DUMP_METHOD virtual void dump() const;
 #endif
   /// Similar to print() but adds a newline. Used for testing.
-  void printPipeline(raw_ostream &OS) const { OS << Name << "\n"; }
+  virtual void printPipeline(raw_ostream &OS) const { OS << Name << "\n"; }
 };
 
 /// A pass that runs on a sandbox::Function.
diff --git a/llvm/include/llvm/SandboxIR/PassManager.h b/llvm/include/llvm/SandboxIR/PassManager.h
index 247c43615f5766..705bd356c5155e 100644
--- a/llvm/include/llvm/SandboxIR/PassManager.h
+++ b/llvm/include/llvm/SandboxIR/PassManager.h
@@ -50,40 +50,130 @@ class PassManager : public ParentPass {
   }
 
   using CreatePassFunc =
-      std::function<std::unique_ptr<ContainedPass>(StringRef)>;
+      std::function<std::unique_ptr<ContainedPass>(StringRef, StringRef)>;
 
   /// Parses \p Pipeline as a comma-separated sequence of pass names and sets
   /// the pass pipeline, using \p CreatePass to instantiate passes by name.
   ///
-  /// After calling this function, the PassManager contains only the specified
-  /// pipeline, any previously added passes are cleared.
+  /// Passes can have arguments, for example:
+  ///   "pass1<arg1,arg2>,pass2,pass3<arg3,arg4>"
+  ///
+  /// The arguments between angle brackets are treated as a mostly opaque string
+  /// and each pass is responsible for parsing its arguments. The exception to
+  /// this are nested angle brackets, which must match pair-wise to allow
+  /// arguments to contain nested pipelines, like:
+  ///
+  ///   "pass1<subpass1,subpass2<arg1,arg2>,subpass3>"
+  ///
+  /// An empty args string is treated the same as no args, so "pass" and
+  /// "pass<>" are equivalent.
   void setPassPipeline(StringRef Pipeline, CreatePassFunc CreatePass) {
     static constexpr const char EndToken = '\0';
+    static constexpr const char BeginArgsToken = '<';
+    static constexpr const char EndArgsToken = '>';
     static constexpr const char PassDelimToken = ',';
 
     assert(Passes.empty() &&
            "setPassPipeline called on a non-empty sandboxir::PassManager");
-    // Add EndToken to the end to ease parsing.
-    std::string PipelineStr = std::string(Pipeline) + EndToken;
-    int FlagBeginIdx = 0;
 
-    for (auto [Idx, C] : enumerate(PipelineStr)) {
-      // Keep moving Idx until we find the end of the pass name.
-      bool FoundDelim = C == EndToken || C == PassDelimToken;
-      if (!FoundDelim)
-        continue;
-      unsigned Sz = Idx - FlagBeginIdx;
-      std::string PassName(&PipelineStr[FlagBeginIdx], Sz);
-      FlagBeginIdx = Idx + 1;
+    // Accept an empty pipeline as a special case. This can be useful, for
+    // example, to test conversion to SandboxIR without running any passes on
+    // it.
+    if (Pipeline.empty())
+      return;
 
+    // Add EndToken to the end to ease parsing.
+    std::string PipelineStr = std::string(Pipeline) + EndToken;
+    Pipeline = StringRef(PipelineStr);
+
+    enum {
+      ScanName,  // reading a pass name
+      ScanArgs,  // reading a list of args
+      ArgsEnded, // read the last '>' in an args list, must read delimiter next
+    } State;
+    State = ScanName;
+    int PassBeginIdx = 0;
+    int ArgsBeginIdx;
+    StringRef PassName;
+    StringRef PassArgs;
+    int NestedArgs = 0;
+
+    auto AddPass = [this, CreatePass](StringRef PassName, StringRef PassArgs) {
+      if (PassName.empty()) {
+        errs() << "Found empty pass name.\n";
+        exit(1);
+      }
       // Get the pass that corresponds to PassName and add it to the pass
       // manager.
-      auto Pass = CreatePass(PassName);
+      auto Pass = CreatePass(PassName, PassArgs);
       if (Pass == nullptr) {
         errs() << "Pass '" << PassName << "' not registered!\n";
         exit(1);
       }
       addPass(std::move(Pass));
+    };
+    for (auto [Idx, C] : enumerate(Pipeline)) {
+      switch (State) {
+      case ScanName:
+        if (C == BeginArgsToken) {
+          // Save pass name for later and begin scanning args.
+          PassName = Pipeline.slice(PassBeginIdx, Idx);
+          ArgsBeginIdx = Idx + 1;
+          ++NestedArgs;
+          State = ScanArgs;
+          break;
+        }
+        if (C == EndArgsToken) {
+          errs() << "Unexpected '>' in pass pipeline.\n";
+          exit(1);
+        }
+        if (C == EndToken || C == PassDelimToken) {
+          // Delimiter found, add the pass (with empty args), stay in the
+          // ScanName state.
+          AddPass(Pipeline.slice(PassBeginIdx, Idx), StringRef());
+          PassBeginIdx = Idx + 1;
+        }
+        break;
+      case ScanArgs:
+        // While scanning args, we only care about making sure nesting of angle
+        // brackets is correct.
+        if (C == BeginArgsToken) {
+          ++NestedArgs;
+          break;
+        }
+        if (C == EndArgsToken) {
+          --NestedArgs;
+          if (NestedArgs == 0) {
+            // Done scanning args.
+            PassArgs = Pipeline.slice(ArgsBeginIdx, Idx);
+            AddPass(PassName, PassArgs);
+            State = ArgsEnded;
+          } else if (NestedArgs < 0) {
+            errs() << "Unbalanced '>' in pass pipeline.\n";
+            exit(1);
+          }
+          break;
+        }
+        if (C == EndToken) {
+          errs() << "Missing '>' in pass pipeline. End-of-string reached while "
+                    "reading arguments for pass '"
+                 << PassName << "'.\n";
+          exit(1);
+        }
+        break;
+      case ArgsEnded:
+        // Once we're done scanning args, only a delimiter is valid. This avoids
+        // accepting strings like "foo<args><more-args>" or "foo<args>bar".
+        if (C == EndToken || C == PassDelimToken) {
+          PassBeginIdx = Idx + 1;
+          State = ScanName;
+        } else {
+          errs() << "Expected delimiter or end-of-string after pass "
+                    "arguments.\n";
+          exit(1);
+        }
+        break;
+      }
     }
   }
 
@@ -101,7 +191,7 @@ class PassManager : public ParentPass {
   }
 #endif
   /// Similar to print() but prints one pass per line. Used for testing.
-  void printPipeline(raw_ostream &OS) const {
+  void printPipeline(raw_ostream &OS) const override {
     OS << this->getName() << "\n";
     for (const auto &PassPtr : Passes)
       PassPtr->printPipeline(OS);
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
index 02abdf0a1ef0df..5cd47efd6b3462 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
@@ -13,15 +13,15 @@
 #define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_BOTTOMUPVEC_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/SandboxIR/Constant.h"
 #include "llvm/SandboxIR/Pass.h"
 #include "llvm/SandboxIR/PassManager.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h"
 
 namespace llvm::sandboxir {
 
-class RegionPassManager;
-
 class BottomUpVec final : public FunctionPass {
   bool Change = false;
   LegalityAnalysis Legality;
@@ -32,8 +32,12 @@ class BottomUpVec final : public FunctionPass {
   RegionPassManager RPM;
 
 public:
-  BottomUpVec();
+  BottomUpVec(StringRef Pipeline);
   bool runOnFunction(Function &F) final;
+  void printPipeline(raw_ostream &OS) const final {
+    OS << getName() << "\n";
+    RPM.printPipeline(OS);
+  }
 };
 
 } // namespace llvm::sandboxir
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h
new file mode 100644
index 00000000000000..9d88bc82803847
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h
@@ -0,0 +1,23 @@
+#ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_PRINTINSTRUCTIONCOUNT_H
+#define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_PRINTINSTRUCTIONCOUNT_H
+
+#include "llvm/SandboxIR/Pass.h"
+#include "llvm/SandboxIR/Region.h"
+
+namespace llvm::sandboxir {
+
+/// A Region pass that prints the instruction count for the region to stdout.
+/// Used to test -sbvec-passes while we don't have any actual optimization
+/// passes.
+class PrintInstructionCount final : public RegionPass {
+public:
+  PrintInstructionCount() : RegionPass("null") {}
+  bool runOnRegion(Region &R) final {
+    outs() << "InstructionCount: " << std::distance(R.begin(), R.end()) << "\n";
+    return false;
+  }
+};
+
+} // namespace llvm::sandboxir
+
+#endif // LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_PRINTINSTRUCTIONCOUNTPASS_H
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h
new file mode 100644
index 00000000000000..09347558f940fa
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h
@@ -0,0 +1,34 @@
+//===- RegionsFromMetadata.h ------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// A SandboxIR function pass that builds regions from IR metadata and then runs
+// a pipeline of region passes on them. This is useful to test region passes in
+// isolation without relying on the output of the bottom-up vectorizer.
+//
+
+#ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_REGIONSFROMMETADATA_H
+#define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_REGIONSFROMMETADATA_H
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/SandboxIR/Pass.h"
+#include "llvm/SandboxIR/PassManager.h"
+
+namespace llvm::sandboxir {
+
+class RegionsFromMetadata final : public FunctionPass {
+  // The PM containing the pipeline of region passes.
+  RegionPassManager RPM;
+
+public:
+  RegionsFromMetadata(StringRef Pipeline);
+  bool runOnFunction(Function &F) final;
+};
+
+} // namespace llvm::sandboxir
+
+#endif // LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_REGIONSFROMMETADATA_H
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h
index b7cb418c00326a..b83744cf9e6cb6 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h
@@ -11,7 +11,7 @@
 #include <memory>
 
 #include "llvm/IR/PassManager.h"
-#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h"
+#include "llvm/SandboxIR/PassManager.h"
 
 namespace llvm {
 
@@ -20,8 +20,8 @@ class TargetTransformInfo;
 class SandboxVectorizerPass : public PassInfoMixin<SandboxVectorizerPass> {
   TargetTransformInfo *TTI = nullptr;
 
-  // The main vectorizer pass.
-  sandboxir::BottomUpVec BottomUpVecPass;
+  // A pipeline of SandboxIR function passes run by the vectorizer.
+  sandboxir::FunctionPassManager FPM;
 
   bool runImpl(Function &F);
 
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h
new file mode 100644
index 00000000000000..e3d6ecae836fce
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h
@@ -0,0 +1,32 @@
+//===- SandboxVectorizerPassBuilder.h ---------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Utility functions so passes with sub-pipelines can create SandboxVectorizer
+// passes without replicating the same logic in each pass.
+//
+#ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_SANDBOXVECTORIZERPASSBUILDER_H
+#define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_SANDBOXVECTORIZERPASSBUILDER_H
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/SandboxIR/Pass.h"
+
+#include <memory>
+
+namespace llvm::sandboxir {
+
+class SandboxVectorizerPassBuilder {
+public:
+  static std::unique_ptr<FunctionPass> createFunctionPass(StringRef Name,
+                                                          StringRef Args);
+  static std::unique_ptr<RegionPass> createRegionPass(StringRef Name,
+                                                      StringRef Args);
+};
+
+} // namespace llvm::sandboxir
+
+#endif // LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_SANDBOXVECTORIZERPASSBUILDER_H
diff --git a/llvm/lib/Transforms/Vectorize/CMakeLists.txt b/llvm/lib/Transforms/Vectorize/CMakeLists.txt
index 9c2e7c1e0c5bc7..f4e98e576379a4 100644
--- a/llvm/lib/Transforms/Vectorize/CMakeLists.txt
+++ b/llvm/lib/Transforms/Vectorize/CMakeLists.txt
@@ -6,7 +6,9 @@ add_llvm_component_library(LLVMVectorize
   SandboxVectorizer/DependencyGraph.cpp
   SandboxVectorizer/Interval.cpp
   SandboxVectorizer/Passes/BottomUpVec.cpp
+  SandboxVectorizer/Passes/RegionsFromMetadata.cpp
   SandboxVectorizer/SandboxVectorizer.cpp
+  SandboxVectorizer/SandboxVectorizerPassBuilder.cpp
   SandboxVectorizer/SeedCollector.cpp
   SLPVectorizer.cpp
   Vectorize.cpp
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
index 77198f932a3ec0..1a4b70c4267c7f 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
@@ -7,42 +7,17 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h"
+
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/SandboxIR/Function.h"
 #include "llvm/SandboxIR/Instruction.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h"
 
 namespace llvm::sandboxir {
 
-static cl::opt<bool>
-    PrintPassPipeline("sbvec-print-pass-pipeline", cl::init(false), cl::Hidden,
-                      cl::desc("Prints the pass pipeline and returns."));
-
-/// A magic string for the default pass pipeline.
-static const char *DefaultPipelineMagicStr = "*";
-
-static cl::opt<std::string> UserDefinedPassPipeline(
-    "sbvec-passes", cl::init(DefaultPipelineMagicStr), cl::Hidden,
-    cl::desc("Comma-separated list of vectorizer passes. If not set "
-             "we run the predefined pipeline."));
-
-static std::unique_ptr<RegionPass> createRegionPass(StringRef Name) {
-#define REGION_PASS(NAME, CREATE_PASS)                                         \
-  if (Name == NAME)                                                            \
-    return std::make_unique<decltype(CREATE_PASS)>(CREATE_PASS);
-#include "PassRegistry.def"
-  return nullptr;
-}
-
-BottomUpVec::BottomUpVec() : FunctionPass("bottom-up-vec"), RPM("rpm") {
-  // Create a pipeline to be run on each Region created by BottomUpVec.
-  if (UserDefinedPassPipeline == DefaultPipelineMagicStr) {
-    // TODO: Add default passes to RPM.
-  } else {
-    // Create the user-defined pipeline.
-    RPM.setPassPipeline(UserDefinedPassPipeline, createRegionPass);
-  }
+BottomUpVec::BottomUpVec(StringRef Pipeline)
+    : FunctionPass("bottom-up-vec"), RPM("rpm") {
+  RPM.setPassPipeline(Pipeline, SandboxVectorizerPassBuilder::createRegionPass);
 }
 
 // TODO: This is a temporary function that returns some seeds.
@@ -82,11 +57,6 @@ void BottomUpVec::vectorizeRec(ArrayRef<Value *> Bndl) {
 void BottomUpVec::tryVectorize(ArrayRef<Value *> Bndl) { vectorizeRec(Bndl); }
 
 bool BottomUpVec::runOnFunction(Function &F) {
-  if (PrintPassPipeline) {
-    RPM.printPipeline(outs());
-    return false;
-  }
-
   Change = false;
   // TODO: Start from innermost BBs first
   for (auto &BB : F) {
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def
index bbb0dcba1ea516..0dc72842f1abe0 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def
@@ -14,9 +14,19 @@
 // NOTE: NO INCLUDE GUARD DESIRED!
 
 #ifndef REGION_PASS
-#define REGION_PASS(NAME, CREATE_PASS)
+#define REGION_PASS(NAME, CLASS_NAME)
 #endif
 
-REGION_PASS("null", NullPass())
+REGION_PASS("null", ::llvm::sandboxir::NullPass)
+REGION_PASS("print-instruction-count", ::llvm::sandboxir::PrintInstructionCount)
 
 #undef REGION_PASS
+
+#ifndef FUNCTION_PASS_WITH_PARAMS
+#define FUNCTION_PASS_WITH_PARAMS(NAME, CLASS_NAME)
+#endif
+
+FUNCTION_PASS_WITH_PARAMS("bottom-up-vec", ::llvm::sandboxir::BottomUpVec)
+FUNCTION_PASS_WITH_PARAMS("regions-from-metadata", ::llvm::sandboxir::RegionsFromMetadata)
+
+#undef FUNCTION_PASS_WITH_PARAMS
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
new file mode 100644
index 00000000000000..4cebbdcd3192a6
--- /dev/null
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
@@ -0,0 +1,30 @@
+//===- RegionsFromMetadata.cpp - A helper to test RegionPasses -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h"
+
+#include "llvm/SandboxIR/Region.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h"
+
+namespace llvm::sandboxir {
+
+RegionsFromMetadata::RegionsFromMetadata(StringRef Pipeline)
+    : FunctionPass("regions-from-metadata"), RPM("rpm") {
+  RPM.setPassPipeline(Pipeline, SandboxVectorizerPassBuilder::createRegionPass);
+}
+
+bool RegionsFromMetadata::runOnFunction(Function &F) {
+  SmallVector<std::unique_ptr<sandboxir::Region>> Regions =
+      sandboxir::Region::createRegionsFromMD(F);
+  for (auto &R : Regions) {
+    RPM.runOnRegion(*R);
+  }
+  return false;
+}
+
+} // namespace llvm::sandboxir
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
index ba4899cc624e99..c68f9482e337dd 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
@@ -9,14 +9,39 @@
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/SandboxIR/Constant.h"
-#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h"
 
 using namespace llvm;
 
 #define SV_NAME "sandbox-vectorizer"
 #define DEBUG_TYPE SV_NAME
 
-SandboxVectorizerPass::SandboxVectorizerPass() = default;
+static cl::opt<bool>
+    PrintPassPipeline("sbvec-print-pass-pipeline", cl::init(false), cl::Hidden,
+                      cl::desc("Prints the pass pipeline and returns."));
+
+/// A magic string for the default pass pipeline.
+static const char *DefaultPipelineMagicStr = "*";
+
+static cl::opt<std::string> UserDefinedPassPipeline(
+    "sbvec-passes", cl::init(DefaultPipelineMagicStr), cl::Hidden,
+    cl::desc("Comma-separated list of vectorizer passes. If not set "
+             "we run the predefined pipeline."));
+
+SandboxVectorizerPass::SandboxVectorizerPass() : FPM("fpm") {
+  if (UserDefinedPassPipeline == DefaultPipelineMagicStr) {
+    // TODO: Add region passes to the default pipeline.
+    FPM.setPassPipeline(
+        "bottom-up-vec<>",
+        sandboxir::SandboxVectorizerPassBuilder::createFunctionPass);
+  } else {
+    // Create the user-defined pipeline.
+    FPM.setPassPipeline(
+        UserDefinedPassPipeline,
+        sandboxir::SandboxVectorizerPassBuilder::createFunctionPass);
+  }
+}
 
 SandboxVectorizerPass::SandboxVectorizerPass(SandboxVectorizerPass &&) =
     default;
@@ -37,6 +62,11 @@ PreservedAnalyses SandboxVectorizerPass::run(Function &F,
 }
 
 bool SandboxVectorizerPass::runImpl(Function &LLVMF) {
+  if (PrintPassPipeline) {
+    FPM.printPipeline(outs());
+    return false;
+  }
+
   // If the target claims to have no vector registers early return.
   if (!TTI->getNumberOfRegisters(TTI->getRegisterClassForType(true))) {
     LLVM_DEBUG(dbgs() << "SBVec: Target has no vector registers, return.\n");
@@ -52,5 +82,5 @@ bool SandboxVectorizerPass::runImpl(Function &LLVMF) {
   // Create SandboxIR for LLVMF and run BottomUpVec on it.
   sandboxir::Context Ctx(LLVMF.getContext());
   sandboxir::Function &F = *Ctx.createFunction(&LLVMF);
-  return BottomUpVecPass.runOnFunction(F);
+  return FPM.runOnFunction(F);
 }
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.cpp
new file mode 100644
index 00000000000000..5ecf7b2ed0d258
--- /dev/null
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.cpp
@@ -0,0 +1,32 @@
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h"
+
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h"
+
+namespace llvm::sandboxir {
+
+std::unique_ptr<sandboxir::RegionPass>
+SandboxVectorizerPassBuilder::createRegionPass(StringRef Name, StringRef Args) {
+#define REGION_PASS(NAME, CLASS_NAME)                                          \
+  if (Name == NAME) {                                                          \
+    assert(Args.empty() && "Unexpected arguments for pass '" NAME "'.");       \
+    return std::make_unique<CLASS_NAME>();                                     \
+  }
+// TODO: Support region passes with params.
+#include "Passes/PassRegistry.def"
+  return nullptr;
+}
+
+std::unique_ptr<sandboxir::FunctionPass>
+SandboxVectorizerPassBuilder::createFunctionPass(StringRef Name,
+                                                 StringRef Args) {
+#define FUNCTION_PASS_WITH_PARAMS(NAME, CLASS_NAME)                            \
+  if (Name == NAME)                                                            \
+    return std::make_unique<CLASS_NAME>(Args);
+#include "Passes/PassRegistry.def"
+  return nullptr;
+}
+
+} // namespace llvm::sandboxir
diff --git a/llvm/test/Transforms/SandboxVectorizer/default_pass_pipeline.ll b/llvm/test/Transforms/SandboxVectorizer/default_pass_pipeline.ll
index 86bfbee6364788..1d7be43336c879 100644
--- a/llvm/test/Transforms/SandboxVectorizer/default_pass_pipeline.ll
+++ b/llvm/test/Transforms/SandboxVectorizer/default_pass_pipeline.ll
@@ -4,6 +4,7 @@
 
 ; This checks the default pass pipeline for the sandbox vectorizer.
 define void @pipeline() {
+; CHECK: bottom-up-vec
 ; CHECK: rpm
 ; CHECK-EMPTY:
   ret void
diff --git a/llvm/test/Transforms/SandboxVectorizer/regions-from-metadata.ll b/llvm/test/Transforms/SandboxVectorizer/regions-from-metadata.ll
new file mode 100644
index 00000000000000..3e57bde76e72da
--- /dev/null
+++ b/llvm/test/Transforms/SandboxVectorizer/regions-from-metadata.ll
@@ -0,0 +1,15 @@
+; RUN: opt -disable-output --passes=sandbox-vectorizer \
+; RUN:    -sbvec-passes='regions-from-metadata<print-instruction-count>' %s | FileCheck %s
+
+define i8 @foo(i8 %v0, i8 %v1) {
+  %t0 = add i8 %v0, 1, !sandboxvec !0
+  %t1 = add i8 %t0, %v1, !sandboxvec !1
+  %t2 = add i8 %t1, %v1, !sandboxvec !1
+  ret i8 %t2
+}
+
+!0 = distinct !{!"sandboxregion"}
+!1 = distinct !{!"sandboxregion"}
+
+; CHECK: InstructionCount: 1
+; CHECK: InstructionCount: 2
diff --git a/llvm/test/Transforms/SandboxVectorizer/user_pass_pipeline.ll b/llvm/test/Transforms/SandboxVectorizer/user_pass_pipeline.ll
index 2e6dab0aa29c74..09433ea3032e00 100644
--- a/llvm/test/Transforms/SandboxVectorizer/user_pass_pipeline.ll
+++ b/llvm/test/Transforms/SandboxVectorizer/user_pass_pipeline.ll
@@ -1,9 +1,11 @@
-; RUN: opt -passes=sandbox-vectorizer -sbvec-print-pass-pipeline -sbvec-passes=null,null %s -disable-output | FileCheck %s
+; RUN: opt -passes=sandbox-vectorizer -sbvec-print-pass-pipeline -sbvec-passes="bottom-up-vec<null,null>" %s -disable-output | FileCheck %s
 
 ; !!!WARNING!!! This won't get updated by update_test_checks.py !
 
 ; This checks the user defined pass pipeline.
 define void @pipeline() {
+; CHECK: fpm
+; CHECK: bottom-up-vec
 ; CHECK: rpm
 ; CHECK: null
 ; CHECK: null
diff --git a/llvm/unittests/SandboxIR/PassTest.cpp b/llvm/unittests/SandboxIR/PassTest.cpp
index ae7284ecf2deb9..ce9ae28cf29848 100644
--- a/llvm/unittests/SandboxIR/PassTest.cpp
+++ b/llvm/unittests/SandboxIR/PassTest.cpp
@@ -265,39 +265,45 @@ define void @f() {
                           "f");
   class FooPass final : public FunctionPass {
     std::string &Str;
+    std::string Args;
 
   public:
-    FooPass(std::string &Str) : FunctionPass("foo-pass"), Str(Str) {}
+    FooPass(std::string &Str, llvm::StringRef Args)
+        : FunctionPass("foo-pass"), Str(Str), Args(Args.str()) {}
     bool runOnFunction(Function &F) final {
-      Str += "foo";
+      Str += "foo<" + Args + ">";
       return false;
     }
   };
   class BarPass final : public FunctionPass {
     std::string &Str;
+    std::string Args;
 
   public:
-    BarPass(std::string &Str) : FunctionPass("bar-pass"), Str(Str) {}
+    BarPass(std::string &Str, llvm::StringRef Args)
+        : FunctionPass("bar-pass"), Str(Str), Args(Args.str()) {}
     bool runOnFunction(Function &F) final {
-      Str += "bar";
+      Str += "bar<" + Args + ">";
       return false;
     }
   };
 
   std::string Str;
   auto CreatePass =
-      [&Str](llvm::StringRef Name) -> std::unique_ptr<FunctionPass> {
+      [&Str](llvm::StringRef Name,
+             llvm::StringRef Args) -> std::unique_ptr<FunctionPass> {
     if (Name == "foo")
-      return std::make_unique<FooPass>(Str);
+      return std::make_unique<FooPass>(Str, Args);
     if (Name == "bar")
-      return std::make_unique<BarPass>(Str);
+      return std::make_unique<BarPass>(Str, Args);
     return nullptr;
   };
 
   FunctionPassManager FPM("test-fpm");
-  FPM.setPassPipeline("foo,bar,foo", CreatePass);
+  FPM.setPassPipeline("foo<abc>,bar<nested1<nested2<nested3>>>,foo",
+                      CreatePass);
   FPM.runOnFunction(*F);
-  EXPECT_EQ(Str, "foobarfoo");
+  EXPECT_EQ(Str, "foo<abc>bar<nested1<nested2<nested3>>>foo<>");
 
   // A second call to setPassPipeline will trigger an assertion in debug mode.
 #ifndef NDEBUG
@@ -308,8 +314,21 @@ define void @f() {
   // Fresh PM for the death tests so they die from bad pipeline strings, rather
   // than from multiple setPassPipeline calls.
   FunctionPassManager FPM2("test-fpm");
+  // Bad/empty pass names.
   EXPECT_DEATH(FPM2.setPassPipeline("bad-pass-name", CreatePass),
                ".*not registered.*");
-  EXPECT_DEATH(FPM2.setPassPipeline("", CreatePass), ".*not registered.*");
-  EXPECT_DEATH(FPM2.setPassPipeline(",", CreatePass), ".*not registered.*");
+  EXPECT_DEATH(FPM2.setPassPipeline(",", CreatePass), ".*empty pass name.*");
+  EXPECT_DEATH(FPM2.setPassPipeline("<>", CreatePass), ".*empty pass name.*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo,<>", CreatePass),
+               ".*empty pass name.*");
+
+  // Mismatched argument brackets.
+  EXPECT_DEATH(FPM2.setPassPipeline("foo<", CreatePass), ".*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo>", CreatePass), ".*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo<bar<>", CreatePass), ".*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo<bar<>>>", CreatePass), ".*");
+  EXPECT_DEATH(FPM2.setPassPipeline(">foo", CreatePass), ".*");
+  EXPECT_DEATH(FPM2.setPassPipeline("<>foo", CreatePass), ".*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo<args><more-args>", CreatePass), ".*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo<args>bar", CreatePass), ".*");
 }

>From 3f5ff9908061086552e97eb11f783c89c42a4413 Mon Sep 17 00:00:00 2001
From: Jorge Gorbe Moya <jgorbe at google.com>
Date: Tue, 15 Oct 2024 13:14:56 -0700
Subject: [PATCH 2/2] Address review feedback

---
 llvm/include/llvm/SandboxIR/PassManager.h     | 59 +++++++++++--------
 .../SandboxVectorizer/Passes/BottomUpVec.cpp  |  5 +-
 .../Passes/RegionsFromMetadata.cpp            |  5 +-
 llvm/unittests/SandboxIR/PassTest.cpp         | 27 ++++++---
 4 files changed, 57 insertions(+), 39 deletions(-)

diff --git a/llvm/include/llvm/SandboxIR/PassManager.h b/llvm/include/llvm/SandboxIR/PassManager.h
index 705bd356c5155e..e8221996bc8f04 100644
--- a/llvm/include/llvm/SandboxIR/PassManager.h
+++ b/llvm/include/llvm/SandboxIR/PassManager.h
@@ -32,11 +32,20 @@ class Value;
 /// Base class.
 template <typename ParentPass, typename ContainedPass>
 class PassManager : public ParentPass {
+public:
+  // CreatePassFunc(StringRef PassName, StringRef PassArgs).
+  using CreatePassFunc =
+      std::function<std::unique_ptr<ContainedPass>(StringRef, StringRef)>;
+
 protected:
   /// The list of passes that this pass manager will run.
   SmallVector<std::unique_ptr<ContainedPass>> Passes;
 
   PassManager(StringRef Name) : ParentPass(Name) {}
+  PassManager(StringRef Name, StringRef Pipeline, CreatePassFunc CreatePass)
+      : ParentPass(Name) {
+    setPassPipeline(Pipeline, CreatePass);
+  }
   PassManager(const PassManager &) = delete;
   PassManager(PassManager &&) = default;
   virtual ~PassManager() = default;
@@ -49,9 +58,6 @@ class PassManager : public ParentPass {
     Passes.push_back(std::move(Pass));
   }
 
-  using CreatePassFunc =
-      std::function<std::unique_ptr<ContainedPass>(StringRef, StringRef)>;
-
   /// Parses \p Pipeline as a comma-separated sequence of pass names and sets
   /// the pass pipeline, using \p CreatePass to instantiate passes by name.
   ///
@@ -86,18 +92,6 @@ class PassManager : public ParentPass {
     std::string PipelineStr = std::string(Pipeline) + EndToken;
     Pipeline = StringRef(PipelineStr);
 
-    enum {
-      ScanName,  // reading a pass name
-      ScanArgs,  // reading a list of args
-      ArgsEnded, // read the last '>' in an args list, must read delimiter next
-    } State;
-    State = ScanName;
-    int PassBeginIdx = 0;
-    int ArgsBeginIdx;
-    StringRef PassName;
-    StringRef PassArgs;
-    int NestedArgs = 0;
-
     auto AddPass = [this, CreatePass](StringRef PassName, StringRef PassArgs) {
       if (PassName.empty()) {
         errs() << "Found empty pass name.\n";
@@ -112,15 +106,25 @@ class PassManager : public ParentPass {
       }
       addPass(std::move(Pass));
     };
+
+    enum class State {
+      ScanName,  // reading a pass name
+      ScanArgs,  // reading a list of args
+      ArgsEnded, // read the last '>' in an args list, must read delimiter next
+    } CurrentState = State::ScanName;
+    int PassBeginIdx = 0;
+    int ArgsBeginIdx;
+    StringRef PassName;
+    int NestedArgs = 0;
     for (auto [Idx, C] : enumerate(Pipeline)) {
-      switch (State) {
-      case ScanName:
+      switch (CurrentState) {
+      case State::ScanName:
         if (C == BeginArgsToken) {
           // Save pass name for later and begin scanning args.
           PassName = Pipeline.slice(PassBeginIdx, Idx);
           ArgsBeginIdx = Idx + 1;
           ++NestedArgs;
-          State = ScanArgs;
+          CurrentState = State::ScanArgs;
           break;
         }
         if (C == EndArgsToken) {
@@ -134,7 +138,7 @@ class PassManager : public ParentPass {
           PassBeginIdx = Idx + 1;
         }
         break;
-      case ScanArgs:
+      case State::ScanArgs:
         // While scanning args, we only care about making sure nesting of angle
         // brackets is correct.
         if (C == BeginArgsToken) {
@@ -145,11 +149,10 @@ class PassManager : public ParentPass {
           --NestedArgs;
           if (NestedArgs == 0) {
             // Done scanning args.
-            PassArgs = Pipeline.slice(ArgsBeginIdx, Idx);
-            AddPass(PassName, PassArgs);
-            State = ArgsEnded;
+            AddPass(PassName, Pipeline.slice(ArgsBeginIdx, Idx));
+            CurrentState = State::ArgsEnded;
           } else if (NestedArgs < 0) {
-            errs() << "Unbalanced '>' in pass pipeline.\n";
+            errs() << "Unexpected '>' in pass pipeline.\n";
             exit(1);
           }
           break;
@@ -161,12 +164,12 @@ class PassManager : public ParentPass {
           exit(1);
         }
         break;
-      case ArgsEnded:
+      case State::ArgsEnded:
         // Once we're done scanning args, only a delimiter is valid. This avoids
         // accepting strings like "foo<args><more-args>" or "foo<args>bar".
         if (C == EndToken || C == PassDelimToken) {
           PassBeginIdx = Idx + 1;
-          State = ScanName;
+          CurrentState = State::ScanName;
         } else {
           errs() << "Expected delimiter or end-of-string after pass "
                     "arguments.\n";
@@ -202,12 +205,18 @@ class FunctionPassManager final
     : public PassManager<FunctionPass, FunctionPass> {
 public:
   FunctionPassManager(StringRef Name) : PassManager(Name) {}
+  FunctionPassManager(StringRef Name, StringRef Pipeline,
+                      CreatePassFunc CreatePass)
+      : PassManager(Name, Pipeline, CreatePass) {}
   bool runOnFunction(Function &F) final;
 };
 
 class RegionPassManager final : public PassManager<RegionPass, RegionPass> {
 public:
   RegionPassManager(StringRef Name) : PassManager(Name) {}
+  RegionPassManager(StringRef Name, StringRef Pipeline,
+                    CreatePassFunc CreatePass)
+      : PassManager(Name, Pipeline, CreatePass) {}
   bool runOnRegion(Region &R) final;
 };
 
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
index 1a4b70c4267c7f..6171d5e52b5869 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
@@ -16,9 +16,8 @@
 namespace llvm::sandboxir {
 
 BottomUpVec::BottomUpVec(StringRef Pipeline)
-    : FunctionPass("bottom-up-vec"), RPM("rpm") {
-  RPM.setPassPipeline(Pipeline, SandboxVectorizerPassBuilder::createRegionPass);
-}
+    : FunctionPass("bottom-up-vec"),
+      RPM("rpm", Pipeline, SandboxVectorizerPassBuilder::createRegionPass) {}
 
 // TODO: This is a temporary function that returns some seeds.
 //       Replace this with SeedCollector's function when it lands.
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
index 4cebbdcd3192a6..5887d5e8bc2683 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
@@ -14,9 +14,8 @@
 namespace llvm::sandboxir {
 
 RegionsFromMetadata::RegionsFromMetadata(StringRef Pipeline)
-    : FunctionPass("regions-from-metadata"), RPM("rpm") {
-  RPM.setPassPipeline(Pipeline, SandboxVectorizerPassBuilder::createRegionPass);
-}
+    : FunctionPass("regions-from-metadata"),
+      RPM("rpm", Pipeline, SandboxVectorizerPassBuilder::createRegionPass) {}
 
 bool RegionsFromMetadata::runOnFunction(Function &F) {
   SmallVector<std::unique_ptr<sandboxir::Region>> Regions =
diff --git a/llvm/unittests/SandboxIR/PassTest.cpp b/llvm/unittests/SandboxIR/PassTest.cpp
index ce9ae28cf29848..866bd8233d8035 100644
--- a/llvm/unittests/SandboxIR/PassTest.cpp
+++ b/llvm/unittests/SandboxIR/PassTest.cpp
@@ -319,16 +319,27 @@ define void @f() {
                ".*not registered.*");
   EXPECT_DEATH(FPM2.setPassPipeline(",", CreatePass), ".*empty pass name.*");
   EXPECT_DEATH(FPM2.setPassPipeline("<>", CreatePass), ".*empty pass name.*");
+  EXPECT_DEATH(FPM2.setPassPipeline("<>foo", CreatePass),
+               ".*empty pass name.*");
   EXPECT_DEATH(FPM2.setPassPipeline("foo,<>", CreatePass),
                ".*empty pass name.*");
 
   // Mismatched argument brackets.
-  EXPECT_DEATH(FPM2.setPassPipeline("foo<", CreatePass), ".*");
-  EXPECT_DEATH(FPM2.setPassPipeline("foo>", CreatePass), ".*");
-  EXPECT_DEATH(FPM2.setPassPipeline("foo<bar<>", CreatePass), ".*");
-  EXPECT_DEATH(FPM2.setPassPipeline("foo<bar<>>>", CreatePass), ".*");
-  EXPECT_DEATH(FPM2.setPassPipeline(">foo", CreatePass), ".*");
-  EXPECT_DEATH(FPM2.setPassPipeline("<>foo", CreatePass), ".*");
-  EXPECT_DEATH(FPM2.setPassPipeline("foo<args><more-args>", CreatePass), ".*");
-  EXPECT_DEATH(FPM2.setPassPipeline("foo<args>bar", CreatePass), ".*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo<", CreatePass), ".*Missing '>'.*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo<bar", CreatePass), ".*Missing '>'.*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo<bar<>", CreatePass),
+               ".*Missing '>'.*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo>", CreatePass), ".*Unexpected '>'.*");
+  EXPECT_DEATH(FPM2.setPassPipeline(">foo", CreatePass), ".*Unexpected '>'.*");
+  // Extra garbage between args and next delimiter/end-of-string.
+  EXPECT_DEATH(FPM2.setPassPipeline("foo<bar<>>>", CreatePass),
+               ".*Expected delimiter.*");
+  EXPECT_DEATH(FPM2.setPassPipeline("bar<>foo", CreatePass),
+               ".*Expected delimiter.*");
+  EXPECT_DEATH(FPM2.setPassPipeline("bar<>foo,baz", CreatePass),
+               ".*Expected delimiter.*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo<args><more-args>", CreatePass),
+               ".*Expected delimiter.*");
+  EXPECT_DEATH(FPM2.setPassPipeline("foo<args>bar", CreatePass),
+               ".*Expected delimiter.*");
 }



More information about the llvm-commits mailing list