diff --git a/mlir/include/mlir/Pass/PassOptions.h b/mlir/include/mlir/Pass/PassOptions.h index a5a3f1c1c19652..b5a9c25e3baca5 100644 --- a/mlir/include/mlir/Pass/PassOptions.h +++ b/mlir/include/mlir/Pass/PassOptions.h @@ -253,6 +253,11 @@ class PassOptions : protected llvm::cl::SubCommand { assert(!(this->getMiscFlags() & llvm::cl::MiscFlags::CommaSeparated) && "ListOption is implicitly comma separated, specifying " "CommaSeparated is extraneous"); + + // Make the default explicitly "empty" if no default was given. + if (!this->isDefaultAssigned()) + this->setInitialValues({}); + parent.options.push_back(this); elementParser.initialize(); } @@ -296,11 +301,21 @@ class PassOptions : protected llvm::cl::SubCommand { const llvm::cl::Option *getOption() const final { return this; } /// Print the name and value of this option to the given stream. + /// Note that there is currently a limitation with regards to + /// `ListOption`: parsing 'option=""` will result in `option` being + /// set to the empty list, not to a size-1 list containing an empty string. void print(raw_ostream &os) final { - // Don't print the list if empty. An empty option value can be treated as - // an element of the list in certain cases (e.g. ListOption). - if ((**this).empty()) - return; + // Don't print the list if the value is the default value. + if (this->isDefaultAssigned() && + this->getDefault().size() == (**this).size()) { + unsigned i = 0; + for (unsigned e = (**this).size(); i < e; i++) { + if (!this->getDefault()[i].compare((**this)[i])) + break; + } + if (i == (**this).size()) + return; + } os << this->ArgStr << "={"; auto printElementFn = [&](const DataType &value) { diff --git a/mlir/lib/Pass/PassRegistry.cpp b/mlir/lib/Pass/PassRegistry.cpp index fe842755958418..ece2fdaed0dfd0 100644 --- a/mlir/lib/Pass/PassRegistry.cpp +++ b/mlir/lib/Pass/PassRegistry.cpp @@ -186,6 +186,30 @@ const PassPipelineInfo *mlir::PassPipelineInfo::lookup(StringRef pipelineArg) { // PassOptions //===----------------------------------------------------------------------===// +/// Attempt to find the next occurance of character 'c' in the string starting +/// from the `index`-th position , omitting any occurances that appear within +/// intervening ranges or literals. +static size_t findChar(StringRef str, size_t index, char c) { + 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); + if (i == StringRef::npos) + return StringRef::npos; + } + return StringRef::npos; +} + /// Extract an argument from 'options' and update it to point after the arg. /// Returns the cleaned argument string. static StringRef extractArgAndUpdateOptions(StringRef &options, @@ -194,47 +218,37 @@ static StringRef extractArgAndUpdateOptions(StringRef &options, options = options.drop_front(argSize).ltrim(); // Early exit if there's no escape sequence. - if (str.size() <= 2) + if (str.size() <= 1) return str; const auto escapePairs = {std::make_pair('\'', '\''), - std::make_pair('"', '"'), std::make_pair('{', '}')}; + std::make_pair('"', '"')}; for (const auto &escape : escapePairs) { if (str.front() == escape.first && str.back() == escape.second) { // Drop the escape characters and trim. - str = str.drop_front().drop_back().trim(); // Don't process additional escape sequences. - break; + return str.drop_front().drop_back().trim(); } } + // Arguments may be wrapped in `{...}`. Unlike the quotation markers that + // denote literals, we respect scoping here. The outer `{...}` should not + // be stripped in cases such as "arg={...},{...}", which can be used to denote + // lists of nested option structs. + if (str.front() == '{') { + unsigned match = findChar(str, 1, '}'); + if (match == str.size() - 1) + str = str.drop_front().drop_back().trim(); + } + return str; } LogicalResult detail::pass_options::parseCommaSeparatedList( llvm::cl::Option &opt, StringRef argName, StringRef optionStr, function_ref elementParseFn) { - // Functor used for finding a character in a string, and skipping over - // various "range" characters. - llvm::unique_function 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; - }; + if (optionStr.empty()) + return success(); size_t nextElePos = findChar(optionStr, 0, ','); while (nextElePos != StringRef::npos) { diff --git a/mlir/test/Pass/pipeline-options-parsing.mlir b/mlir/test/Pass/pipeline-options-parsing.mlir index b6c2b688b7cfb3..b8cd605a83a2bc 100644 --- a/mlir/test/Pass/pipeline-options-parsing.mlir +++ b/mlir/test/Pass/pipeline-options-parsing.mlir @@ -14,6 +14,22 @@ // RUN: mlir-opt %s -verify-each=false '-test-options-super-pass-pipeline=super-list={{enum=zero list=1 string=foo},{enum=one list=2 string="bar"},{enum=two list=3 string={baz}}}' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_7 %s // RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.module(func.func(test-options-super-pass{list={{enum=zero list={1} string=foo },{enum=one list={2} string=bar },{enum=two list={3} string=baz }}}))' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_7 %s + +// This test checks that lists-of-nested-options like 'option1={...},{....}' can be parsed +// just like how 'option=1,2,3' is also allowed: + +// RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.module(func.func(test-options-super-pass{list={enum=zero list={1} string=foo },{enum=one list={2} string=bar },{enum=two list={3} string=baz }}))' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_7 %s + +// This test checks that it is legal to specify an empty list using '{}'. +// RUN: mlir-opt %s -verify-each=false '--test-options-super-pass=list={enum=zero list={1} string=foo},{enum=one list={} string=bar}' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_8 %s + +// It is not possible to specify a size-1 list of empty string. +// It is possible to specify a size > 1 list of empty strings. +// RUN: mlir-opt %s -verify-each=false '--pass-pipeline=builtin.module(func.func(test-options-pass{string-list={""}}))' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_9 %s +// RUN: mlir-opt %s -verify-each=false '--pass-pipeline=builtin.module(func.func(test-options-pass{string-list={,}}))' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_10 %s +// RUN: mlir-opt %s -verify-each=false '--pass-pipeline=builtin.module(func.func(test-options-pass{string-list={"",}}))' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_10 %s + + // CHECK_ERROR_1: missing closing '}' while processing pass options // CHECK_ERROR_2: no such option test-option // CHECK_ERROR_3: no such option invalid-option @@ -27,3 +43,6 @@ // CHECK_5: builtin.module(builtin.module(func.func(test-options-pass{enum=zero list={3} string= }),func.func(test-options-pass{enum=one list={1,2,3,4} string={foo bar baz} }))) // CHECK_6: builtin.module(builtin.module(func.func(test-options-pass{enum=zero list={3} string= }),func.func(test-options-pass{enum=one list={1,2,3,4} string=foo"bar"baz }))) // CHECK_7{LITERAL}: builtin.module(func.func(test-options-super-pass{list={{enum=zero list={1} string=foo },{enum=one list={2} string=bar },{enum=two list={3} string=baz }}})) +// CHECK_8{LITERAL}: builtin.module(func.func(test-options-super-pass{list={{enum=zero list={1} string=foo },{enum=one string=bar }}})) +// CHECK_9: builtin.module(func.func(test-options-pass{enum=zero string= string-list={}})) +// CHECK_10: builtin.module(func.func(test-options-pass{enum=zero string= string-list={,}}))