[Mlir-commits] [mlir] 01700c4 - Store an Identifier instead of a StringRef for the OperationName inside an AbstractOperation (NFC)

[Mehdi Amini]

Author: Mehdi Amini
Date: 2020-09-02T19:10:56Z
New Revision: 01700c45eb22d848dd1dd980d7d46ae9aa034ade

URL: https://github.com/llvm/llvm-project/commit/01700c45eb22d848dd1dd980d7d46ae9aa034ade
DIFF: https://github.com/llvm/llvm-project/commit/01700c45eb22d848dd1dd980d7d46ae9aa034ade.diff

LOG: Store an Identifier instead of a StringRef for the OperationName inside an AbstractOperation (NFC)

Instead of storing a StringRef, we keep an Identifier which otherwise requires a lock on the context to retrieve.
This will allow to get an Identifier for any registered Operation for "free".

Reviewed By: rriddle

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

Added: 
    

Modified: 
    mlir/include/mlir/IR/Identifier.h
    mlir/include/mlir/IR/OperationSupport.h
    mlir/lib/IR/MLIRContext.cpp
    mlir/lib/IR/Operation.cpp
    mlir/lib/Parser/Parser.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/Identifier.h b/mlir/include/mlir/IR/Identifier.h
index ca1946bd8ad0..353dbc902087 100644
--- a/mlir/include/mlir/IR/Identifier.h
+++ b/mlir/include/mlir/IR/Identifier.h
@@ -67,6 +67,9 @@ class Identifier {
     return Identifier(static_cast<const EntryType *>(entry));
   }
 
+  /// Compare the underlying StringRef.
+  int compare(Identifier rhs) const { return strref().compare(rhs.strref()); }
+
 private:
   /// This contains the bytes of the string, which is guaranteed to be nul
   /// terminated.

diff  --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index b0e1205eefe6..7fce4b808d2e 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -82,7 +82,7 @@ class AbstractOperation {
   using OperationProperties = uint32_t;
 
   /// This is the name of the operation.
-  const StringRef name;
+  const Identifier name;
 
   /// This is the dialect that this operation belongs to.
   Dialect &dialect;
@@ -171,13 +171,7 @@ class AbstractOperation {
                                 SmallVectorImpl<OpFoldResult> &results),
       void (&getCanonicalizationPatterns)(OwningRewritePatternList &results,
                                           MLIRContext *context),
-      detail::InterfaceMap &&interfaceMap, bool (&hasTrait)(TypeID traitID))
-      : name(name), dialect(dialect), typeID(typeID),
-        parseAssembly(parseAssembly), printAssembly(printAssembly),
-        verifyInvariants(verifyInvariants), foldHook(foldHook),
-        getCanonicalizationPatterns(getCanonicalizationPatterns),
-        opProperties(opProperties), interfaceMap(std::move(interfaceMap)),
-        hasRawTrait(hasTrait) {}
+      detail::InterfaceMap &&interfaceMap, bool (&hasTrait)(TypeID traitID));
 
   /// The properties of the operation.
   const OperationProperties opProperties;
@@ -302,9 +296,12 @@ class OperationName {
   /// Return the operation name with dialect name stripped, if it has one.
   StringRef stripDialect() const;
 
-  /// Return the name of this operation.  This always succeeds.
+  /// Return the name of this operation. This always succeeds.
   StringRef getStringRef() const;
 
+  /// Return the name of this operation as an identifier. This always succeeds.
+  Identifier getIdentifier() const;
+
   /// If this operation has a registered operation description, return it.
   /// Otherwise return null.
   const AbstractOperation *getAbstractOperation() const;

diff  --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index 67658a9ca33a..a6246024a5ae 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -668,6 +668,25 @@ const AbstractOperation *AbstractOperation::lookup(StringRef opName,
   return nullptr;
 }
 
+AbstractOperation::AbstractOperation(
+    StringRef name, Dialect &dialect, OperationProperties opProperties,
+    TypeID typeID,
+    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),
+    detail::InterfaceMap &&interfaceMap, bool (&hasTrait)(TypeID traitID))
+    : name(Identifier::get(name, dialect.getContext())), dialect(dialect),
+      typeID(typeID), parseAssembly(parseAssembly),
+      printAssembly(printAssembly), verifyInvariants(verifyInvariants),
+      foldHook(foldHook),
+      getCanonicalizationPatterns(getCanonicalizationPatterns),
+      opProperties(opProperties), interfaceMap(std::move(interfaceMap)),
+      hasRawTrait(hasTrait) {}
+
 /// Get the dialect that registered the type with the provided typeid.
 const AbstractType &AbstractType::lookup(TypeID typeID, MLIRContext *context) {
   auto &impl = context->getImpl();

diff  --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index 67249b83b104..b8f9e6c9fdfc 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -45,11 +45,16 @@ StringRef OperationName::stripDialect() const {
   return splitName.second.empty() ? splitName.first : splitName.second;
 }
 
-/// Return the name of this operation.  This always succeeds.
+/// Return the name of this operation. This always succeeds.
 StringRef OperationName::getStringRef() const {
+  return getIdentifier().strref();
+}
+
+/// Return the name of this operation as an identifier. This always succeeds.
+Identifier OperationName::getIdentifier() const {
   if (auto *op = representation.dyn_cast<const AbstractOperation *>())
     return op->name;
-  return representation.get<Identifier>().strref();
+  return representation.get<Identifier>();
 }
 
 const AbstractOperation *OperationName::getAbstractOperation() const {

diff  --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp
index d6065f758fc1..48651a98561c 100644
--- a/mlir/lib/Parser/Parser.cpp
+++ b/mlir/lib/Parser/Parser.cpp
@@ -863,8 +863,8 @@ class CustomOpAsmParser : public OpAsmParser {
   /// Emit a diagnostic at the specified location and return failure.
   InFlightDiagnostic emitError(llvm::SMLoc loc, const Twine &message) override {
     emittedError = true;
-    return parser.emitError(loc, "custom op '" + opDefinition->name + "' " +
-                                     message);
+    return parser.emitError(loc, "custom op '" + opDefinition->name.strref() +
+                                     "' " + message);
   }
 
   llvm::SMLoc getCurrentLocation() override {