[mlir:PassOption] Rework ListOption parsing and add support for std::vector/SmallVector options

[rriddle]

Summary

ListOption currently uses llvm:🆑:list under the hood, but the usages
of ListOption are generally a tad different from llvm:🆑:list. This
commit codifies this by making ListOption implicitly comma separated,
and removes the explicit flag set for all of the current list options.
The new parsing for comma separation of ListOption also adds in support
for skipping over delimited sub-ranges (i.e. {}, [], (), "", ''). This
more easily supports nested options that use those as part of the
format, and this constraint (balanced delimiters) is already codified
in the syntax of pass pipelines.

See https://discourse.llvm.org/t/list-of-lists-pass-option/5950 for
related discussion

[gysit]

Very nice, thanks!

[gysit]

mlir/include/mlir/Pass/PassOptions.h

29

nit: maybe passing _them_?

[mehdi_amini]

I see not mention in the description, but isn't there a GitHub issue this is fixing?

mlir/include/mlir/Pass/PassOptions.h

77

I know we're not very code-size focused, but what about extracting the part of the function that aren't template dependent?
Maybe just making the findChar lambda a separate function would be enough?

198–201

May be worth updating the doc with the specificities of how it is parsed

[rriddle]

mlir/include/mlir/Pass/PassOptions.h

29

Thanks!

77

Thanks for pointing this out! Forgot to rework this after adding the template args.

[rriddle]

In D122879#3423006, @mehdi_amini wrote:

I see not mention in the description, but isn't there a GitHub issue this is fixing?

There was a discourse post (which I added a link to), but I don't think we have an actual issue for this.

