[mlir] Support verification order (3/3)

In this CL, update the function name of verifier according to the
behavior. If a verifier needs to access the region then it'll be updated
to `verifyRegions`.

Reviewed By: rriddle

Differential Revision: https://reviews.llvm.org/D120373

mlir/docs/OpDefinitions.md

@@ -565,18 +565,16 @@ _additional_ verification, you can use
 
 ```tablegen
 let hasVerifier = 1;
-```
-
-or
-
-```tablegen
 let hasRegionVerifier = 1;
 ```
 
-This will generate either `LogicalResult verify()` or
-`LogicalResult verifyRegions()` method declaration on the op class
-that can be defined with any additional verification constraints. These method
-will be invoked on its verification order.
+This will generate `LogicalResult verify()`/`LogicalResult verifyRegions()`
+method declarations on the op class that can be defined with any additional
+verification constraints. For verificaiton which needs to access the nested
+operations, you should use `hasRegionVerifier` to ensure that it won't access
+any ill-formed operation. Except that, The other verifications can be
+implemented with `hasVerifier`. Check the next section for the execution order
+of these verification methods.
 
 #### Verification Ordering
 

mlir/include/mlir/Dialect/Affine/IR/AffineOps.td

@@ -344,7 +344,7 @@ def AffineForOp : Affine_Op<"for",
   let hasCanonicalizer = 1;
   let hasCustomAssemblyFormat = 1;
   let hasFolder = 1;
-  let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 def AffineIfOp : Affine_Op<"if",

mlir/include/mlir/Dialect/Async/IR/AsyncOps.td

@@ -84,7 +84,7 @@ def Async_ExecuteOp :
 
   let hasCustomAssemblyFormat = 1;
   let skipDefaultBuilders = 1;
-  let hasVerifier = 1;
+  let hasRegionVerifier = 1;
   let builders = [
     OpBuilder<(ins "TypeRange":$resultTypes, "ValueRange":$dependencies,
       "ValueRange":$operands,

mlir/include/mlir/Dialect/GPU/GPUOps.td

@@ -556,7 +556,7 @@ def GPU_LaunchOp : GPU_Op<"launch", [AutomaticAllocationScope]>,
 
   let hasCanonicalizer = 1;
   let hasCustomAssemblyFormat = 1;
-  let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 def GPU_PrintfOp : GPU_Op<"printf", [MemoryEffects<[MemWrite]>]>,
@@ -677,7 +677,7 @@ def GPU_AllReduceOp : GPU_Op<"all_reduce",
   let regions = (region AnyRegion:$body);
   let assemblyFormat = [{ custom<AllReduceOperation>($op) $value $body attr-dict
                           `:` functional-type(operands, results) }];
-  let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 def GPU_ShuffleOpXor  : I32EnumAttrCase<"XOR",  0, "xor">;

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

@@ -1140,6 +1140,7 @@ def LLVM_GlobalOp : LLVM_Op<"mlir.global",
 
   let hasCustomAssemblyFormat = 1;
   let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 def LLVM_GlobalCtorsOp : LLVM_Op<"mlir.global_ctors", [
@@ -1277,6 +1278,7 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
 
   let hasCustomAssemblyFormat = 1;
   let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 def LLVM_NullOp

mlir/include/mlir/Dialect/Linalg/IR/LinalgInterfaces.td

@@ -34,6 +34,7 @@ def LinalgContractionOpInterface : OpInterface<"ContractionOpInterface"> {
   }];
   let cppNamespace = "::mlir::linalg";
   let verify = [{ return detail::verifyContractionInterface($_op); }];
+  let verifyWithRegions = 1;
   let methods = [
     InterfaceMethod<
     /*desc=*/"Returns the left-hand side operand.",
@@ -1136,6 +1137,7 @@ def LinalgStructuredInterface : OpInterface<"LinalgOp"> {
   }];
 
   let verify = [{ return detail::verifyStructuredOpInterface($_op); }];
+  let verifyWithRegions = 1;
 }
 
 #endif // LINALG_IR_LINALGINTERFACES

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

@@ -204,6 +204,7 @@ def SectionsOp : OpenMP_Op<"sections", [AttrSizedOperandSegments]> {
   }];
 
   let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//
@@ -437,7 +438,8 @@ def CriticalDeclareOp : OpenMP_Op<"critical.declare", [Symbol]> {
 }
 
 
-def CriticalOp : OpenMP_Op<"critical"> {
+def CriticalOp : OpenMP_Op<"critical",
+    [DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
   let summary = "critical construct";
   let description = [{
     The critical construct imposes a restriction on the associated structured
@@ -451,7 +453,6 @@ def CriticalOp : OpenMP_Op<"critical"> {
   let assemblyFormat = [{
     (`(` $name^ `)`)? $region attr-dict
   }];
-  let hasVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//
@@ -671,6 +672,7 @@ def AtomicUpdateOp : OpenMP_Op<"atomic.update",
     $x `:` type($x) $region attr-dict
   }];
   let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 def AtomicCaptureOp : OpenMP_Op<"atomic.capture",
@@ -717,7 +719,7 @@ def AtomicCaptureOp : OpenMP_Op<"atomic.capture",
           |`hint` `(` custom<SynchronizationHint>($hint_val) `)`)
     $region attr-dict
   }];
