[Mlir-commits] [mlir] f4a3686 - [mlir:PassOptions] Fix parsing of nested option values/better handle escaping
River Riddle
llvmlistbot at llvm.org
Wed Jan 26 21:38:01 PST 2022
Author: River Riddle
Date: 2022-01-26T21:37:22-08:00
New Revision: f4a368689f34a8ae779e96097e4b18958f1659fa
URL: https://github.com/llvm/llvm-project/commit/f4a368689f34a8ae779e96097e4b18958f1659fa
DIFF: https://github.com/llvm/llvm-project/commit/f4a368689f34a8ae779e96097e4b18958f1659fa.diff
LOG: [mlir:PassOptions] Fix parsing of nested option values/better handle escaping
The option parser currently does not properly handle nested options, meaning that
in some cases we can print pass pipelines that we can't actually parse back in. For example,
from #52885 we currently can't parse in inliner pipelines that have nested pipeline strings.
This commit adds handling for string values (e.g. "...") and nested options
(e.g. `foo{baz{bar=10 pizza=11}}`).
Fixes #52885
Differential Revision: https://reviews.llvm.org/D118078
Added:
Modified:
mlir/lib/Pass/PassRegistry.cpp
mlir/test/Pass/pipeline-options-parsing.mlir
Removed:
################################################################################
diff --git a/mlir/lib/Pass/PassRegistry.cpp b/mlir/lib/Pass/PassRegistry.cpp
index 6f92e2d80b4e3..58870a6ea93b7 100644
--- a/mlir/lib/Pass/PassRegistry.cpp
+++ b/mlir/lib/Pass/PassRegistry.cpp
@@ -154,35 +154,87 @@ void detail::PassOptions::copyOptionValuesFrom(const PassOptions &other) {
std::get<0>(optionsIt)->copyValueFrom(*std::get<1>(optionsIt));
}
+/// Parse in the next argument from the given options string. Returns a tuple
+/// containing [the key of the option, the value of the option, updated
+/// `options` string pointing after the parsed option].
+static std::tuple<StringRef, StringRef, StringRef>
+parseNextArg(StringRef options) {
+ // Functor used to extract an argument from 'options' and update it to point
+ // after the arg.
+ auto extractArgAndUpdateOptions = [&](size_t argSize) {
+ StringRef str = options.take_front(argSize).trim();
+ options = options.drop_front(argSize).ltrim();
+ return str;
+ };
+ // Try to process the given punctuation, properly escaping any contained
+ // characters.
+ auto tryProcessPunct = [&](size_t ¤tPos, char punct) {
+ if (options[currentPos] != punct)
+ return false;
+ size_t nextIt = options.find_first_of(punct, currentPos + 1);
+ if (nextIt != StringRef::npos)
+ currentPos = nextIt;
+ return true;
+ };
+
+ // Parse the argument name of the option.
+ StringRef argName;
+ for (size_t argEndIt = 0, optionsE = options.size();; ++argEndIt) {
+ // Check for the end of the full option.
+ if (argEndIt == optionsE || options[argEndIt] == ' ') {
+ argName = extractArgAndUpdateOptions(argEndIt);
+ return std::make_tuple(argName, StringRef(), options);
+ }
+
+ // Check for the end of the name and the start of the value.
+ if (options[argEndIt] == '=') {
+ argName = extractArgAndUpdateOptions(argEndIt);
+ options = options.drop_front();
+ break;
+ }
+ }
+
+ // Parse the value of the option.
+ for (size_t argEndIt = 0, optionsE = options.size();; ++argEndIt) {
+ // Handle the end of the options string.
+ if (argEndIt == optionsE || options[argEndIt] == ' ') {
+ StringRef value = extractArgAndUpdateOptions(argEndIt);
+ return std::make_tuple(argName, value, options);
+ }
+
+ // Skip over escaped sequences.
+ char c = options[argEndIt];
+ if (tryProcessPunct(argEndIt, '\'') || tryProcessPunct(argEndIt, '"'))
+ continue;
+ // '{...}' is used to specify options to passes, properly escape it so
+ // that we don't accidentally split any nested options.
+ if (c == '{') {
+ size_t braceCount = 1;
+ for (++argEndIt; argEndIt != optionsE; ++argEndIt) {
+ // Allow nested punctuation.
+ if (tryProcessPunct(argEndIt, '\'') || tryProcessPunct(argEndIt, '"'))
+ continue;
+ if (options[argEndIt] == '{')
+ ++braceCount;
+ else if (options[argEndIt] == '}' && --braceCount == 0)
+ break;
+ }
+ // Account for the increment at the top of the loop.
+ --argEndIt;
+ }
+ }
+ llvm_unreachable("unexpected control flow in pass option parsing");
+}
+
LogicalResult detail::PassOptions::parseFromString(StringRef options) {
- // TODO: Handle escaping strings.
// NOTE: `options` is modified in place to always refer to the unprocessed
// part of the string.
while (!options.empty()) {
- size_t spacePos = options.find(' ');
- StringRef arg = options;
- if (spacePos != StringRef::npos) {
- arg = options.substr(0, spacePos);
- options = options.substr(spacePos + 1);
- } else {
- options = StringRef();
- }
- if (arg.empty())
+ StringRef key, value;
+ std::tie(key, value, options) = parseNextArg(options);
+ if (key.empty())
continue;
- // At this point, arg refers to everything that is non-space in options
- // upto the next space, and options refers to the rest of the string after
- // that point.
-
- // Split the individual option on '=' to form key and value. If there is no
- // '=', then value is `StringRef()`.
- size_t equalPos = arg.find('=');
- StringRef key = arg;
- StringRef value;
- if (equalPos != StringRef::npos) {
- key = arg.substr(0, equalPos);
- value = arg.substr(equalPos + 1);
- }
auto it = OptionsMap.find(key);
if (it == OptionsMap.end()) {
llvm::errs() << "<Pass-Options-Parser>: no such option " << key << "\n";
diff --git a/mlir/test/Pass/pipeline-options-parsing.mlir b/mlir/test/Pass/pipeline-options-parsing.mlir
index 7fe3f76326847..ab0e482373ff7 100644
--- a/mlir/test/Pass/pipeline-options-parsing.mlir
+++ b/mlir/test/Pass/pipeline-options-parsing.mlir
@@ -3,7 +3,7 @@
// RUN: not mlir-opt %s -pass-pipeline='builtin.module(builtin.func(test-options-pass{list=3}), test-module-pass{invalid-option=3})' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_3 %s
// RUN: not mlir-opt %s -pass-pipeline='test-options-pass{list=3 list=notaninteger}' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_4 %s
// RUN: not mlir-opt %s -pass-pipeline='builtin.func(test-options-pass{list=1,2,3,4 list=5 string=value1 string=value2})' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_5 %s
-// RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.func(test-options-pass{string-list=a list=1,2,3,4 string-list=b,c list=5 string-list=d string=some_value})' -test-dump-pipeline 2>&1 | FileCheck --check-prefix=CHECK_1 %s
+// RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.func(test-options-pass{string-list=a list=1,2,3,4 string-list=b,c list=5 string-list=d string=nested_pipeline{arg1=10 arg2=" {} " arg3=true}})' -test-dump-pipeline 2>&1 | FileCheck --check-prefix=CHECK_1 %s
// RUN: mlir-opt %s -verify-each=false -test-options-pass-pipeline='list=1 string-list=a,b' -test-dump-pipeline 2>&1 | FileCheck --check-prefix=CHECK_2 %s
// RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.module(builtin.func(test-options-pass{list=3}), builtin.func(test-options-pass{list=1,2,3,4}))' -test-dump-pipeline 2>&1 | FileCheck --check-prefix=CHECK_3 %s
@@ -13,6 +13,6 @@
// CHECK_ERROR_4: 'notaninteger' value invalid for integer argument
// CHECK_ERROR_5: string option: may only occur zero or one times
-// CHECK_1: test-options-pass{list=1,2,3,4,5 string=some_value string-list=a,b,c,d}
+// CHECK_1: test-options-pass{list=1,2,3,4,5 string=nested_pipeline{arg1=10 arg2=" {} " arg3=true} string-list=a,b,c,d}
// CHECK_2: test-options-pass{list=1 string= string-list=a,b}
// CHECK_3: builtin.module(builtin.func(test-options-pass{list=3 string= }), builtin.func(test-options-pass{list=1,2,3,4 string= }))
More information about the Mlir-commits
mailing list