mlir/docs/PassManagement.md

 [instrumentations](#pass-instrumentation), and other pass manager related
 components are integrated with the dynamic pipeline being executed.
 
 ## Instance Specific Pass Options
 
 MLIR provides a builtin mechanism for passes to specify options that configure
 its behavior. These options are parsed at pass construction time independently
 for each instance of the pass. Options are defined using the `Option<>` and
-`ListOption<>` classes, and follow the
+`ListOption<>` classes, and generally follow the
 [LLVM command line](https://llvm.org/docs/CommandLine.html) flag definition
-rules. See below for a few examples:
+rules. One major distinction from the LLVM command line functionality is that
+all `ListOption`s are comma-separated, and delimited sub-ranges within individual
+elements of the list may contain commas that are not treated as separators for the
+top-level list.
 
 ```c++
 struct MyPass ... {
   /// Make sure that we have a valid default constructor and copy constructor to
   /// ensure that the options are initialized properly.
   MyPass() = default;
   MyPass(const MyPass& pass) {}
 
   /// Any parameters after the description are forwarded to llvm::cl::list and
   /// llvm::cl::opt respectively.
   Option<int> exampleOption{*this, "flag-name", llvm::cl::desc("...")};
-  ListOption<int> exampleListOption{*this, "list-flag-name",
-                                    llvm::cl::desc("...")};
+  ListOption<int> exampleListOption{*this, "list-flag-name", llvm::cl::desc("...")};
 };
 ```
 
 For pass pipelines, the `PassPipelineRegistration` templates take an additional
 template parameter for an optional `Option` struct definition. This struct
 should inherit from `mlir::PassPipelineOptions` and contain the desired pipeline
 options. When using `PassPipelineRegistration`, the constructor now takes a
 function with the signature `void (OpPassManager &pm, const MyPipelineOptions&)`
struct MyPass : PassWrapper<MyPass, OperationPass<ModuleOp>> {
   ...
 
   // Specify any options.
   Option<bool> option{
       *this, "example-option",
       llvm::cl::desc("An example option"), llvm::cl::init(true)};
   ListOption<int64_t> listOption{
       *this, "example-list",
-      llvm::cl::desc("An example list option"), llvm::cl::ZeroOrMore,
-      llvm::cl::MiscFlags::CommaSeparated};
+      llvm::cl::desc("An example list option"), llvm::cl::ZeroOrMore};
 
   // Specify any statistics.
   Statistic statistic{this, "example-statistic", "An example statistic"};
 };
 
 /// Expose this pass to the outside world.
 std::unique_ptr<Pass> foo::createMyPass() {
   return std::make_unique<MyPass>();
def MyPass : Pass<"my-pass", "ModuleOp"> {
   // of MyPass.
   let constructor = "foo::createMyPass()";
 
   // Specify any options.
   let options = [
     Option<"option", "example-option", "bool", /*default=*/"true",
            "An example option">,
     ListOption<"listOption", "example-list", "int64_t",
-               "An example list option",
-               "llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated">
+               "An example list option", "llvm::cl::ZeroOrMore">
   ];
 
   // Specify any statistics.
   let statistics = [
     Statistic<"statistic", "example-statistic", "An example statistic">
   ];
 }
 ```

 *   additional option flags
     -   A string containing any additional options necessary to construct the
         option.
 
 ```tablegen
 def MyPass : Pass<"my-pass"> {
   let options = [
     ListOption<"listOption", "example-list", "int64_t",
-               "An example list option",
-               "llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated">
+               "An example list option", "llvm::cl::ZeroOrMore">
   ];
 }
 ```
 
 #### Statistic
 
 Statistics may be specified via the `Statistic`, which takes the following
 template parameters:

mlir/docs/PatternRewriter.md

 Two common pattern filtering options are exposed, `disable-patterns` and
 `enable-patterns`, matching the behavior of the `disabledPatterns` and
 `enabledPatterns` lists described in the [Pattern Filtering](#pattern-filtering)
 section above. A snippet of the tablegen definition of these options is shown
 below:
 
 ```tablegen
 ListOption<"disabledPatterns", "disable-patterns", "std::string",
-           "Labels of patterns that should be filtered out during application",
-           "llvm::cl::MiscFlags::CommaSeparated">,
+           "Labels of patterns that should be filtered out during application">,
 ListOption<"enabledPatterns", "enable-patterns", "std::string",
            "Labels of patterns that should be used during application, all "
-           "other patterns are filtered out",
-           "llvm::cl::MiscFlags::CommaSeparated">,
+           "other patterns are filtered out">,
 ```
 
 These options may be used to provide filtering behavior when constructing any
 `FrozenRewritePatternSet`s within the pass:
 
 ```c++
 void MyRewritePass::initialize(MLIRContext *context) {
   RewritePatternSet rewritePatterns(context);
   // ...
 
   // When constructing the `FrozenRewritePatternSet`, we provide the filter
   // list options.
   frozenPatterns = FrozenRewritePatternSet(rewritePatterns, disabledPatterns,
                                            enabledPatterns);
 }
 ```

mlir/include/mlir/Dialect/Affine/Passes.td

 
 def AffineVectorize : Pass<"affine-super-vectorize", "FuncOp"> {
   let summary = "Vectorize to a target independent n-D vector abstraction";
   let constructor = "mlir::createSuperVectorizePass()";
   let dependentDialects = ["vector::VectorDialect"];
   let options = [
     ListOption<"vectorSizes", "virtual-vector-size", "int64_t",
                "Specify an n-D virtual vector size for vectorization",
-               "llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated">,
+               "llvm::cl::ZeroOrMore">,
     // Optionally, the fixed mapping from loop to fastest varying MemRef
     // dimension for all the MemRefs within a loop pattern:
     //   the index represents the loop depth, the value represents the k^th
     //   fastest varying memory dimension.
     // This is voluntarily restrictive and is meant to precisely target a
     // particular loop/op pair, for testing purposes.
     ListOption<"fastestVaryingPattern", "test-fastest-varying", "int64_t",
                "Specify a 1-D, 2-D or 3-D pattern of fastest varying memory "
                "dimensions to match. See defaultPatterns in Vectorize.cpp for "
                "a description and examples. This is used for testing purposes",
-               "llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated">,
+               "llvm::cl::ZeroOrMore">,
     Option<"vectorizeReductions", "vectorize-reductions", "bool",
            /*default=*/"false",
            "Vectorize known reductions expressed via iter_args. "
            "Switched off by default.">
   ];
 }
 
 def AffineParallelize : Pass<"affine-parallelize", "FuncOp"> {

mlir/include/mlir/Dialect/Bufferization/Transforms/Passes.td

Option<"allowUnknownOps", "allow-unknown-ops", "bool",
            "Allows unknown (not bufferizable) ops in the input IR.">,
     Option<"analysisFuzzerSeed", "analysis-fuzzer-seed", "unsigned",
            /*default=*/"0",
            "Test only: Analyze ops in random order with a given seed (fuzzer)">,
     Option<"createDeallocs", "create-deallocs", "bool", /*default=*/"true",
            "Specify if buffers should be deallocated. For compatibility with "
            "core bufferization passes.">,
     ListOption<"dialectFilter", "dialect-filter", "std::string",
-               "Restrict bufferization to ops from these dialects.",
-               "llvm::cl::MiscFlags::CommaSeparated">,
+               "Restrict bufferization to ops from these dialects.">,
     Option<"fullyDynamicLayoutMaps", "fully-dynamic-layout-maps", "bool",
            /*default=*/"true",
            "Generate MemRef types with dynamic offset+strides by default.">,
     Option<"testAnalysisOnly", "test-analysis-only", "bool",
             /*default=*/"false",
            "Test only: Only run inplaceability analysis and annotate IR">,
     Option<"printConflicts", "print-conflicts", "bool",
             /*default=*/"false",

mlir/include/mlir/Dialect/Linalg/Passes.td

def LinalgTiling : Pass<"linalg-tile", "FuncOp"> {
   let dependentDialects = [
     "AffineDialect",
     "linalg::LinalgDialect",
     "memref::MemRefDialect",
     "scf::SCFDialect"
   ];
   let options = [
     ListOption<"tileSizes", "tile-sizes", "int64_t", "Tile sizes",
-               "llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated">,
+               "llvm::cl::ZeroOrMore">,
     Option<"loopType", "loop-type", "std::string", /*default=*/"\"for\"",
            "Specify the type of loops to generate: for, parallel">
   ];
 }
 
 def LinalgGeneralization : Pass<"linalg-generalize-named-ops", "FuncOp"> {
   let summary = "Convert named ops into generic ops";
   let constructor = "mlir::createLinalgGeneralizationPass()";

mlir/include/mlir/Dialect/SCF/Passes.td

def SCFParallelLoopFusion : Pass<"scf-parallel-loop-fusion"> {
   let constructor = "mlir::createParallelLoopFusionPass()";
 }
 
 def SCFParallelLoopCollapsing : Pass<"scf-parallel-loop-collapsing"> {
   let summary = "Collapse parallel loops to use less induction variables";
   let constructor = "mlir::createParallelLoopCollapsingPass()";
   let options = [
     ListOption<"clCollapsedIndices0", "collapsed-indices-0", "unsigned",
-               "Which loop indices to combine 0th loop index",
-               "llvm::cl::MiscFlags::CommaSeparated">,
+               "Which loop indices to combine 0th loop index">,
     ListOption<"clCollapsedIndices1", "collapsed-indices-1", "unsigned",
-               "Which loop indices to combine into the position 1 loop index",
-               "llvm::cl::MiscFlags::CommaSeparated">,
+               "Which loop indices to combine into the position 1 loop index">,
     ListOption<"clCollapsedIndices2", "collapsed-indices-2", "unsigned",
-               "Which loop indices to combine into the position 2 loop index",
-               "llvm::cl::MiscFlags::CommaSeparated">,
+               "Which loop indices to combine into the position 2 loop index">,
   ];
 }
 
 def SCFParallelLoopSpecialization
     : Pass<"scf-parallel-loop-specialization", "FuncOp"> {
   let summary = "Specialize parallel loops for vectorization";
   let constructor = "mlir::createParallelLoopSpecializationPass()";
 }
 
 def SCFParallelLoopTiling : Pass<"scf-parallel-loop-tiling", "FuncOp"> {
   let summary = "Tile parallel loops";
   let constructor = "mlir::createParallelLoopTilingPass()";
   let options = [
     ListOption<"tileSizes", "parallel-loop-tile-sizes", "int64_t",
-               "Factors to tile parallel loops by",
-               "llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated">,
+               "Factors to tile parallel loops by", "llvm::cl::ZeroOrMore">,
     Option<"noMinMaxBounds", "no-min-max-bounds", "bool",
            /*default=*/"false",
            "Perform tiling with fixed upper bound with inbound check "
            "inside the internal loops">
   ];
   let dependentDialects = ["AffineDialect"];
 }
 

mlir/include/mlir/Pass/PassOptions.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_PASS_PASSOPTIONS_H_
 #define MLIR_PASS_PASSOPTIONS_H_
 
 #include "mlir/Support/LLVM.h"
 #include "mlir/Support/LogicalResult.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compiler.h"
 #include <memory>
 
 namespace mlir {
 namespace detail {
+namespace pass_options {
+/// Parse a string containing a list of comma-delimited elements, invoking the
+/// given parser for each sub-element and passing them to the provided
+/// element-append functor.
+LogicalResult
+parseCommaSeparatedList(llvm::cl::Option &opt, StringRef argName,
+                        StringRef optionStr,
+                        function_ref<LogicalResult(StringRef)> elementParseFn);
+template <typename ElementParser, typename ElementAppendFn>
+LogicalResult parseCommaSeparatedList(llvm::cl::Option &opt, StringRef argName,
+                                      StringRef optionStr,
+                                      ElementParser &elementParser,
+                                      ElementAppendFn &&appendFn) {
+  return parseCommaSeparatedList(
+      opt, argName, optionStr, [&](StringRef valueStr) {
+        typename ElementParser::parser_data_type value = {};
+        if (elementParser.parse(opt, argName, valueStr, value))
+          return failure();
+        appendFn(value);
+        return success();
+      });
+}
+
+/// Trait used to detect if a type has a operator<< method.
+template <typename T>
+using has_stream_operator_trait =
+    decltype(std::declval<raw_ostream &>() << std::declval<T>());
+template <typename T>
+using has_stream_operator = llvm::is_detected<has_stream_operator_trait, T>;
+
+/// Utility methods for printing option values.
+template <typename ParserT>
+static void printOptionValue(raw_ostream &os, const bool &value) {
+  os << (value ? StringRef("true") : StringRef("false"));
+}
+template <typename ParserT, typename DataT>
+static std::enable_if_t<has_stream_operator<DataT>::value>
+printOptionValue(raw_ostream &os, const DataT &value) {
+  os << value;
+}
+template <typename ParserT, typename DataT>
+static std::enable_if_t<!has_stream_operator<DataT>::value>
+printOptionValue(raw_ostream &os, const DataT &value) {
+  // If the value can't be streamed, fallback to checking for a print in the
+  // parser.
+  ParserT::print(os, value);
+}
+} // namespace pass_options
+
 /// Base container class and manager for all pass options.
 class PassOptions : protected llvm::cl::SubCommand {
 private:
   /// This is the type-erased option base class. This provides some additional
   /// hooks into the options that are not available via llvm::cl::Option.
   class OptionBase {
   public:
     virtual ~OptionBase() = default;
 
     /// Out of line virtual function to provide home for the class.
static void printValue(raw_ostream &os, GenericOptionParser<DataT> &parser,
                          const DataT &value) {
     if (Optional<StringRef> argStr = parser.findArgStrForValue(value))
       os << argStr;
     else
       llvm_unreachable("unknown data value for option");
   }
   template <typename DataT, typename ParserT>
   static void printValue(raw_ostream &os, ParserT &parser, const DataT &value) {
-    os << value;
-  }
-  template <typename ParserT>
-  static void printValue(raw_ostream &os, ParserT &parser, const bool &value) {
-    os << (value ? StringRef("true") : StringRef("false"));
+    detail::pass_options::printOptionValue<ParserT>(os, value);
   }
 
 public:
   /// The specific parser to use depending on llvm::cl parser used. This is only
   /// necessary because we need to provide additional methods for certain data
   /// type parsers.
   /// TODO: We should upstream the methods in GenericOptionParser to avoid the
   /// need to do this.
private:
     void copyValueFrom(const OptionBase &other) final {
       this->setValue(static_cast<const Option<DataType, OptionParser> &>(other)
                          .getValue());
       optHasValue = other.optHasValue;
     }
   };
 
   /// This class represents a specific pass option that contains a list of
-  /// values of the provided data type.
+  /// values of the provided data type. The elements within the textual form of
+  /// this option are parsed assuming they are comma-separated. Delimited
+  /// sub-ranges within individual elements of the list may contain commas that
+  /// are not treated as separators for the top-level list.
   template <typename DataType, typename OptionParser = OptionParser<DataType>>
   class ListOption
       : public llvm::cl::list<DataType, /*StorageClass=*/bool, OptionParser>,
         public OptionBase {
   public:
     template <typename... Args>
-    ListOption(PassOptions &parent, StringRef arg, Args &&... args)
+    ListOption(PassOptions &parent, StringRef arg, Args &&...args)
         : llvm::cl::list<DataType, /*StorageClass=*/bool, OptionParser>(
-              arg, llvm::cl::sub(parent), std::forward<Args>(args)...) {
+              arg, llvm::cl::sub(parent), std::forward<Args>(args)...),
+          elementParser(*this) {
       assert(!this->isPositional() && !this->isSink() &&
              "sink and positional options are not supported");
+      assert(!(this->getMiscFlags() & llvm::cl::MiscFlags::CommaSeparated) &&
+             "ListOption is implicitly comma separated, specifying "
+             "CommaSeparated is extraneous");
       parent.options.push_back(this);
-
-      // Set a callback to track if this option has a value.
-      this->setCallback([this](const auto &) { this->optHasValue = true; });
+      elementParser.initialize();
     }
     ~ListOption() override = default;
     ListOption<DataType, OptionParser> &
     operator=(const ListOption<DataType, OptionParser> &other) {
       *this = ArrayRef<DataType>(other);
       this->optHasValue = other.optHasValue;
       return *this;
     }
 
+    bool handleOccurrence(unsigned pos, StringRef argName,
+                          StringRef arg) override {
+      this->optHasValue = true;
+      return failed(detail::pass_options::parseCommaSeparatedList(
+          *this, argName, arg, elementParser,
+          [&](const DataType &value) { this->addValue(value); }));
+    }
+
     /// Allow assigning from an ArrayRef.
     ListOption<DataType, OptionParser> &operator=(ArrayRef<DataType> values) {
       ((std::vector<DataType> &)*this).assign(values.begin(), values.end());
       optHasValue = true;
       return *this;
     }
 
     /// Allow accessing the data held by this option.
void print(raw_ostream &os) final {
       };
       llvm::interleave(*this, os, printElementFn, ",");
     }
 
     /// Copy the value from the given option into this one.
     void copyValueFrom(const OptionBase &other) final {
       *this = static_cast<const ListOption<DataType, OptionParser> &>(other);
     }
+
+    /// The parser to use for parsing the list elements.
+    OptionParser elementParser;
   };
 
   PassOptions() = default;
   /// Delete the copy constructor to avoid copying the internal options map.
   PassOptions(const PassOptions &) = delete;
   PassOptions(PassOptions &&) = delete;
 
   /// Copy the option values from 'other' into 'this', where 'other' has the

 
 /// Subclasses of PassPipelineOptions provide a set of options that can be used
 /// to initialize a pass pipeline. See PassPipelineRegistration for usage
 /// details.
 ///
 /// Usage:
 ///
 /// struct MyPipelineOptions : PassPipelineOptions<MyPassOptions> {
-///   ListOption<int> someListFlag{
-///        *this, "flag-name", llvm::cl::MiscFlags::CommaSeparated,
-///        llvm::cl::desc("...")};
+///   ListOption<int> someListFlag{*this, "flag-name", llvm::cl::desc("...")};
 /// };
 template <typename T> class PassPipelineOptions : public detail::PassOptions {
 public:
   /// Factory that parses the provided options and returns a unique_ptr to the
   /// struct.
   static std::unique_ptr<T> createFromString(StringRef options) {
     auto result = std::make_unique<T>();
     if (failed(result->parseFromString(options)))
       return nullptr;
     return result;
   }
 };
 
 /// A default empty option struct to be used for passes that do not need to take
 /// any options.
 struct EmptyPipelineOptions : public PassPipelineOptions<EmptyPipelineOptions> {
 };
 
 } // namespace mlir
 
+//===----------------------------------------------------------------------===//
+// MLIR Options
+//===----------------------------------------------------------------------===//
+
+namespace llvm {
+namespace cl {
+//===----------------------------------------------------------------------===//
+// std::vector+SmallVector
+
+namespace detail {
+template <typename VectorT, typename ElementT>
+class VectorParserBase : public basic_parser_impl {
+public:
+  VectorParserBase(Option &opt) : basic_parser_impl(opt), elementParser(opt) {}
+
+  using parser_data_type = VectorT;
+
+  bool parse(Option &opt, StringRef argName, StringRef arg,
+             parser_data_type &vector) {
+    if (!arg.consume_front("[") || !arg.consume_back("]")) {
+      return opt.error("expected vector option to be wrapped with '[]'",
+                       argName);
+    }
+
+    return failed(mlir::detail::pass_options::parseCommaSeparatedList(
+        opt, argName, arg, elementParser,
+        [&](const ElementT &value) { vector.push_back(value); }));
+  }
+
+  static void print(raw_ostream &os, const VectorT &vector) {
+    llvm::interleave(
+        vector, os,
+        [&](const ElementT &value) {
+          mlir::detail::pass_options::printOptionValue<
+              llvm::cl::parser<ElementT>>(os, value);
+        },
+        ",");
+  }
+
+  void printOptionInfo(const Option &opt, size_t globalWidth) const {
+    // Add the `vector<>` qualifier to the option info.
+    outs() << "  --" << opt.ArgStr;
+    outs() << "=<vector<" << elementParser.getValueName() << ">>";
+    Option::printHelpStr(opt.HelpStr, globalWidth, getOptionWidth(opt));
+  }
+
+  size_t getOptionWidth(const Option &opt) const {
+    // Add the `vector<>` qualifier to the option width.
+    StringRef vectorExt("vector<>");
+    return elementParser.getOptionWidth(opt) + vectorExt.size();
+  }
+
+private:
+  llvm::cl::parser<ElementT> elementParser;
+};
+} // namespace detail
+
+template <typename T>
+class parser<std::vector<T>>
+    : public detail::VectorParserBase<std::vector<T>, T> {
+public:
+  parser(Option &opt) : detail::VectorParserBase<std::vector<T>, T>(opt) {}
+};
+template <typename T, unsigned N>
+class parser<SmallVector<T, N>>
+    : public detail::VectorParserBase<SmallVector<T, N>, T> {
+public:
+  parser(Option &opt) : detail::VectorParserBase<SmallVector<T, N>, T>(opt) {}
+};
+} // end namespace cl
+} // end namespace llvm
+
 #endif // MLIR_PASS_PASSOPTIONS_H_
 

mlir/include/mlir/Reducer/Passes.td

 
 include "mlir/Pass/PassBase.td"
 
 def CommonReductionPassOptions {
   list<Option> options = [
     Option<"testerName", "test", "std::string", /* default */"",
            "The location of the tester which tests the file interestingness">,
     ListOption<"testerArgs", "test-arg", "std::string",
-               "arguments of the tester",
-               "llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated">,
+               "arguments of the tester", "llvm::cl::ZeroOrMore">,
   ];
 }
 
 def ReductionTree : Pass<"reduction-tree"> {
   let summary = "Reduce the input with reduction-tree algorithm";
 
   let constructor = "mlir::createReductionTreePass()";
 

mlir/include/mlir/Rewrite/PassUtil.td

 def RewritePassUtils {
   // A set of options commonly options used for pattern rewrites.
   list<Option> options = [
     // These two options provide filtering for which patterns are applied. These
     // should be passed directly to the FrozenRewritePatternSet when it is
     // created.
     ListOption<"disabledPatterns", "disable-patterns", "std::string",
                "Labels of patterns that should be filtered out during"
-               " application",
-               "llvm::cl::MiscFlags::CommaSeparated">,
+               " application">,
     ListOption<"enabledPatterns", "enable-patterns", "std::string",
                "Labels of patterns that should be used during"
-               " application, all other patterns are filtered out",
-               "llvm::cl::MiscFlags::CommaSeparated">,
+               " application, all other patterns are filtered out">,
   ];
 }
 
 #endif // MLIR_REWRITE_PASSUTIL_TD_

mlir/include/mlir/Transforms/Passes.td

 def Inliner : Pass<"inline"> {
   let summary = "Inline function calls";
   let constructor = "mlir::createInlinerPass()";
   let options = [
     Option<"defaultPipelineStr", "default-pipeline", "std::string",
            /*default=*/"", "The default optimizer pipeline used for callables">,
     ListOption<"opPipelineStrs", "op-pipelines", "std::string",
                "Callable operation specific optimizer pipelines (in the form "
-               "of `dialect.op(pipeline)`)",
-               "llvm::cl::MiscFlags::CommaSeparated">,
+               "of `dialect.op(pipeline)`)">,
     Option<"maxInliningIterations", "max-iterations", "unsigned",
            /*default=*/"4",
            "Maximum number of iterations when inlining within an SCC">,
   ];
 }
 
 def LocationSnapshot : Pass<"snapshot-op-locations"> {
   let summary = "Generate new locations from the current IR";

 def SymbolPrivatize : Pass<"symbol-privatize"> {
   let summary = "Mark symbols private";
   let description = [{
     This pass marks all top-level symbols of the operation run as `private`
     except if listed in `exclude` pass option.
   }];
   let options = [
     ListOption<"exclude", "exclude", "std::string",
-       "Comma separated list of symbols that should not be marked private",
-       "llvm::cl::MiscFlags::CommaSeparated">
+       "Comma separated list of symbols that should not be marked private">
   ];
   let constructor = "mlir::createSymbolPrivatizePass()";
 }
 
 def ViewOpGraph : Pass<"view-op-graph"> {
   let summary = "Print Graphviz visualization of an operation";
   let description = [{
     This pass prints a Graphviz graph of a module.

mlir/lib/Pass/PassRegistry.cpp

const PassInfo *mlir::Pass::lookupPassInfo(StringRef passArg) {
   auto it = passRegistry->find(passArg);
   return it == passRegistry->end() ? nullptr : &it->second;
 }
 
 //===----------------------------------------------------------------------===//
 // PassOptions
 //===----------------------------------------------------------------------===//
 
+LogicalResult detail::pass_options::parseCommaSeparatedList(
+    llvm::cl::Option &opt, StringRef argName, StringRef optionStr,
+    function_ref<LogicalResult(StringRef)> elementParseFn) {
+  // Functor used for finding a character in a string, and skipping over
+  // various "range" characters.
+  llvm::unique_function<size_t(StringRef, size_t, char)> findChar =
+      [&](StringRef str, size_t index, char c) -> size_t {
+    for (size_t i = index, e = str.size(); i < e; ++i) {
+      if (str[i] == c)
+        return i;
+      // Check for various range characters.
+      if (str[i] == '{')
+        i = findChar(str, i + 1, '}');
+      else if (str[i] == '(')
+        i = findChar(str, i + 1, ')');
+      else if (str[i] == '[')
+        i = findChar(str, i + 1, ']');
+      else if (str[i] == '\"')
+        i = str.find_first_of('\"', i + 1);
+      else if (str[i] == '\'')
+        i = str.find_first_of('\'', i + 1);
+    }
+    return StringRef::npos;
+  };
+
+  size_t nextElePos = findChar(optionStr, 0, ',');
+  while (nextElePos != StringRef::npos) {
+    // Process the portion before the comma.
+    if (failed(elementParseFn(optionStr.substr(0, nextElePos))))
+      return failure();
+
+    optionStr = optionStr.substr(nextElePos + 1);
+    nextElePos = findChar(optionStr, 0, ',');
+  }
+  return elementParseFn(optionStr.substr(0, nextElePos));
+}
+
 /// Out of line virtual function to provide home for the class.
 void detail::PassOptions::OptionBase::anchor() {}
 
 /// Copy the option values from 'other'.
 void detail::PassOptions::copyOptionValuesFrom(const PassOptions &other) {
   assert(options.size() == other.options.size());
   if (options.empty())
     return;

mlir/test/Dialect/Linalg/hoist-padding.mlir

 // RUN: mlir-opt %s -test-linalg-codegen-strategy="anchor-op=linalg.matvec pad hoist-paddings=1,1,0 run-enable-pass=false" -cse -canonicalize -split-input-file | FileCheck %s --check-prefix=MATVEC
-// RUN: mlir-opt %s -test-linalg-codegen-strategy="anchor-op=linalg.matvec pad hoist-paddings=1,1,0 transpose-paddings=1:0,0,0 run-enable-pass=false" -cse -canonicalize -split-input-file | FileCheck %s --check-prefix=TRANSP
+// RUN: mlir-opt %s -test-linalg-codegen-strategy="anchor-op=linalg.matvec pad hoist-paddings=1,1,0 transpose-paddings=[1,0],[0],[0] run-enable-pass=false" -cse -canonicalize -split-input-file | FileCheck %s --check-prefix=TRANSP
 // RUN: mlir-opt %s -test-linalg-codegen-strategy="anchor-op=linalg.matmul pad hoist-paddings=1,2,1 run-enable-pass=false" -cse -canonicalize -split-input-file | FileCheck %s --check-prefix=MATMUL
 
 //  MATVEC-DAG: #[[DIV4:[0-9a-z]+]] = affine_map<(d0) -> (d0 ceildiv 4)>
 
 //      MATVEC:  static_size_divisible
 // MATVEC-SAME:    %[[ARG1:[0-9a-zA-Z]*]]: tensor<12xf32>
 func @static_size_divisible(%arg0: tensor<24x12xf32>,
                             %arg1: tensor<12xf32>,

mlir/test/lib/Dialect/Affine/TestLoopPermutation.cpp

struct TestLoopPermutation
 
   void runOnOperation() override;
 
 private:
   /// Permutation specifying loop i is mapped to permList[i] in
   /// transformed nest (with i going from outermost to innermost).
   ListOption<unsigned> permList{*this, "permutation-map",
                                 llvm::cl::desc("Specify the loop permutation"),
-                                llvm::cl::OneOrMore, llvm::cl::CommaSeparated};
+                                llvm::cl::OneOrMore};
 };
 
 } // namespace
 
 void TestLoopPermutation::runOnOperation() {
 
   SmallVector<unsigned, 4> permMap(permList.begin(), permList.end());
 

mlir/test/lib/Dialect/Linalg/TestLinalgCodegenStrategy.cpp

void runStrategy(const LinalgTilingAndFusionOptions &tilingAndFusionOptions,
                    vector::VectorContractLowering vectorContractLowering,
                    vector::VectorTransferSplit vectorTransferSplit);
 
   Option<bool> fuse{
       *this, "fuse",
       llvm::cl::desc("Fuse the producers after tiling the root op."),
       llvm::cl::init(false)};
   ListOption<int64_t> tileSizes{*this, "tile-sizes",
-                                llvm::cl::MiscFlags::CommaSeparated,
                                 llvm::cl::desc("Specifies the tile sizes.")};
   ListOption<int64_t> tileInterchange{
-      *this, "tile-interchange", llvm::cl::MiscFlags::CommaSeparated,
+      *this, "tile-interchange",
       llvm::cl::desc("Specifies the tile interchange.")};
 
   Option<bool> promote{
       *this, "promote",
       llvm::cl::desc("Promote the tile into a small aligned memory buffer."),
       llvm::cl::init(false)};
   Option<bool> promoteFullTile{
       *this, "promote-full-tile-pad",
       llvm::cl::desc("Pad the small aligned memory buffer to the tile sizes."),
       llvm::cl::init(false)};
   ListOption<int64_t> registerTileSizes{
-      *this, "register-tile-sizes", llvm::cl::MiscFlags::CommaSeparated,
+      *this, "register-tile-sizes",
       llvm::cl::desc(
           "Specifies the size of the register tile that will be used "
           " to vectorize")};
   Option<bool> registerPromote{
       *this, "register-promote",
       llvm::cl::desc(
           "Promote the register tile into a small aligned memory buffer."),
       llvm::cl::init(false)};
   Option<bool> registerPromoteFullTile{
       *this, "register-promote-full-tile-pad",
       llvm::cl::desc("Pad the small aligned memory buffer to the tile sizes."),
       llvm::cl::init(false)};
   Option<bool> pad{*this, "pad", llvm::cl::desc("Pad the operands."),
                    llvm::cl::init(false)};
   ListOption<std::string> paddingValues{
       *this, "padding-values",
       llvm::cl::desc("Operand padding values parsed by the attribute parser."),
-      llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated};
+      llvm::cl::ZeroOrMore};
   ListOption<int64_t> paddingDimensions{
       *this, "padding-dimensions",
       llvm::cl::desc("Operation iterator dimensions to pad."),
-      llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated};
-  ListOption<int64_t> packPaddings{
-      *this, "pack-paddings", llvm::cl::desc("Operand packing flags."),
-      llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated};
-  ListOption<int64_t> hoistPaddings{
-      *this, "hoist-paddings", llvm::cl::desc("Operand hoisting depths."),
-      llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated};
-  ListOption<std::string> transposePaddings{
+      llvm::cl::ZeroOrMore};
+  ListOption<int64_t> packPaddings{*this, "pack-paddings",
+                                   llvm::cl::desc("Operand packing flags."),
+                                   llvm::cl::ZeroOrMore};
+  ListOption<int64_t> hoistPaddings{*this, "hoist-paddings",
+                                    llvm::cl::desc("Operand hoisting depths."),
+                                    llvm::cl::ZeroOrMore};
+  ListOption<SmallVector<int64_t>> transposePaddings{
       *this, "transpose-paddings",
       llvm::cl::desc(
           "Transpose paddings. Specify a operand dimension interchange "
           "using the following format:\n"
-          "-transpose-paddings=1:0:2,0:1,0:1\n"
+          "-transpose-paddings=[1,0,2],[0,1],[0,1]\n"
           "It defines the interchange [1, 0, 2] for operand one and "
           "the interchange [0, 1] (no transpose) for the remaining operands."
           "All interchange vectors have to be permuations matching the "
           "operand rank."),
-      llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated};
+      llvm::cl::ZeroOrMore};
   Option<bool> generalize{*this, "generalize",
                           llvm::cl::desc("Generalize named operations."),
                           llvm::cl::init(false)};
   ListOption<int64_t> iteratorInterchange{
-      *this, "iterator-interchange", llvm::cl::MiscFlags::CommaSeparated,
+      *this, "iterator-interchange",
       llvm::cl::desc("Specifies the iterator interchange.")};
   Option<bool> decompose{
       *this, "decompose",
       llvm::cl::desc("Decompose convolutions to lower dimensional ones."),
       llvm::cl::init(false)};
   Option<bool> vectorize{
       *this, "vectorize",
       llvm::cl::desc("Rewrite the linalg op as a vector operation."),
void TestLinalgCodegenStrategy::runOnOperation() {
   // Parse the padding values.
   SmallVector<Attribute> paddingValueAttributes;
   for (const std::string &paddingValue : paddingValues) {
     paddingValueAttributes.push_back(
         parseAttribute(paddingValue, &getContext()));
   }
 
   // Parse the transpose vectors.
-  SmallVector<SmallVector<int64_t>> transposePaddingVectors;
-  for (const std::string &transposePadding : transposePaddings) {
-    SmallVector<int64_t> transposeVector = {};
-    SmallVector<StringRef> tokens;
-    StringRef(transposePadding).split(tokens, ':');
-    for (StringRef token : tokens)
-      transposeVector.push_back(std::stoi(token.str()));
-    transposePaddingVectors.push_back(transposeVector);
-  }
-
   LinalgPaddingOptions paddingOptions;
   paddingOptions.setPaddingValues(paddingValueAttributes);
   paddingOptions.setPaddingDimensions(
       SmallVector<int64_t>{paddingDimensions.begin(), paddingDimensions.end()});
   paddingOptions.setPackPaddings(
       SmallVector<bool>{packPaddings.begin(), packPaddings.end()});
   paddingOptions.setHoistPaddings(
       SmallVector<int64_t>{hoistPaddings.begin(), hoistPaddings.end()});
-  paddingOptions.setTransposePaddings(transposePaddingVectors);
+  paddingOptions.setTransposePaddings(transposePaddings);
 
   vector::VectorContractLowering vectorContractLowering =
       llvm::StringSwitch<vector::VectorContractLowering>(
           vectorizeContractionTo.getValue())
           .Case("matrixintrinsics", vector::VectorContractLowering::Matmul)
           .Case("dot", vector::VectorContractLowering::Dot)
           .Case("outerproduct", vector::VectorContractLowering::OuterProduct)
           .Default(vector::VectorContractLowering::OuterProduct);

mlir/test/lib/Dialect/Linalg/TestLinalgFusionTransforms.cpp

struct TestLinalgTileAndFuseSequencePass
   StringRef getDescription() const final {
     return "Test Linalg tiling and fusion of a sequence of Linalg operations.";
   }
   TestLinalgTileAndFuseSequencePass() = default;
   TestLinalgTileAndFuseSequencePass(
       const TestLinalgTileAndFuseSequencePass &pass)
       : PassWrapper(pass){};
 
-  ListOption<int64_t> tileSizes{
-      *this, "tile-sizes", llvm::cl::desc("Tile sizes to use for ops"),
-      llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated};
+  ListOption<int64_t> tileSizes{*this, "tile-sizes",
+                                llvm::cl::desc("Tile sizes to use for ops"),
+                                llvm::cl::ZeroOrMore};
 
   void getDependentDialects(DialectRegistry &registry) const override {
     registry.insert<AffineDialect, linalg::LinalgDialect, memref::MemRefDialect,
                     scf::SCFDialect>();
   }
 
   void runOnOperation() override {
     FuncOp funcOp = getOperation();

mlir/test/lib/Dialect/Linalg/TestLinalgTransforms.cpp

Option<bool> testSwapSubTensorPadTensor{
       llvm::cl::init(false)};
   Option<bool> testSplitReduction{
       *this, "test-split-reduction",
       llvm::cl::desc("Test split reduction transformation"),
       llvm::cl::init(false)};
   ListOption<int64_t> peeledLoops{
       *this, "peeled-loops",
       llvm::cl::desc("Loops to be peeled when test-tile-pattern"),
-      llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated};
+      llvm::cl::ZeroOrMore};
   ListOption<int64_t> tileSizes{
       *this, "tile-sizes",
       llvm::cl::desc("Linalg tile sizes for test-tile-pattern"),
-      llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated};
+      llvm::cl::ZeroOrMore};
   Option<bool> skipPartial{
       *this, "skip-partial",
       llvm::cl::desc("Skip loops inside partial iterations during peeling"),
       llvm::cl::init(false)};
   Option<std::string> loopType{
       *this, "loop-type",
       llvm::cl::desc("Specify the type of loops to generate: for, parallel or "
                      "tiled_loop"),

mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp

getOperation()->walk([this](scf::ForOp op) {
       // Ignore nested loops.
       if (op->getParentRegion()->getParentOfType<scf::ForOp>())
         return;
       extractFixedOuterLoops(op, sizes);
     });
   }
 
   ListOption<int64_t> sizes{
-      *this, "test-outer-loop-sizes", llvm::cl::MiscFlags::CommaSeparated,
+      *this, "test-outer-loop-sizes",
       llvm::cl::desc(
           "fixed number of iterations that the outer loops should have")};
 };
 } // namespace
 
 namespace mlir {
 namespace test {
 void registerSimpleParametricTilingPass() {
   PassRegistration<SimpleParametricLoopTilingPass>();
 }
 } // namespace test
 } // namespace mlir

mlir/test/lib/Dialect/SPIRV/TestEntryPointAbi.cpp

 
 private:
   Pass::ListOption<int32_t> workgroupSize{
       *this, "workgroup-size",
       llvm::cl::desc(
           "Workgroup size to use for all gpu.func kernels in the module, "
           "specified with x-dimension first, y-dimension next and z-dimension "
           "last. Unspecified dimensions will be set to 1"),
-      llvm::cl::ZeroOrMore, llvm::cl::MiscFlags::CommaSeparated};
+      llvm::cl::ZeroOrMore};
 };
 } // namespace
 
 void TestSpirvEntryPointABIPass::runOnOperation() {
   gpu::GPUModuleOp gpuModule = getOperation();
   MLIRContext *context = &getContext();
   StringRef attrName = spirv::getEntryPointABIAttrName();
   for (gpu::GPUFuncOp gpuFunc : gpuModule.getOps<gpu::GPUFuncOp>()) {

mlir/test/lib/Dialect/Vector/TestVectorTransforms.cpp

struct TestVectorDistributePatterns
   TestVectorDistributePatterns() = default;
   TestVectorDistributePatterns(const TestVectorDistributePatterns &pass)
       : PassWrapper(pass) {}
   void getDependentDialects(DialectRegistry &registry) const override {
     registry.insert<VectorDialect>();
     registry.insert<AffineDialect>();
   }
   ListOption<int32_t> multiplicity{
-      *this, "distribution-multiplicity", llvm::cl::MiscFlags::CommaSeparated,
+      *this, "distribution-multiplicity",
       llvm::cl::desc("Set the multiplicity used for distributing vector")};
 
   void runOnOperation() override {
     MLIRContext *ctx = &getContext();
     RewritePatternSet patterns(ctx);
     FuncOp func = getOperation();
     func.walk([&](arith::AddFOp op) {
       OpBuilder builder(op);

mlir/test/lib/IR/TestDiagnostics.cpp

void runOnOperation() override {
     // Emit a diagnostic for every operation with a valid loc.
     getOperation()->walk([&](Operation *op) {
       if (LocationAttr locAttr = op->getAttrOfType<LocationAttr>("test.loc"))
         emitError(locAttr, "test diagnostic");
     });
   }
 
   ListOption<std::string> filters{
-      *this, "filters", llvm::cl::MiscFlags::CommaSeparated,
+      *this, "filters",
       llvm::cl::desc("Specifies the diagnostic file name filters.")};
 };
 
 } // namespace
 
 namespace mlir {
 namespace test {
 void registerTestDiagnosticsPass() {
   PassRegistration<TestDiagnosticFilterPass>{};
 }
 } // namespace test
 } // namespace mlir

mlir/test/lib/Pass/TestDynamicPipeline.cpp

Option<bool> runOnParent{
       *this, "run-on-parent",
       llvm::cl::desc("This will apply the pipeline on the parent operation if "
                      "it exist, this is expected to fail.")};
   Option<std::string> pipeline{
       *this, "dynamic-pipeline",
       llvm::cl::desc("The pipeline description that "
                      "will run on the filtered function.")};
   ListOption<std::string> opNames{
-      *this, "op-name", llvm::cl::MiscFlags::CommaSeparated,
+      *this, "op-name",
       llvm::cl::desc("List of function name to apply the pipeline to")};
 };
 } // namespace
 
 namespace mlir {
 namespace test {
 void registerTestDynamicPipelinePass() {
   PassRegistration<TestDynamicPipelinePass>();
 }
 } // namespace test
 } // namespace mlir

mlir/test/lib/Pass/TestPassManager.cpp

return "Test an interface pass (running on FunctionOpInterface) in the "
            "pass manager";
   }
 };
 class TestOptionsPass
     : public PassWrapper<TestOptionsPass, OperationPass<FuncOp>> {
 public:
   struct Options : public PassPipelineOptions<Options> {
     ListOption<int> listOption{*this, "list",
-                               llvm::cl::MiscFlags::CommaSeparated,
                                llvm::cl::desc("Example list option")};
     ListOption<std::string> stringListOption{
-        *this, "string-list", llvm::cl::MiscFlags::CommaSeparated,
-        llvm::cl::desc("Example string list option")};
+        *this, "string-list", llvm::cl::desc("Example string list option")};
     Option<std::string> stringOption{*this, "string",
                                      llvm::cl::desc("Example string option")};
   };
   TestOptionsPass() = default;
   TestOptionsPass(const TestOptionsPass &) {}
   TestOptionsPass(const Options &options) {
     listOption = options.listOption;
     stringOption = options.stringOption;
     stringListOption = options.stringListOption;
   }
 
   void runOnOperation() final {}
   StringRef getArgument() const final { return "test-options-pass"; }
   StringRef getDescription() const final {
     return "Test options parsing capabilities";
   }
 
-  ListOption<int> listOption{*this, "list", llvm::cl::MiscFlags::CommaSeparated,
+  ListOption<int> listOption{*this, "list",
                              llvm::cl::desc("Example list option")};
   ListOption<std::string> stringListOption{
-      *this, "string-list", llvm::cl::MiscFlags::CommaSeparated,
-      llvm::cl::desc("Example string list option")};
+      *this, "string-list", llvm::cl::desc("Example string list option")};
   Option<std::string> stringOption{*this, "string",
                                    llvm::cl::desc("Example string option")};
 };
 
 /// A test pass that always aborts to enable testing the crash recovery
 /// mechanism of the pass manager.
 class TestCrashRecoveryPass
     : public PassWrapper<TestCrashRecoveryPass, OperationPass<>> {