[mlir][IR] SingleBlockImplicitTerminator: Declare "inherited" trait in ODS instead of C++

[springerm]

Summary

SingleBlockImplicitTerminator is now a combination of two traits: SingleBlock and SingleBlockImplicitTerminatorImpl (the original SingleBlockImplicitTerminator).

This change makes it possible to check if the SingleBlock op trait is implemented. Until now, Operation::hasTrait<OpTrait::SingleBlock>() returned false for ops that implement SingleBlockImplicitTerminator.

Discussion: https://discourse.llvm.org/t/optrait-inheritance-should-be-discouraged/73100

[springerm]

address comments from Discourse

[zero9178]

CI seems to complain that OpDefinition.h is not properly clang-formatted which should probably be fixed (line 945 if I had to guess).

Otherwise LGTM.

[jpienaar]

Nice, thanks

mlir/include/mlir/IR/OpBase.td

def ElementwiseMappable : TraitList<[
     Scalarizable,
     Vectorizable,
     Tensorizable,
 ]>;
 
 // Op's regions have a single block.
 def SingleBlock : NativeOpTrait<"SingleBlock">, StructuralOpTrait;
 
+class SingleBlockImplicitTerminatorImpl<string op>
+    : ParamNativeOpTrait<"SingleBlockImplicitTerminator", op, [SingleBlock]>,
+      StructuralOpTrait;
+
 // Op's regions have a single block with the specified terminator.
 class SingleBlockImplicitTerminator<string op>
-    : ParamNativeOpTrait<"SingleBlockImplicitTerminator", op>,
-      StructuralOpTrait;
+    : TraitList<[SingleBlock, SingleBlockImplicitTerminatorImpl<op>]>;
 
 // Op's regions don't have terminator.
 def NoTerminator : NativeOpTrait<"NoTerminator">, StructuralOpTrait;
 
 // Op's parent operation is the provided one.
 class HasParent<string op>
     : ParamNativeOpTrait<"HasParent", op>, StructuralOpTrait;
 

mlir/include/mlir/IR/OpDefinition.h

 //===----------------------------------------------------------------------===//
 // SingleBlockImplicitTerminator
 
 /// This class provides APIs and verifiers for ops with regions having a single
 /// block that must terminate with `TerminatorOpType`.
 template <typename TerminatorOpType>
 struct SingleBlockImplicitTerminator {
   template <typename ConcreteType>
-  class Impl : public SingleBlock<ConcreteType> {
+  class Impl {
   private:
-    using Base = SingleBlock<ConcreteType>;
     /// Builds a terminator operation without relying on OpBuilder APIs to avoid
     /// cyclic header inclusion.
     static Operation *buildTerminator(OpBuilder &builder, Location loc) {
       OperationState state(loc, TerminatorOpType::getOperationName());
       TerminatorOpType::build(builder, state);
       return Operation::create(state);
     }
 
   public:
     /// The type of the operation used as the implicit terminator type.
     using ImplicitTerminatorOpT = TerminatorOpType;
 
     static LogicalResult verifyRegionTrait(Operation *op) {
-      if (failed(Base::verifyTrait(op)))
-        return failure();
       for (unsigned i = 0, e = op->getNumRegions(); i < e; ++i) {
         Region &region = op->getRegion(i);
         // Empty regions are fine.
         if (region.empty())
           continue;
         Operation &terminator = region.front().back();
         if (isa<TerminatorOpType>(terminator))
           continue;
static void ensureTerminator(Region &region, OpBuilder &builder,
                                  Location loc) {
       ::mlir::impl::ensureRegionTerminator(region, builder, loc,
                                            buildTerminator);
     }
 
     //===------------------------------------------------------------------===//
     // Single Region Utilities
     //===------------------------------------------------------------------===//
-    using Base::getBody;
 
     template <typename OpT, typename T = void>
     using enable_if_single_region =
         std::enable_if_t<OpT::template hasTrait<OneRegion>(), T>;
 
     /// Insert the operation into the back of the body, before the terminator.
     template <typename OpT = ConcreteType>
     enable_if_single_region<OpT> push_back(Operation *op) {
-      insert(Block::iterator(getBody()->getTerminator()), op);
+      Block *body = static_cast<SingleBlock<ConcreteType> *>(this)->getBody();
+      insert(Block::iterator(body->getTerminator()), op);
     }
 
     /// Insert the operation at the given insertion point. Note: The operation
     /// is never inserted after the terminator, even if the insertion point is
     /// end().
     template <typename OpT = ConcreteType>
     enable_if_single_region<OpT> insert(Operation *insertPt, Operation *op) {
       insert(Block::iterator(insertPt), op);
     }
     template <typename OpT = ConcreteType>
     enable_if_single_region<OpT> insert(Block::iterator insertPt,
                                         Operation *op) {
-      auto *body = getBody();
+      Block *body = static_cast<SingleBlock<ConcreteType> *>(this)->getBody();
       if (insertPt == body->end())
         insertPt = Block::iterator(body->getTerminator());
       body->getOperations().insert(insertPt, op);
     }
   };
 };
 
 /// Check is an op defines the `ImplicitTerminatorOpT` member. This is intended

mlir/test/mlir-tblgen/gen-dialect-doc.td

 // CHECK-NOT: [TOC]
 // CHECK: test.e
 // CHECK: Group of ops
 // CHECK: test.a
 // CHECK: test.d
 // CHECK: Other group
 // CHECK: test.b
 // CHECK: test.c
-// CHECK: Traits: SingleBlockImplicitTerminator<YieldOp>
+// CHECK: Traits: SingleBlock, SingleBlockImplicitTerminator<YieldOp>
 // CHECK: Interfaces: NoMemoryEffect (MemoryEffectOpInterface)
 // CHECK: Effects: MemoryEffects::Effect{}
 
 // CHECK: ## Attribute constraint definition
 // CHECK: ### attribute summary
 // CHECK: attribute description
 
 // CHECK: TestAttrDefAttr

mlir/tools/mlir-tblgen/OpFormatGen.cpp

struct OperationFormat {
   OperationFormat(const Operator &op)
       : useProperties(op.getDialect().usePropertiesForAttributes() &&
                       !op.getAttributes().empty()),
         opCppClassName(op.getCppClassName()) {
     operandTypes.resize(op.getNumOperands(), TypeResolution());
     resultTypes.resize(op.getNumResults(), TypeResolution());
 
     hasImplicitTermTrait = llvm::any_of(op.getTraits(), [](const Trait &trait) {
-      return trait.getDef().isSubClassOf("SingleBlockImplicitTerminator");
+      return trait.getDef().isSubClassOf("SingleBlockImplicitTerminatorImpl");
     });
 
-    hasSingleBlockTrait =
-        hasImplicitTermTrait || op.getTrait("::mlir::OpTrait::SingleBlock");
+    hasSingleBlockTrait = op.getTrait("::mlir::OpTrait::SingleBlock");
   }
 
   /// Generate the operation parser from this format.
   void genParser(Operator &op, OpClass &opClass);
   /// Generate the parser code for a specific format element.
   void genElementParser(FormatElement *element, MethodBody &body,
                         FmtContext &attrTypeCtx,
                         GenContext genCtx = GenContext::Normal);