[mlir] Replace AbstractOperation::classof with a ClassID instance.

[rriddle]

Summary

This field is currently not used by anything, and using a ClassID instance provides better support for more efficient classof.

[merge_guards_bot]

Unit tests: pass. 61895 tests passed, 0 failed and 782 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[stephenneuendorffer]

Thanks for this, it looks like a good solution to the problems I've seen, although I haven't tested it.

[vchuravy]

I built a shared library (D73130) and run into the case that my HasParent<FuncOp> trait failed to verify.

(gdb) 
mlir::Op<mlir::FuncOp, mlir::OpTrait::ZeroOperands, mlir::OpTrait::ZeroResult, mlir::OpTrait::IsIsolatedFromAbove, mlir::OpTrait::Symbol, mlir::OpTrait::FunctionLike, mlir::CallableOpInterface::Trait>::classof (op=0x555555e67550) at /data/vchuravy/mlir/build/usr/include/mlir/IR/OpDefinition.h:987
987           return ClassID::getID<ConcreteType>() == abstractOp->classID;

(gdb) p abstractOp->classID
$7 = (mlir::ClassID *) 0x7ffff698e020 <mlir::ClassID* mlir::ClassID::getID<mlir::FuncOp>()::id>

(gdb) p ClassID::getID<mlir::FuncOp>()
$8 = (mlir::ClassID *) 0x7ffff7aee120 <mlir::ClassID* mlir::ClassID::getID<mlir::FuncOp>()::id>

So it seems it still might be the case that pointer can differ. D72584 fixed this by falling back more often to the string comparison.

[rriddle]

So it seems it still might be the case that pointer can differ. D72584 fixed this by falling back more often to the string comparison.

You are going to run into a much bigger problem given that this depends on ClassOf, which is a static typeid that is used pervasively across the entire code base. Making that work with shared libraries is likely the most important(difficult?) task item.

[mehdi_amini]

In D72822#1845630, @vchuravy wrote:

I built a shared library (D73130) and run into the case that my HasParent<FuncOp> trait failed to verify.

Can you provide reproduction instruction (and platform)?

mlir/include/mlir/IR/OpDefinition.h

public:
   Dialect *getDialect() { return getOperation()->getDialect(); }
 
   /// Return the parent Region of this operation.
   Region *getParentRegion() { return getOperation()->getParentRegion(); }
 
   /// Return true if this "op class" can match against the specified operation.
   static bool classof(Operation *op) {
     if (auto *abstractOp = op->getAbstractOperation())
-      return &classof == abstractOp->classof;
+      return ClassID::getID<ConcreteType>() == abstractOp->classID;
     return op->getName().getStringRef() == ConcreteType::getOperationName();
   }
 
   /// This is the hook used by the AsmParser to parse the custom form of this
   /// op from an .mlir file.  Op implementations should provide a parse method,
   /// which returns failure.  On success, they should return fill in result with
   /// the fields to use.
   static ParseResult parseAssembly(OpAsmParser &parser,

mlir/include/mlir/IR/OperationSupport.h

public:
   using OperationProperties = uint32_t;
 
   /// This is the name of the operation.
   const StringRef name;
 
   /// This is the dialect that this operation belongs to.
   Dialect &dialect;
 
-  /// Return true if this "op class" can match against the specified operation.
-  bool (&classof)(Operation *op);
+  /// The unique identifier of the derived Op class.
+  ClassID *classID;
 
   /// Use the specified object to parse this ops custom assembly format.
   ParseResult (&parseAssembly)(OpAsmParser &parser, OperationState &result);
 
   /// This hook implements the AsmPrinter for this operation.
   void (&printAssembly)(Operation *op, OpAsmPrinter &p);
 
   /// This hook implements the verifier for this operation.  It should emits an
public:
   /// pointer to it if present.  Otherwise, return a null pointer.
   static const AbstractOperation *lookup(StringRef opName,
                                          MLIRContext *context);
 
   /// This constructor is used by Dialect objects when they register the list of
   /// operations they contain.
   template <typename T> static AbstractOperation get(Dialect &dialect) {
     return AbstractOperation(
-        T::getOperationName(), dialect, T::getOperationProperties(), T::classof,
-        T::parseAssembly, T::printAssembly, T::verifyInvariants, T::foldHook,
-        T::getCanonicalizationPatterns, T::getRawInterface, T::hasTrait);
+        T::getOperationName(), dialect, T::getOperationProperties(),
+        ClassID::getID<T>(), T::parseAssembly, T::printAssembly,
+        T::verifyInvariants, T::foldHook, T::getCanonicalizationPatterns,
+        T::getRawInterface, T::hasTrait);
   }
 
 private:
   AbstractOperation(
       StringRef name, Dialect &dialect, OperationProperties opProperties,
-      bool (&classof)(Operation *op),
+      ClassID *classID,
       ParseResult (&parseAssembly)(OpAsmParser &parser, OperationState &result),
       void (&printAssembly)(Operation *op, OpAsmPrinter &p),
       LogicalResult (&verifyInvariants)(Operation *op),
       LogicalResult (&foldHook)(Operation *op, ArrayRef<Attribute> operands,
                                 SmallVectorImpl<OpFoldResult> &results),
       void (&getCanonicalizationPatterns)(OwningRewritePatternList &results,
                                           MLIRContext *context),
       void *(&getRawInterface)(ClassID *interfaceID),
       bool (&hasTrait)(ClassID *traitID))
-      : name(name), dialect(dialect), classof(classof),
+      : name(name), dialect(dialect), classID(classID),
         parseAssembly(parseAssembly), printAssembly(printAssembly),
         verifyInvariants(verifyInvariants), foldHook(foldHook),
         getCanonicalizationPatterns(getCanonicalizationPatterns),
         opProperties(opProperties), getRawInterface(getRawInterface),
         hasRawTrait(hasTrait) {}
 
   /// The properties of the operation.
   const OperationProperties opProperties;