[Mlir-commits] [mlir] 1079fc4 - [mlir][pass] Add `errorHandler` param to `Pass::initializeOptions` (#87289)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Mon Apr 1 16:43:08 PDT 2024
Author: Ivan Butygin
Date: 2024-04-02T02:43:04+03:00
New Revision: 1079fc4f543c42bb09a33d2d79d90edd9c0bac91
URL: https://github.com/llvm/llvm-project/commit/1079fc4f543c42bb09a33d2d79d90edd9c0bac91
DIFF: https://github.com/llvm/llvm-project/commit/1079fc4f543c42bb09a33d2d79d90edd9c0bac91.diff
LOG: [mlir][pass] Add `errorHandler` param to `Pass::initializeOptions` (#87289)
There is no good way to report detailed errors from inside
`Pass::initializeOptions` function as context may not be available at
this point and writing directly to `llvm::errs()` is not composable.
See
https://github.com/llvm/llvm-project/pull/87166#discussion_r1546426763
* Add error handler callback to `Pass::initializeOptions`
* Update `PassOptions::parseFromString` to support custom error stream
instead of using `llvm::errs()` directly.
* Update default `Pass::initializeOptions` implementation to propagate
error string from `parseFromString` to new error handler.
* Update `MapMemRefStorageClassPass` to report error details using new
API.
Added:
Modified:
mlir/include/mlir/Pass/Pass.h
mlir/include/mlir/Pass/PassOptions.h
mlir/lib/Conversion/MemRefToSPIRV/MapMemRefStorageClassPass.cpp
mlir/lib/Pass/Pass.cpp
mlir/lib/Pass/PassRegistry.cpp
mlir/lib/Transforms/InlinerPass.cpp
mlir/test/Dialect/Transform/test-pass-application.mlir
Removed:
################################################################################
diff --git a/mlir/include/mlir/Pass/Pass.h b/mlir/include/mlir/Pass/Pass.h
index 070e0cad38787c..0f50f3064f1780 100644
--- a/mlir/include/mlir/Pass/Pass.h
+++ b/mlir/include/mlir/Pass/Pass.h
@@ -114,7 +114,9 @@ class Pass {
/// Derived classes may override this method to hook into the point at which
/// options are initialized, but should generally always invoke this base
/// class variant.
- virtual LogicalResult initializeOptions(StringRef options);
+ virtual LogicalResult
+ initializeOptions(StringRef options,
+ function_ref<LogicalResult(const Twine &)> errorHandler);
/// Prints out the pass in the textual representation of pipelines. If this is
/// an adaptor pass, print its pass managers.
diff --git a/mlir/include/mlir/Pass/PassOptions.h b/mlir/include/mlir/Pass/PassOptions.h
index 6717a3585d12a5..3a5e3224133e6f 100644
--- a/mlir/include/mlir/Pass/PassOptions.h
+++ b/mlir/include/mlir/Pass/PassOptions.h
@@ -293,7 +293,8 @@ class PassOptions : protected llvm::cl::SubCommand {
/// Parse options out as key=value pairs that can then be handed off to the
/// `llvm::cl` command line passing infrastructure. Everything is space
/// separated.
- LogicalResult parseFromString(StringRef options);
+ LogicalResult parseFromString(StringRef options,
+ raw_ostream &errorStream = llvm::errs());
/// Print the options held by this struct in a form that can be parsed via
/// 'parseFromString'.
diff --git a/mlir/lib/Conversion/MemRefToSPIRV/MapMemRefStorageClassPass.cpp b/mlir/lib/Conversion/MemRefToSPIRV/MapMemRefStorageClassPass.cpp
index 76dab8ee4ac336..4cbc3dfdae223c 100644
--- a/mlir/lib/Conversion/MemRefToSPIRV/MapMemRefStorageClassPass.cpp
+++ b/mlir/lib/Conversion/MemRefToSPIRV/MapMemRefStorageClassPass.cpp
@@ -272,14 +272,16 @@ class MapMemRefStorageClassPass final
const spirv::MemorySpaceToStorageClassMap &memorySpaceMap)
: memorySpaceMap(memorySpaceMap) {}
- LogicalResult initializeOptions(StringRef options) override {
- if (failed(Pass::initializeOptions(options)))
+ LogicalResult initializeOptions(
+ StringRef options,
+ function_ref<LogicalResult(const Twine &)> errorHandler) override {
+ if (failed(Pass::initializeOptions(options, errorHandler)))
return failure();
if (clientAPI == "opencl")
memorySpaceMap = spirv::mapMemorySpaceToOpenCLStorageClass;
else if (clientAPI != "vulkan")
- return failure();
+ return errorHandler(llvm::Twine("Invalid clienAPI: ") + clientAPI);
return success();
}
diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index 3fb05e53866607..57a6c20141d2c1 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -60,8 +60,16 @@ Operation *PassExecutionAction::getOp() const {
void Pass::anchor() {}
/// Attempt to initialize the options of this pass from the given string.
-LogicalResult Pass::initializeOptions(StringRef options) {
- return passOptions.parseFromString(options);
+LogicalResult Pass::initializeOptions(
+ StringRef options,
+ function_ref<LogicalResult(const Twine &)> errorHandler) {
+ std::string errStr;
+ llvm::raw_string_ostream os(errStr);
+ if (failed(passOptions.parseFromString(options, os))) {
+ os.flush();
+ return errorHandler(errStr);
+ }
+ return success();
}
/// Copy the option values from 'other', which is another instance of this
diff --git a/mlir/lib/Pass/PassRegistry.cpp b/mlir/lib/Pass/PassRegistry.cpp
index b0c314369190a4..f8149673a40939 100644
--- a/mlir/lib/Pass/PassRegistry.cpp
+++ b/mlir/lib/Pass/PassRegistry.cpp
@@ -40,7 +40,7 @@ buildDefaultRegistryFn(const PassAllocatorFunction &allocator) {
return [=](OpPassManager &pm, StringRef options,
function_ref<LogicalResult(const Twine &)> errorHandler) {
std::unique_ptr<Pass> pass = allocator();
- LogicalResult result = pass->initializeOptions(options);
+ LogicalResult result = pass->initializeOptions(options, errorHandler);
std::optional<StringRef> pmOpName = pm.getOpName();
std::optional<StringRef> passOpName = pass->getOpName();
@@ -280,7 +280,8 @@ parseNextArg(StringRef options) {
llvm_unreachable("unexpected control flow in pass option parsing");
}
-LogicalResult detail::PassOptions::parseFromString(StringRef options) {
+LogicalResult detail::PassOptions::parseFromString(StringRef options,
+ raw_ostream &errorStream) {
// NOTE: `options` is modified in place to always refer to the unprocessed
// part of the string.
while (!options.empty()) {
@@ -291,7 +292,7 @@ LogicalResult detail::PassOptions::parseFromString(StringRef options) {
auto it = OptionsMap.find(key);
if (it == OptionsMap.end()) {
- llvm::errs() << "<Pass-Options-Parser>: no such option " << key << "\n";
+ errorStream << "<Pass-Options-Parser>: no such option " << key << "\n";
return failure();
}
if (llvm::cl::ProvidePositionalOption(it->second, value, 0))
diff --git a/mlir/lib/Transforms/InlinerPass.cpp b/mlir/lib/Transforms/InlinerPass.cpp
index 9a7d5403a95dc5..43ca5cac8b76f3 100644
--- a/mlir/lib/Transforms/InlinerPass.cpp
+++ b/mlir/lib/Transforms/InlinerPass.cpp
@@ -64,7 +64,9 @@ class InlinerPass : public impl::InlinerBase<InlinerPass> {
/// Derived classes may override this method to hook into the point at which
/// options are initialized, but should generally always invoke this base
/// class variant.
- LogicalResult initializeOptions(StringRef options) override;
+ LogicalResult initializeOptions(
+ StringRef options,
+ function_ref<LogicalResult(const Twine &)> errorHandler) override;
/// Inliner configuration parameters created from the pass options.
InlinerConfig config;
@@ -153,8 +155,10 @@ void InlinerPass::runOnOperation() {
return;
}
-LogicalResult InlinerPass::initializeOptions(StringRef options) {
- if (failed(Pass::initializeOptions(options)))
+LogicalResult InlinerPass::initializeOptions(
+ StringRef options,
+ function_ref<LogicalResult(const Twine &)> errorHandler) {
+ if (failed(Pass::initializeOptions(options, errorHandler)))
return failure();
// Initialize the pipeline builder for operations without the dedicated
diff --git a/mlir/test/Dialect/Transform/test-pass-application.mlir b/mlir/test/Dialect/Transform/test-pass-application.mlir
index 7cb5387b937d45..460ac3947f5c82 100644
--- a/mlir/test/Dialect/Transform/test-pass-application.mlir
+++ b/mlir/test/Dialect/Transform/test-pass-application.mlir
@@ -78,6 +78,7 @@ module attributes {transform.with_named_sequence} {
transform.named_sequence @__transform_main(%arg1: !transform.any_op) {
%1 = transform.structured.match ops{["func.func"]} in %arg1 : (!transform.any_op) -> !transform.any_op
// expected-error @below {{failed to add pass or pass pipeline to pipeline: canonicalize}}
+ // expected-error @below {{<Pass-Options-Parser>: no such option invalid-option}}
transform.apply_registered_pass "canonicalize" to %1 {options = "invalid-option=1"} : (!transform.any_op) -> !transform.any_op
transform.yield
}
More information about the Mlir-commits
mailing list