-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[mlir][linalg] Fix crashes in parser on linalg ops without operands #97944
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
Conversation
`parseDstStyleOp` parses both `ins()` and `outs()` optionally. The parsers for `linalg.transpose`, `linalg.broadcast` and `linalg.map` however assume that at least one operand is present in the state, leading to crashes otherwise. This patch adds checks to the parsers which stop them from crashing if no operands were parsed. After the Ops are parsed successfuly, the verifier takes it from there. Fix llvm#97857
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-linalg Author: Felix Schneider (ubfx) Changes
This patch adds checks to the parsers which stop them from crashing if no operands were parsed. After the Ops are parsed successfuly, the verifier takes it from there. Fix #97857 Full diff: https://github.com/llvm/llvm-project/pull/97944.diff 2 Files Affected:
diff --git a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
index 57d126603ebd7..dec84ed579096 100644
--- a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
+++ b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
@@ -1356,8 +1356,10 @@ ParseResult MapOp::parse(OpAsmParser &parser, OperationState &result) {
return failure();
if (payloadOpName.has_value()) {
- addBodyWithPayloadOp(parser, result, payloadOpName.value(), payloadOpAttrs,
- ArrayRef(result.operands).drop_back());
+ if (!result.operands.empty())
+ addBodyWithPayloadOp(parser, result, payloadOpName.value(),
+ payloadOpAttrs,
+ ArrayRef(result.operands).drop_back());
} else {
SmallVector<OpAsmParser::Argument> regionArgs;
if (parser.parseArgumentList(regionArgs, OpAsmParser::Delimiter::Paren,
@@ -1739,7 +1741,8 @@ static void buildIdentityRegion(OpBuilder &builder, Location loc,
ValueRange outputs) {
buildGenericRegion(builder, loc, region, inputs, outputs,
[](OpBuilder &b, Location loc, ValueRange args) {
- b.create<linalg::YieldOp>(loc, args[0]);
+ if (!args.empty())
+ b.create<linalg::YieldOp>(loc, args[0]);
});
}
diff --git a/mlir/test/Dialect/Linalg/invalid.mlir b/mlir/test/Dialect/Linalg/invalid.mlir
index 44c81c31ace0f..cfd269e3e6e3a 100644
--- a/mlir/test/Dialect/Linalg/invalid.mlir
+++ b/mlir/test/Dialect/Linalg/invalid.mlir
@@ -455,6 +455,18 @@ func.func @map_input_output_shape_mismatch(
// -----
+func.func @map_no_operands(
+ %lhs: tensor<64xf32>, %rhs: tensor<64xf32>, %init: tensor<64xf32>)
+ -> tensor<64xf32> {
+ // This must not crash the parser.
+ linalg.map { arith.addf }
+ // expected-error @+1 {{cannot name an operation with no results}}
+ %add = linalg.map { arith.addf }
+ func.return %add : tensor<64xf32>
+}
+
+// -----
+
func.func @reduce_input_vs_init_dimension_mismatch(
%input: tensor<16x32x64xf32>,
%init: tensor<16x64xf32>) -> tensor<16x64xf32> {
@@ -676,6 +688,16 @@ func.func @transpose_input_init_rank_mismatch(%input: tensor<16x32xf32>,
// -----
+func.func @transpose_no_operands() -> tensor<32x64x16xf32> {
+ // This must not crash the parser.
+ linalg.transpose permutation = [1, 0, 2]
+ // expected-error @+1 {{cannot name an operation with no results}}
+ %transpose = linalg.transpose permutation = [1, 0, 2]
+ func.return %transpose : tensor<32x64x16xf32>
+}
+
+// -----
+
func.func @broadcast_input_dims_rank_mismatch(
%input: tensor<4x16xf32>, %init: tensor<4x8x16xf32>)
-> tensor<4x8x16xf32> {
@@ -725,6 +747,15 @@ func.func @broadcast_size_1_extension_not_supported(
dimensions = [1]
func.return %bcast : tensor<4x?x16xf32>
}
+// -----
+
+func.func @broadcast_no_operands()
+ -> tensor<4x?x16xf32> {
+ linalg.broadcast dimensions = [1]
+ // expected-error @+1 {{cannot name an operation with no results}}
+ %broadcast = linalg.broadcast dimensions = [1]
+ func.return %broadcast : tensor<32x64x16xf32>
+}
// -----
|
7e78483
to
148e1ec
Compare
parseDstStyleOp
parses bothins()
andouts()
optionally. The parsers forlinalg.transpose
,linalg.broadcast
andlinalg.map
however assume that at least one operand is present in the state, leading to crashes otherwise.This patch adds checks to the parsers which stop them from crashing if no operands were parsed. After the Ops are parsed successfuly, the verifier takes it from there.
Fix #97857