diff --git a/mlir/include/mlir/Dialect/Func/IR/FuncOps.td b/mlir/include/mlir/Dialect/Func/IR/FuncOps.td index ce2b7228cb954..a31b860276099 100644 --- a/mlir/include/mlir/Dialect/Func/IR/FuncOps.td +++ b/mlir/include/mlir/Dialect/Func/IR/FuncOps.td @@ -357,7 +357,7 @@ def FuncOp : Func_Op<"func", [ bool isDeclaration() { return isExternal(); } }]; let hasCustomAssemblyFormat = 1; - let hasVerifier = 1; + let hasRegionVerifier = 1; } //===----------------------------------------------------------------------===// diff --git a/mlir/lib/Dialect/Func/IR/FuncOps.cpp b/mlir/lib/Dialect/Func/IR/FuncOps.cpp index 2749ad5262f2b..d803e99154499 100644 --- a/mlir/lib/Dialect/Func/IR/FuncOps.cpp +++ b/mlir/lib/Dialect/Func/IR/FuncOps.cpp @@ -284,7 +284,7 @@ FuncOp FuncOp::clone() { // ReturnOp //===----------------------------------------------------------------------===// -LogicalResult FuncOp::verify() { +LogicalResult FuncOp::verifyRegions() { // External declarations have no body to check. if (isDeclaration()) return success(); diff --git a/mlir/test/Dialect/Func/invalid.mlir b/mlir/test/Dialect/Func/invalid.mlir index 2620d284c1d06..3143bda77ebba 100644 --- a/mlir/test/Dialect/Func/invalid.mlir +++ b/mlir/test/Dialect/Func/invalid.mlir @@ -213,3 +213,13 @@ func.func @region_branch_term_count_mismatch(%arg: i32) { // expected-error @+1 {{'test.loop_block_term' op has 1 operands, but enclosing function (@region_branch_term_count_mismatch) returns 0}} test.loop_block_term iter %arg exit %0 } + +// ----- + +// After moving the return-type checks from FuncOp::verify() to +// FuncOp::verifyRegions() (hasRegionVerifier=1), the terminator is verified +// first and emits a clean diagnostic instead of crashing. +func.func @func_verifier_runs_before_region() -> () { + // expected-error @below {{requires 'valid' unit attribute}} + test.crashing_return +} diff --git a/mlir/test/lib/Dialect/Test/TestOpDefs.cpp b/mlir/test/lib/Dialect/Test/TestOpDefs.cpp index c243bd79a44a8..7cf728f933395 100644 --- a/mlir/test/lib/Dialect/Test/TestOpDefs.cpp +++ b/mlir/test/lib/Dialect/Test/TestOpDefs.cpp @@ -1287,6 +1287,24 @@ LoopBlockTerminatorOp::getMutableSuccessorOperands(RegionSuccessor successor) { return getNextIterArgMutable(); } +//===----------------------------------------------------------------------===// +// TestCrashingReturnOp +//===----------------------------------------------------------------------===// + +LogicalResult TestCrashingReturnOp::verify() { + if (!getValid()) + return emitOpError("requires 'valid' unit attribute"); + return success(); +} + +MutableOperandRange +TestCrashingReturnOp::getMutableSuccessorOperands(RegionSuccessor successor) { + if (!getValid()) + llvm::report_fatal_error("getMutableSuccessorOperands called on unverified " + "test.crashing_return"); + return getArgsMutable(); +} + //===----------------------------------------------------------------------===// // SwitchWithNoBreakOp //===----------------------------------------------------------------------===// diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td index fe02536a1df5b..bd0b6e25efa53 100644 --- a/mlir/test/lib/Dialect/Test/TestOps.td +++ b/mlir/test/lib/Dialect/Test/TestOps.td @@ -2239,6 +2239,17 @@ def TestReturnOp : TEST_Op<"return", [Pure, ReturnLike, Terminator]> { [{ build($_builder, $_state, {}); }]> ]; } +// A return-like terminator used to test FuncOp verifier ordering. +// When 'valid' unit-attr is absent the op is malformed; its verifier +// emits an error, but getMutableSuccessorOperands() calls report_fatal_error +// to expose the fact that FuncOp::verify() runs before the region is checked. +def TestCrashingReturnOp : TEST_Op<"crashing_return", [ + DeclareOpInterfaceMethods, + Terminator]> { + let arguments = (ins Variadic:$args, UnitAttr:$valid); + let assemblyFormat = "($args^ `:` type($args))? attr-dict"; + let hasVerifier = 1; +} def TestCastOp : TEST_Op<"cast">, Arguments<(ins Variadic)>, Results<(outs AnyType)>; def TestInvalidOp : TEST_Op<"invalid", [Terminator]>,