-  let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//
@@ -771,7 +773,7 @@ def ReductionDeclareOp : OpenMP_Op<"reduction.declare", [Symbol]> {
       return atomicReductionRegion().front().getArgument(0).getType();
     }
   }];
-  let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//

mlir/include/mlir/Dialect/PDL/IR/PDLOps.td

@@ -453,7 +453,7 @@ def PDL_PatternOp : PDL_Op<"pattern", [
     /// Returns the rewrite operation of this pattern.
     RewriteOp getRewriter();
   }];
-  let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//
@@ -632,7 +632,7 @@ def PDL_RewriteOp : PDL_Op<"rewrite", [
               ($body^)?
     attr-dict-with-keyword
   }];
-  let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//

mlir/include/mlir/Dialect/SCF/SCFOps.td

@@ -308,6 +308,7 @@ def ForOp : SCF_Op<"for",
   let hasCanonicalizer = 1;
   let hasCustomAssemblyFormat = 1;
   let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 def IfOp : SCF_Op<"if",
@@ -535,7 +536,7 @@ def ReduceOp : SCF_Op<"reduce", [HasParent<"ParallelOp">]> {
   let arguments = (ins AnyType:$operand);
   let hasCustomAssemblyFormat = 1;
   let regions = (region SizedRegion<1>:$reductionOperator);
-  let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 def ReduceReturnOp :
@@ -688,7 +689,7 @@ def WhileOp : SCF_Op<"while",
 
   let hasCanonicalizer = 1;
   let hasCustomAssemblyFormat = 1;
-  let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 def YieldOp : SCF_Op<"yield", [NoSideEffect, ReturnLike, Terminator,

mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td

@@ -309,6 +309,9 @@ def SPV_LoopOp : SPV_Op<"mlir.loop", [InFunctionScope]> {
   let hasOpcode = 0;
 
   let autogenSerialization = 0;
+
+  let hasVerifier = 0;
+  let hasRegionVerifier = 1;
 }
 
 // -----
@@ -470,6 +473,9 @@ def SPV_SelectionOp : SPV_Op<"mlir.selection", [InFunctionScope]> {
   let autogenSerialization = 0;
 
   let hasCanonicalizer = 1;
+
+  let hasVerifier = 0;
+  let hasRegionVerifier = 1;
 }
 
 #endif // MLIR_DIALECT_SPIRV_IR_CONTROLFLOW_OPS

mlir/include/mlir/Dialect/SPIRV/IR/SPIRVStructureOps.td

@@ -516,6 +516,9 @@ def SPV_ModuleOp : SPV_Op<"module",
 
     static StringRef getVCETripleAttrName() { return "vce_triple"; }
   }];
+
+  let hasVerifier = 0;
+  let hasRegionVerifier = 1;
 }
 
 // -----
@@ -749,6 +752,9 @@ def SPV_SpecConstantOperationOp : SPV_Op<"SpecConstantOperation", [
   let hasOpcode = 0;
 
   let autogenSerialization = 0;
+
+  let hasVerifier = 0;
+  let hasRegionVerifier = 1;
 }
 
 // -----

mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td

@@ -408,6 +408,7 @@ def Tensor_GenerateOp : Tensor_Op<"generate",
 
   let hasCanonicalizer = 1;
   let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//
