-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[mlir][Pass] Fix crash when applying a pass to an optional interface #168499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[mlir][Pass] Fix crash when applying a pass to an optional interface #168499
Conversation
|
@llvm/pr-subscribers-mlir-core @llvm/pr-subscribers-mlir Author: Matthias Springer (matthias-springer) ChangesInterfaces can be optional: whether an op implements an interface or not can depend on the state of the operation. The current This commit fixes a crash when scheduling an Full diff: https://github.com/llvm/llvm-project/pull/168499.diff 3 Files Affected:
diff --git a/mlir/include/mlir/Pass/Pass.h b/mlir/include/mlir/Pass/Pass.h
index 16893c6db87b1..448a688243491 100644
--- a/mlir/include/mlir/Pass/Pass.h
+++ b/mlir/include/mlir/Pass/Pass.h
@@ -193,6 +193,13 @@ class Pass {
/// This is useful for generic operation passes to add restrictions on the
/// operations they operate on.
virtual bool canScheduleOn(RegisteredOperationName opName) const = 0;
+ virtual bool canScheduleOn(Operation *op) const {
+ std::optional<RegisteredOperationName> registeredInfo =
+ op->getName().getRegisteredInfo();
+ if (!registeredInfo)
+ return false;
+ return canScheduleOn(*registeredInfo);
+ }
/// Schedule an arbitrary pass pipeline on the provided operation.
/// This can be invoke any time in a pass to dynamic schedule more passes.
@@ -436,6 +443,7 @@ class InterfacePass : public OperationPass<> {
/// Indicate if the current pass can be scheduled on the given operation type.
/// For an InterfacePass, this checks if the operation implements the given
/// interface.
+ bool canScheduleOn(Operation *op) const final { return isa<InterfaceT>(op); }
bool canScheduleOn(RegisteredOperationName opName) const final {
return opName.hasInterface<InterfaceT>();
}
diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index 521c7c6be17b6..c904a8d869b6a 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -559,7 +559,7 @@ LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
return op->emitOpError() << "trying to schedule a pass on an operation not "
"marked as 'IsolatedFromAbove'";
}
- if (!pass->canScheduleOn(*op->getName().getRegisteredInfo())) {
+ if (!pass->canScheduleOn(op)) {
return op->emitOpError()
<< "trying to schedule a pass on an unsupported operation";
}
diff --git a/mlir/test/Analysis/test-liveness-invalid.mlir b/mlir/test/Analysis/test-liveness-invalid.mlir
new file mode 100644
index 0000000000000..32781e93c7b6e
--- /dev/null
+++ b/mlir/test/Analysis/test-liveness-invalid.mlir
@@ -0,0 +1,11 @@
+
+// RUN: mlir-opt %s -test-print-liveness -split-input-file -verify-diagnostics
+
+// Unnamed modules do not implement SymbolOpInterface.
+// expected-error @+1 {{trying to schedule a pass on an unsupported operation}}
+module {}
+
+// -----
+
+// Named modules implement SymbolOpInterface.
+module @named_module {}
|
🐧 Linux x64 Test Results
|
25a8fb6 to
719c41a
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/116/builds/21238 Here is the relevant piece of the build log for the reference |
…optional interface" (#168847) Reverts llvm/llvm-project#168499
Interfaces can be optional: whether an op implements an interface or not can depend on the state of the operation.
The current
Pass::canScheduleOn(RegisteredOperationName)is insufficient. This commit adds an additional overload to inspectOperation *.This commit fixes a crash when scheduling an
InterfacePassfor an optional interface on an operation that does not actually implement the interface.