[PATCH] D72584: [MLIR] Fix OpDefinition::classof check for shared libraries.

[Stephen Neuendorffer via Phabricator via llvm-commits]

stephenneuendorffer created this revision.
Herald added subscribers: llvm-commits, liufengdb, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, burmako, jpienaar, rriddle, mehdi_amini.
Herald added a project: LLVM.
stephenneuendorffer added a reviewer: rriddle.

When using MLIR in a shared library, it is possible for the shared library
to load a different, but equivalent version of a class method.  As a result,
function pointer comparison is not a reliable check of class equivalence.
This causes problems related to this code in OpDefinition.h:

  /// 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 op->getName().getStringRef() == ConcreteType::getOperationName();
  }

It appears that the intention is to implement the normal LLVM cast<>/isa<>
mechanism, but without a fixed enum of Kinds.  Primarily, the operation
name check allows casting based on the operation name, which are assumed
to be unique for an operation class.  (Aside: is this enforced anywhere?)
Before this, another, more efficient, check based on pointer equality of
the classof method.  Unfortunately, this pointer equality check fails
if/when the classof method is inlined into a shared object (e.g. libLLVM.so)
and also exists in the main executable, resulting in triggering the assert()
inside cast<>.

This patch fixes this method to fall back to the slow, but accurate string
comparison if the function pointer comparison fails.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72584

Files:
  mlir/include/mlir/IR/OpDefinition.h


Index: mlir/include/mlir/IR/OpDefinition.h
===================================================================
--- mlir/include/mlir/IR/OpDefinition.h
+++ mlir/include/mlir/IR/OpDefinition.h
@@ -21,6 +21,7 @@
 
 #include "mlir/IR/Operation.h"
 #include <type_traits>
+#include "llvm/Support/Debug.h"
 
 namespace mlir {
 class Builder;
@@ -979,8 +980,9 @@
 
   /// 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;
+    if (auto *abstractOp = op->getAbstractOperation()) {
+      if(&classof == abstractOp->classof) return true;
+    }
     return op->getName().getStringRef() == ConcreteType::getOperationName();
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72584.237582.patch
Type: text/x-patch
Size: 788 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200113/6597dd1c/attachment.bin>