@@ -983,6 +984,7 @@ def Tensor_PadOp : Tensor_Op<"pad", [AttrSizedOperandSegments, NoSideEffect,
   let hasCanonicalizer = 1;
   let hasFolder = 1;
   let hasVerifier = 1;
+  let hasRegionVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//

mlir/include/mlir/IR/OpBase.td

@@ -2073,9 +2073,11 @@ def ResultsBroadcastableShape :
 // X op Y == Y op X
 def Commutative  : NativeOpTrait<"IsCommutative">;
 // op op X == op X (unary) / X op X == X (binary)
-def Idempotent  : NativeOpTrait<"IsIdempotent">;
+// FIXME: Idempotent should depend on SameOperandsAndResultType
+def Idempotent : NativeOpTrait<"IsIdempotent">;
 // op op X == X
-def Involution  : NativeOpTrait<"IsInvolution">;
+// FIXME: Involution should depend on SameOperandsAndResultType
+def Involution : NativeOpTrait<"IsInvolution">;
 // Op behaves like a constant.
 def ConstantLike : NativeOpTrait<"ConstantLike">;
 // Op is isolated from above.
@@ -2103,11 +2105,11 @@ def MemRefsNormalizable : NativeOpTrait<"MemRefsNormalizable">;
 // Op is elementwise on tensor/vector operands and results.
 def Elementwise : NativeOpTrait<"Elementwise">;
 // Elementwise op can be applied to scalars instead tensor/vector operands.
-def Scalarizable : NativeOpTrait<"Scalarizable">;
+def Scalarizable : NativeOpTrait<"Scalarizable", [Elementwise]>;
 // Elementwise op can be applied to all-vector operands.
-def Vectorizable : NativeOpTrait<"Vectorizable">;
+def Vectorizable : NativeOpTrait<"Vectorizable", [Elementwise]>;
 // Elementwise op can be applied to all-tensor operands.
-def Tensorizable : NativeOpTrait<"Tensorizable">;
+def Tensorizable : NativeOpTrait<"Tensorizable", [Elementwise]>;
 
 // Group together `Elementwise`, `Scalarizable`, `Vectorizable`, and
 // `Tensorizable` for convenience.
@@ -2489,7 +2491,9 @@ class Op<Dialect dialect, string mnemonic, list<Trait> props = []> {
   // verified (aside from those verified by other ODS constructs). If set to `1`,
   // an additional `LogicalResult verify()` declaration will be generated on the
   // operation class. The operation should implement this method and verify the
-  // additional necessary invariants.
+  // additional necessary invariants. This verifier shouldn't access any nested
+  // operations because those operations may ill-formed. Use the
+  // `hasRegionVerifier` below instead.
   bit hasVerifier = 0;
 
   // A bit indicating if the operation has additional invariants that need to

mlir/include/mlir/IR/OpDefinition.h

@@ -920,7 +920,7 @@ struct SingleBlockImplicitTerminator {
     /// The type of the operation used as the implicit terminator type.
     using ImplicitTerminatorOpT = TerminatorOpType;
 
-    static LogicalResult verifyTrait(Operation *op) {
+    static LogicalResult verifyRegionTrait(Operation *op) {
       if (failed(Base::verifyTrait(op)))
         return failure();
       for (unsigned i = 0, e = op->getNumRegions(); i < e; ++i) {
@@ -1218,7 +1218,7 @@ template <typename ConcreteType>
 class IsIsolatedFromAbove
     : public TraitBase<ConcreteType, IsIsolatedFromAbove> {
 public:
-  static LogicalResult verifyTrait(Operation *op) {
+  static LogicalResult verifyRegionTrait(Operation *op) {
     return impl::verifyIsIsolatedFromAbove(op);
   }
 };
@@ -1262,7 +1262,7 @@ struct HasParent {
   class Impl : public TraitBase<ConcreteType, Impl> {
   public:
     static LogicalResult verifyTrait(Operation *op) {
-      if (llvm::isa<ParentOpTypes...>(op->getParentOp()))
+      if (llvm::isa_and_nonnull<ParentOpTypes...>(op->getParentOp()))
         return success();
 
       return op->emitOpError()

mlir/include/mlir/IR/SymbolTable.h

@@ -337,7 +337,7 @@ namespace OpTrait {
 template <typename ConcreteType>
 class SymbolTable : public TraitBase<ConcreteType, SymbolTable> {
 public:
-  static LogicalResult verifyTrait(Operation *op) {
+  static LogicalResult verifyRegionTrait(Operation *op) {
     return ::mlir::detail::verifySymbolTable(op);
   }
 

mlir/include/mlir/Interfaces/InferTypeOpInterface.td

@@ -64,6 +64,8 @@ def InferTypeOpInterface : OpInterface<"InferTypeOpInterface"> {
     >,
   ];
 
+  // Inferring result types may need to access the region operations.
+  let verifyWithRegions = 1;
   let verify = [{
     return detail::verifyInferredResultTypes($_op);
   }];

mlir/lib/Dialect/Affine/IR/AffineOps.cpp

@@ -1304,7 +1304,7 @@ void AffineForOp::build(OpBuilder &builder, OperationState &result, int64_t lb,
                bodyBuilder);
 }
 
-LogicalResult AffineForOp::verify() {
+LogicalResult AffineForOp::verifyRegions() {
   // Check that the body defines as single block argument for the induction
   // variable.
   auto *body = getBody();

mlir/lib/Dialect/Async/IR/Async.cpp

@@ -236,7 +236,7 @@ ParseResult ExecuteOp::parse(OpAsmParser &parser, OperationState &result) {
   return success();
 }
 
-LogicalResult ExecuteOp::verify() {
+LogicalResult ExecuteOp::verifyRegions() {
   // Unwrap async.execute value operands types.
   auto unwrappedTypes = llvm::map_range(operands(), [](Value operand) {
     return operand.getType().cast<ValueType>().getValueType();

mlir/lib/Dialect/GPU/IR/GPUDialect.cpp

@@ -275,7 +275,7 @@ LogicalResult GPUDialect::verifyOperationAttribute(Operation *op,
   return walkResult.wasInterrupted() ? failure() : success();
 }
 
-LogicalResult gpu::AllReduceOp::verify() {
+LogicalResult gpu::AllReduceOp::verifyRegions() {
   if (body().empty() != op().hasValue())
     return emitError("expected either an op attribute or a non-empty body");
   if (!body().empty()) {
@@ -407,7 +407,7 @@ KernelDim3 LaunchOp::getBlockSizeOperandValues() {
   return KernelDim3{getOperand(3), getOperand(4), getOperand(5)};
 }
 
-LogicalResult LaunchOp::verify() {
+LogicalResult LaunchOp::verifyRegions() {
   // Kernel launch takes kNumConfigOperands leading operands for grid/block
   // sizes and transforms them into kNumConfigRegionAttributes region arguments
   // for block/thread identifiers and grid/block sizes.

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

@@ -1795,26 +1795,6 @@ LogicalResult GlobalOp::verify() {
           "attribute");
   }
 
-  if (Block *b = getInitializerBlock()) {
-    ReturnOp ret = cast<ReturnOp>(b->getTerminator());
-    if (ret.operand_type_begin() == ret.operand_type_end())
-      return emitOpError("initializer region cannot return void");
-    if (*ret.operand_type_begin() != getType())
-      return emitOpError("initializer region type ")
-             << *ret.operand_type_begin() << " does not match global type "
-             << getType();
-
-    for (Operation &op : *b) {
-      auto iface = dyn_cast<MemoryEffectOpInterface>(op);
-      if (!iface || !iface.hasNoEffect())
-        return op.emitError()
-               << "ops with side effects not allowed in global initializers";
-    }
-
-    if (getValueOrNull())
-      return emitOpError("cannot have both initializer value and region");
-  }
-
   if (getLinkage() == Linkage::Common) {
     if (Attribute value = getValueOrNull()) {
       if (!isZeroAttribute(value)) {
@@ -1843,6 +1823,30 @@ LogicalResult GlobalOp::verify() {
   return success();
 }
 
+LogicalResult GlobalOp::verifyRegions() {
+  if (Block *b = getInitializerBlock()) {
+    ReturnOp ret = cast<ReturnOp>(b->getTerminator());
+    if (ret.operand_type_begin() == ret.operand_type_end())
+      return emitOpError("initializer region cannot return void");
+    if (*ret.operand_type_begin() != getType())
+      return emitOpError("initializer region type ")
+             << *ret.operand_type_begin() << " does not match global type "
+             << getType();
+
+    for (Operation &op : *b) {
+      auto iface = dyn_cast<MemoryEffectOpInterface>(op);
+      if (!iface || !iface.hasNoEffect())
+        return op.emitError()
+               << "ops with side effects not allowed in global initializers";
+    }
+
+    if (getValueOrNull())
+      return emitOpError("cannot have both initializer value and region");
+  }
+
+  return success();
+}
+
 //===----------------------------------------------------------------------===//
 // LLVM::GlobalCtorsOp
 //===----------------------------------------------------------------------===//
@@ -2123,7 +2127,6 @@ LogicalResult LLVMFuncOp::verifyType() {
 // - functions don't have 'common' linkage
 // - external functions have 'external' or 'extern_weak' linkage;
 // - vararg is (currently) only supported for external functions;
-// - entry block arguments are of LLVM types and match the function signature.
 LogicalResult LLVMFuncOp::verify() {
   if (getLinkage() == LLVM::Linkage::Common)
     return emitOpError() << "functions cannot have '"
@@ -2152,6 +2155,15 @@ LogicalResult LLVMFuncOp::verify() {
   if (isVarArg())
     return emitOpError("only external functions can be variadic");
 
+  return success();
+}
+
+/// Verifies LLVM- and implementation-specific properties of the LLVM func Op:
+/// - entry block arguments are of LLVM types and match the function signature.
+LogicalResult LLVMFuncOp::verifyRegions() {
+  if (isExternal())
+    return success();
+
   unsigned numArguments = getType().getNumParams();
   Block &entryBlock = front();
   for (unsigned i = 0; i < numArguments; ++i) {

mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp

@@ -1207,7 +1207,7 @@ LogicalResult linalg::YieldOp::verify() {
     return emitOpError("expected single non-empty parent region");
 
   if (auto linalgOp = dyn_cast<LinalgOp>(parentOp))
-    return verifyYield(*this, cast<LinalgOp>(parentOp));
+    return verifyYield(*this, linalgOp);
 
   return emitOpError("expected parent op with LinalgOp interface");
 }

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

@@ -369,6 +369,8 @@ static LogicalResult verifyReductionVarList(Operation *op,
     return success();
   }
 
+  // TODO: The followings should be done in
+  // SymbolUserOpInterface::verifySymbolUses.
   DenseSet<Value> accumulators;
   for (auto args : llvm::zip(reductionVars, *reductions)) {
     Value accum = std::get<0>(args);
@@ -719,6 +721,10 @@ LogicalResult SectionsOp::verify() {
     return emitError(
         "expected equal sizes for allocate and allocator variables");
 
+  return verifyReductionVarList(*this, reductions(), reduction_vars());
+}
+
+LogicalResult SectionsOp::verifyRegions() {
   for (auto &inst : *region().begin()) {
     if (!(isa<SectionOp>(inst) || isa<TerminatorOp>(inst))) {
       return emitOpError()
@@ -726,7 +732,7 @@ LogicalResult SectionsOp::verify() {
     }
   }
 
-  return verifyReductionVarList(*this, reductions(), reduction_vars());
+  return success();
 }
 
 /// Parses an OpenMP Workshare Loop operation
@@ -851,7 +857,7 @@ static void printAtomicReductionRegion(OpAsmPrinter &printer,
   printer.printRegion(region);
 }
 
-LogicalResult ReductionDeclareOp::verify() {
+LogicalResult ReductionDeclareOp::verifyRegions() {
   if (initializerRegion().empty())
     return emitOpError() << "expects non-empty initializer region";
   Block &initializerEntryBlock = initializerRegion().front();
@@ -941,10 +947,11 @@ LogicalResult CriticalDeclareOp::verify() {
   return verifySynchronizationHint(*this, hint_val());
 }
 
-LogicalResult CriticalOp::verify() {
+LogicalResult
+CriticalOp::verifySymbolUses(SymbolTableCollection &symbol_table) {
   if (nameAttr()) {
     SymbolRefAttr symbolRef = nameAttr();
-    auto decl = SymbolTable::lookupNearestSymbolFrom<CriticalDeclareOp>(
+    auto decl = symbol_table.lookupNearestSymbolFrom<CriticalDeclareOp>(
         *this, symbolRef);
     if (!decl) {
       return emitOpError() << "expected symbol reference " << symbolRef
@@ -1038,15 +1045,19 @@ LogicalResult AtomicUpdateOp::verify() {
     }
   }
 
-  if (region().getNumArguments() != 1)
-    return emitError("the region must accept exactly one argument");
-
   if (x().getType().cast<PointerLikeType>().getElementType() !=
       region().getArgument(0).getType()) {
     return emitError("the type of the operand must be a pointer type whose "
                      "element type is the same as that of the region argument");
   }
 
+  return success();
+}
+
+LogicalResult AtomicUpdateOp::verifyRegions() {
+  if (region().getNumArguments() != 1)
+    return emitError("the region must accept exactly one argument");
+
   if (region().front().getOperations().size() < 2)
     return emitError() << "the update region must have at least two operations "
                           "(binop and terminator)";
@@ -1064,7 +1075,7 @@ LogicalResult AtomicUpdateOp::verify() {
 // Verifier for AtomicCaptureOp
 //===----------------------------------------------------------------------===//
 
-LogicalResult AtomicCaptureOp::verify() {
+LogicalResult AtomicCaptureOp::verifyRegions() {
   Block::OpListType &ops = region().front().getOperations();
   if (ops.size() != 3)
     return emitError()

mlir/lib/Dialect/PDL/IR/PDL.cpp

@@ -269,7 +269,7 @@ bool OperationOp::hasTypeInference() {
 // pdl::PatternOp
 //===----------------------------------------------------------------------===//
 
-LogicalResult PatternOp::verify() {
+LogicalResult PatternOp::verifyRegions() {
   Region &body = getBodyRegion();
   Operation *term = body.front().getTerminator();
   auto rewriteOp = dyn_cast<RewriteOp>(term);
@@ -402,7 +402,7 @@ LogicalResult ResultsOp::verify() {
 // pdl::RewriteOp
 //===----------------------------------------------------------------------===//
 
-LogicalResult RewriteOp::verify() {
+LogicalResult RewriteOp::verifyRegions() {
   Region &rewriteRegion = body();
 
   // Handle the case where the rewrite is external.

mlir/lib/Dialect/SCF/SCF.cpp

@@ -282,6 +282,19 @@ LogicalResult ForOp::verify() {
     if (cst.value() <= 0)
       return emitOpError("constant step operand must be positive");
 
+  auto opNumResults = getNumResults();
+  if (opNumResults == 0)
+    return success();
+  // If ForOp defines values, check that the number and types of
+  // the defined values match ForOp initial iter operands and backedge
+  // basic block arguments.
+  if (getNumIterOperands() != opNumResults)
+    return emitOpError(
+        "mismatch in number of loop-carried values and defined values");
+  return success();
+}
+
+LogicalResult ForOp::verifyRegions() {
   // Check that the body defines as single block argument for the induction
   // variable.
   auto *body = getBody();
@@ -293,15 +306,11 @@ LogicalResult ForOp::verify() {
   auto opNumResults = getNumResults();
   if (opNumResults == 0)
     return success();
-  // If ForOp defines values, check that the number and types of
-  // the defined values match ForOp initial iter operands and backedge
-  // basic block arguments.
-  if (getNumIterOperands() != opNumResults)
-    return emitOpError(
-        "mismatch in number of loop-carried values and defined values");
+
   if (getNumRegionIterArgs() != opNumResults)
     return emitOpError(
         "mismatch in number of basic block args and defined values");
+
   auto iterOperands = getIterOperands();
   auto iterArgs = getRegionIterArgs();
   auto opResults = getResults();
@@ -2130,7 +2139,7 @@ void ReduceOp::build(
                   body->getArgument(1));
 }
 
-LogicalResult ReduceOp::verify() {
+LogicalResult ReduceOp::verifyRegions() {
   // The region of a ReduceOp has two arguments of the same type as its operand.
   auto type = getOperand().getType();
   Block &block = getReductionOperator().front();
@@ -2333,7 +2342,7 @@ static TerminatorTy verifyAndGetTerminator(scf::WhileOp op, Region &region,
   return nullptr;
 }
 
-LogicalResult scf::WhileOp::verify() {
+LogicalResult scf::WhileOp::verifyRegions() {
   auto beforeTerminator = verifyAndGetTerminator<scf::ConditionOp>(
       *this, getBefore(),
       "expects the 'before' region to terminate with 'scf.condition'");

mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp

@@ -2935,7 +2935,7 @@ static inline bool hasOneBranchOpTo(Block &srcBlock, Block &dstBlock) {
   return branchOp && branchOp.getSuccessor() == &dstBlock;
 }
 
-LogicalResult spirv::LoopOp::verify() {
+LogicalResult spirv::LoopOp::verifyRegions() {
   auto *op = getOperation();
 
   // We need to verify that the blocks follow the following layout:
@@ -3072,6 +3072,7 @@ LogicalResult spirv::MergeOp::verify() {
     return emitOpError(
         "expected parent op to be 'spv.mlir.selection' or 'spv.mlir.loop'");
 
+  // TODO: This check should be done in `verifyRegions` of parent op.
   Block &parentLastBlock = (*this)->getParentRegion()->back();
   if (getOperation() != parentLastBlock.getTerminator())
     return emitOpError("can only be used in the last block of "
@@ -3173,7 +3174,7 @@ void spirv::ModuleOp::print(OpAsmPrinter &printer) {
   printer.printRegion(getRegion());
 }
 
-LogicalResult spirv::ModuleOp::verify() {
+LogicalResult spirv::ModuleOp::verifyRegions() {
   Dialect *dialect = (*this)->getDialect();
   DenseMap<std::pair<spirv::FuncOp, spirv::ExecutionModel>, spirv::EntryPointOp>
       entryPoints;
@@ -3322,7 +3323,7 @@ void spirv::SelectionOp::print(OpAsmPrinter &printer) {
                       /*printBlockTerminators=*/true);
 }
 
-LogicalResult spirv::SelectionOp::verify() {
+LogicalResult spirv::SelectionOp::verifyRegions() {
   auto *op = getOperation();
 
   // We need to verify that the blocks follow the following layout:
@@ -4106,7 +4107,7 @@ void spirv::SpecConstantOperationOp::print(OpAsmPrinter &printer) {
   printer.printGenericOp(&body().front().front());
 }
 
-LogicalResult spirv::SpecConstantOperationOp::verify() {
+LogicalResult spirv::SpecConstantOperationOp::verifyRegions() {
   Block &block = getRegion().getBlocks().front();
 
   if (block.getOperations().size() != 2)

mlir/lib/Dialect/Tensor/IR/TensorOps.cpp

@@ -533,6 +533,11 @@ LogicalResult GenerateOp::verify() {
     return emitError("must have as many index operands as dynamic extents "
                      "in the result type");
 
+  return success();
+}
+
+LogicalResult GenerateOp::verifyRegions() {
+  RankedTensorType resultTy = getType().cast<RankedTensorType>();
   // Ensure that region arguments span the index space.
   if (!llvm::all_of(body().getArgumentTypes(),
                     [](Type ty) { return ty.isIndex(); }))
@@ -1749,8 +1754,12 @@ LogicalResult PadOp::verify() {
            << expectedType;
   }
 
+  return success();
+}
+
+LogicalResult PadOp::verifyRegions() {
   auto &region = getRegion();
-  unsigned rank = resultType.getRank();
+  unsigned rank = result().getType().cast<RankedTensorType>().getRank();
   Block &block = region.front();
   if (block.getNumArguments() != rank)
     return emitError("expected the block to have ") << rank << " arguments";

mlir/lib/IR/Operation.cpp

@@ -1088,12 +1088,6 @@ LogicalResult OpTrait::impl::verifyIsIsolatedFromAbove(Operation *isolatedOp) {
     while (!pendingRegions.empty()) {
       for (Operation &op : pendingRegions.pop_back_val()->getOps()) {
         for (Value operand : op.getOperands()) {
-          // operand should be non-null here if the IR is well-formed. But
-          // we don't assert here as this function is called from the verifier
-          // and so could be called on invalid IR.
-          if (!operand)
-            return op.emitOpError("operation's operand is null");
-
           // Check that any value that is used by an operation is defined in the
           // same region as either an operation result.
           auto *operandRegion = operand.getParentRegion();

mlir/test/Dialect/GPU/invalid.mlir

@@ -15,7 +15,7 @@ func @no_region_attrs(%sz : index) {
  "gpu.launch"(%sz, %sz, %sz, %sz, %sz, %sz) ({
   ^bb1(%bx: index, %by: index, %bz: index,
        %tx: index, %ty: index, %tz: index):
-    gpu.return
+    gpu.terminator
   }) : (index, index, index, index, index, index) -> ()
   return
 }
@@ -26,8 +26,9 @@ func @launch_requires_gpu_return(%sz : index) {
   // @expected-note@+1 {{in 'gpu.launch' body region}}
   gpu.launch blocks(%bx, %by, %bz) in (%sbx = %sz, %sby = %sz, %sbz = %sz)
              threads(%tx, %ty, %tz) in (%stx = %sz, %sty = %sz, %stz = %sz) {
-    // @expected-error@+1 {{expected 'gpu.terminator' or a terminator with successors}}
-    return
+    // @expected-error@+2 {{expected 'gpu.terminator' or a terminator with successors}}
+    %one = arith.constant 1 : i32
+    "gpu.yield"(%one) : (i32) -> ()
   }
   return
 }
@@ -293,7 +294,7 @@ func @reduce_incorrect_yield(%arg0 : f32) {
   // expected-error@+1 {{expected gpu.yield op in region}}
   %res = gpu.all_reduce %arg0 {
   ^bb(%lhs : f32, %rhs : f32):
-    return
+    "test.finish" () : () -> ()
   } : (f32) -> (f32)
   return
 }

mlir/test/Dialect/LLVMIR/global.mlir

@@ -172,8 +172,7 @@ llvm.func @bar() {
 
 // -----
 
-// expected-error @+2 {{'llvm.mlir.global' op expects regions to end with 'llvm.return', found 'llvm.mlir.constant'}}
-// expected-note @+1 {{in custom textual format, the absence of terminator implies 'llvm.return'}}
+// expected-error @+2 {{block with no terminator}}
 llvm.mlir.global internal @g() : i64 {
   %c = llvm.mlir.constant(42 : i64) : i64
 }

mlir/test/Dialect/Linalg/invalid.mlir

@@ -202,7 +202,7 @@ func @generic_empty_region(%arg0: memref<f32>) {
 // -----
 
 func @generic_mismatched_num_arguments(%arg0: memref<f32>) {
-  // expected-error @+1 {{expected as many non-induction variable region arguments as the number of input/output operands}}
+  // expected-error @+6 {{'linalg.yield' op expected number of yield values (2) to match the number of operands of the enclosing LinalgOp (1)}}
   linalg.generic {
       indexing_maps =  [ affine_map<() -> ()>, affine_map<() -> ()> ],
       iterator_types = []}
@@ -215,7 +215,7 @@ func @generic_mismatched_num_arguments(%arg0: memref<f32>) {
 // -----
 
 func @generic_shaped_operand_block_arg_type(%arg0: memref<f32>) {
-  // expected-error @+1 {{expected type of bb argument #0 ('i1') to match element or self type of the corresponding operand ('f32')}}
+  // expected-error @+6 {{'linalg.yield' op type of yield operand 1 ('i1') doesn't match the element type of the enclosing linalg.generic op ('f32')}}
   linalg.generic {
     indexing_maps =  [ affine_map<() -> ()> ],
     iterator_types = []}
@@ -228,7 +228,7 @@ func @generic_shaped_operand_block_arg_type(%arg0: memref<f32>) {
 // -----
 
 func @generic_scalar_operand_block_arg_type(%arg0: tensor<f32>) {
-  // expected-error @+1 {{expected type of bb argument #0 ('i1') to match element or self type of the corresponding operand ('f32')}}
+  // expected-error @+6 {{'linalg.yield' op type of yield operand 1 ('i1') doesn't match the element type of the enclosing linalg.generic op ('f32')}}
   linalg.generic {
     indexing_maps =  [ affine_map<() -> ()> ],
     iterator_types = []}
@@ -284,15 +284,14 @@ func @generic_result_tensor_type(%arg0: memref<?xf32, affine_map<(i)[off]->(off
 
 // -----
 
-func @generic(%arg0: memref<?x?xi4>) {
-  // expected-error @+2 {{op expects regions to end with 'linalg.yield', found 'arith.addf'}}
-  // expected-note @+1 {{in custom textual format, the absence of terminator implies 'linalg.yield'}}
+func @generic(%arg0: memref<?x?xf32>) {
+  // expected-error @+6 {{block with no terminator, has %0 = "arith.addf"(%arg1, %arg1) : (f32, f32) -> f32}}
   linalg.generic  {
     indexing_maps = [ affine_map<(i, j) -> (i, j)> ],
     iterator_types = ["parallel", "parallel"]}
-      outs(%arg0 : memref<?x?xi4>) {
-    ^bb(%0: i4) :
-      %1 = arith.addf %0, %0: i4
+      outs(%arg0 : memref<?x?xf32>) {
+    ^bb(%0: f32) :
+      %1 = arith.addf %0, %0: f32
   }
   return
 }

mlir/test/Dialect/PDL/invalid.mlir

@@ -159,7 +159,7 @@ pdl.pattern : benefit(1) {
 // expected-error@below {{expected body to terminate with `pdl.rewrite`}}
 pdl.pattern : benefit(1) {
   // expected-note@below {{see terminator defined here}}
-  return
+  "test.finish" () : () -> ()
 }
 
 // -----

mlir/test/Dialect/SCF/invalid.mlir

@@ -241,7 +241,7 @@ func @reduce_empty_block(%arg0 : index, %arg1 : f32) {
   %zero = arith.constant 0.0 : f32
   %res = scf.parallel (%i0) = (%arg0) to (%arg0)
                                        step (%arg0) init (%zero) -> f32 {
-    // expected-error@+1 {{the block inside reduce should not be empty}}
+    // expected-error@+1 {{empty block: expect at least a terminator}}
     scf.reduce(%arg1) : f32 {
       ^bb0(%lhs : f32, %rhs : f32):
     }
@@ -289,7 +289,7 @@ func @reduce_wrong_terminator(%arg0 : index, %arg1 : f32) {
     // expected-error@+1 {{the block inside reduce should be terminated with a 'scf.reduce.return' op}}
     scf.reduce(%arg1) : f32 {
       ^bb0(%lhs : f32, %rhs : f32):
-        scf.yield
+        "test.finish" () : () -> ()
     }
   }
   return

mlir/test/Dialect/Tensor/invalid.mlir

@@ -91,8 +91,7 @@ func @tensor.generate(%m : index, %n : index)
 
 func @tensor.generate(%m : index, %n : index)
     -> tensor<?x3x?xf32> {
-  // expected-error @+2 {{op expects regions to end with 'tensor.yield', found 'func.return'}}
-  // expected-note @+1 {{in custom textual format, the absence of terminator implies 'tensor.yield'}}
+  // expected-error @+4 {{'func.return' op expects parent op 'builtin.func'}}
   %tnsr = tensor.generate %m, %n {
     ^bb0(%i : index, %j : index, %k : index):
       %elem = arith.constant 8.0 : f32
@@ -377,4 +376,4 @@ func @invalid_splat(%v : vector<8xf32>) {
   // expected-error@+1 {{must be integer/index/float type}}
   %w = tensor.splat %v : tensor<8xvector<8xf32>>
   return
-}
+}

mlir/test/IR/invalid.mlir

@@ -536,8 +536,7 @@ func @return_type_mismatch() -> i32 {
 
 func @return_inside_loop() {
   affine.for %i = 1 to 100 {
-    // expected-error@-1 {{op expects regions to end with 'affine.yield', found 'func.return'}}
-    // expected-note@-2 {{in custom textual format, the absence of terminator implies}}
+    // expected-error@+1 {{'func.return' op expects parent op 'builtin.func'}}
     return
   }
   return

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

@@ -2297,10 +2297,6 @@ void OpEmitter::genCustomVerifier() {
   if (def.getValueAsBit("hasVerifier")) {
     auto *method = opClass.declareMethod("::mlir::LogicalResult", "verify");
     ERROR_IF_PRUNED(method, "verify", op);
-  } else if (def.getValueAsBit("hasRegionVerifier")) {
-    auto *method =
-        opClass.declareMethod("::mlir::LogicalResult", "verifyRegions");
-    ERROR_IF_PRUNED(method, "verifyRegions", op);
   } else if (hasCustomVerifyCodeBlock) {
     auto *method = opClass.addMethod("::mlir::LogicalResult", "verify");
     ERROR_IF_PRUNED(method, "verify", op);
@@ -2311,6 +2307,12 @@ void OpEmitter::genCustomVerifier() {
     auto printer = stringInit->getValue().ltrim().rtrim(" \t\v\f\r");
     body << "  " << tgfmt(printer, &fctx);
   }
+
+  if (def.getValueAsBit("hasRegionVerifier")) {
+    auto *method =
+        opClass.declareMethod("::mlir::LogicalResult", "verifyRegions");
+    ERROR_IF_PRUNED(method, "verifyRegions", op);
+  }
 }
 
 void OpEmitter::genOperandResultVerifier(MethodBody &body,