Make FunctionOpInterface check ReturnLike
#112615
Conversation
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-bufferization Author: Tzung-Han Juang (tzunghanjuang) ChangesDescription: This PR is a follow-up for #110322. The previous PR makes This PR tries to tie Full diff: https://github.com/llvm/llvm-project/pull/112615.diff 3 Files Affected:
diff --git a/mlir/include/mlir/Interfaces/FunctionInterfaces.h b/mlir/include/mlir/Interfaces/FunctionInterfaces.h
index e10e9bd342702a..f121a6823711a0 100644
--- a/mlir/include/mlir/Interfaces/FunctionInterfaces.h
+++ b/mlir/include/mlir/Interfaces/FunctionInterfaces.h
@@ -20,6 +20,7 @@
#include "mlir/IR/SymbolTable.h"
#include "mlir/IR/TypeUtilities.h"
#include "mlir/Interfaces/CallInterfaces.h"
+#include "mlir/Interfaces/ControlFlowInterfaces.h"
#include "llvm/ADT/BitVector.h"
#include "llvm/ADT/SmallString.h"
diff --git a/mlir/include/mlir/Interfaces/FunctionInterfaces.td b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
index 697f951748c675..244c7b33f0119b 100644
--- a/mlir/include/mlir/Interfaces/FunctionInterfaces.td
+++ b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
@@ -108,6 +108,29 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
}
}
+ // FunctionOpInterface is tied to a ReturnLike.
+ Operation *terminator = entryBlock.getTerminator();
+ if (terminator->hasTrait<OpTrait::ReturnLike>()) {
+ return $_op.emitOpError("The body of a FunctionOpInterface must")
+ << "have a ReturnLike terminator.";
+ }
+
+ // Match ReturnLike's operand types and FunctionOpInterface's
+ // result types.
+ auto returnOperandTypes = terminator->getOperandTypes();
+ auto funcResultTypes = $_op->getResultTypes();
+ if (funcResultTypes.size() != returnOperandTypes.size()) {
+ return $_op.emitOpError("The number of a FunctionOpInterface's")
+ << "result must match that of the ReturnLike operands.";
+ }
+
+ for (unsigned i = 0; i < funcResultTypes.size(); ++i) {
+ if (funcResultTypes[i] != returnOperandTypes[i]) {
+ return $_op.emitOpError("The result types of a FunctionOpInterface")
+ << "must match the operand types of the ReturnLike.";
+ }
+ }
+
return success();
}]>,
InterfaceMethod<[{
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/OneShotModuleBufferize.cpp b/mlir/lib/Dialect/Bufferization/Transforms/OneShotModuleBufferize.cpp
index a0e5c7fff7690f..bb1cf2fc3fd209 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/OneShotModuleBufferize.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/OneShotModuleBufferize.cpp
@@ -302,8 +302,8 @@ getFuncOpsOrderedByCalls(ModuleOp moduleOp,
Operation *returnOp = getAssumedUniqueReturnOp(funcOp);
if (!returnOp)
return funcOp->emitError()
- << "cannot bufferize a FuncOp with tensors and "
- "without a unique ReturnOp";
+ << "cannot bufferize a FunctionOpInterface with tensors and "
+ "without a unique ReturnLike";
}
// Collect function calls and populate the caller map.
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
There was a problem hiding this comment.
I don't see a sufficiently strong rationale to change the requirements of the built-in interface or trait. Return-like is used in dataflow sense outside functions. There are function-like operations that don't use return-like terminators (consider co-routines). If bufferization needs some additional guarantees/information from terminators to work, it is better to define a new, bufferization-specific interface and mark complying operations as implementing this interface.
| // FunctionOpInterface is tied to a ReturnLike. | ||
| Operation *terminator = entryBlock.getTerminator(); | ||
| if (!terminator->hasTrait<OpTrait::ReturnLike>()) { | ||
| return $_op.emitOpError("The body of a FunctionOpInterface must have ") | ||
| << "a ReturnLike terminator, but the current terminator does not " | ||
| << "have this trait."; | ||
| } | ||
|
|
||
| // Match ReturnLike's operand types and FunctionOpInterface's | ||
| // result types. | ||
| auto returnOperandTypes = terminator->getOperandTypes(); | ||
| auto funcResultTypes = $_op->getResultTypes(); | ||
| if (funcResultTypes.size() != returnOperandTypes.size()) { | ||
| return $_op.emitOpError("The number of a FunctionOpInterface's") | ||
| << "results must match that of the ReturnLike operands."; | ||
| } | ||
|
|
||
| for (unsigned i = 0; i < funcResultTypes.size(); ++i) { | ||
| if (funcResultTypes[i] != returnOperandTypes[i]) { | ||
| return $_op.emitOpError("The result types of a FunctionOpInterface") | ||
| << "must match the operand types of the ReturnLike."; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This is way too long to be inline code in a .td file. Please move this into a helper function in a .cpp file and call it from here. Having code as code rather than inlined strings makes it analyzable and processable by tooling.
There was a problem hiding this comment.
Thank you very much. It makes sense. I'll make the changes.
Description:
This PR is a follow-up for #110322. The previous PR makes
OneShotBufferizecheck if a function operation has a terminator with theReturnLiketrait. However,transform.named_sequencedoes not expectyieldOpto have the trait. In that PR, the issue is circumvented by--transform-interpreter="debug-payload-root-tag=payload".This PR tries to tie
ReturnLiketoFunctionOpInface. However, I am not sure if it is correct to checkReturnLikein the tablegen forFunctionOpInface. I have not yet handled all the errors. Just want to create this PR for further confirmation.We might also need to wait for this fix for
transform.named_sequence(#111408) to be merged?