[mlir] Switch create to use NamedAttrList&&

Avoids needing the two parallel functions as NamedAttrList already takes care of caching DictionaryAttr and implicitly can convert from either. Differential Revision: https://reviews.llvm.org/D129527
llvm · Jul 12, 2022 · 0db084d · 0db084d
1 parent 2a0aa98
commit 0db084d
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 26 deletions.
diff --git a/mlir/include/mlir/IR/Operation.h b/mlir/include/mlir/IR/Operation.h
@@ -33,14 +33,7 @@ class alignas(8) Operation final
   /// Create a new Operation with the specific fields.
   static Operation *create(Location location, OperationName name,
                            TypeRange resultTypes, ValueRange operands,
-                           ArrayRef<NamedAttribute> attributes,
-                           BlockRange successors, unsigned numRegions);
-
-  /// Overload of create that takes an existing DictionaryAttr to avoid
-  /// unnecessarily uniquing a list of attributes.
-  static Operation *create(Location location, OperationName name,
-                           TypeRange resultTypes, ValueRange operands,
-                           DictionaryAttr attributes, BlockRange successors,
+                           NamedAttrList &&attributes, BlockRange successors,
                            unsigned numRegions);
 
   /// Create a new Operation from the fields stored in `state`.
@@ -49,7 +42,7 @@ class alignas(8) Operation final
   /// Create a new Operation with the specific fields.
   static Operation *create(Location location, OperationName name,
                            TypeRange resultTypes, ValueRange operands,
-                           DictionaryAttr attributes,
+                           NamedAttrList &&attributes,
                            BlockRange successors = {},
                            RegionRange regions = {});
 

diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
@@ -485,10 +485,15 @@ class NamedAttrList {
   using size_type = size_t;
 
   NamedAttrList() : dictionarySorted({}, true) {}
+  NamedAttrList(llvm::NoneType none) : NamedAttrList() {}
   NamedAttrList(ArrayRef<NamedAttribute> attributes);
   NamedAttrList(DictionaryAttr attributes);
   NamedAttrList(const_iterator inStart, const_iterator inEnd);
 
+  template <typename Container>
+  NamedAttrList(const Container &vec)
+      : NamedAttrList(ArrayRef<NamedAttribute>(vec)) {}
+
   bool operator!=(const NamedAttrList &other) const {
     return !(*this == other);
   }

diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
@@ -23,16 +23,6 @@ using namespace mlir;
 // Operation
 //===----------------------------------------------------------------------===//
 
-/// Create a new Operation with the specific fields.
-Operation *Operation::create(Location location, OperationName name,
-                             TypeRange resultTypes, ValueRange operands,
-                             ArrayRef<NamedAttribute> attributes,
-                             BlockRange successors, unsigned numRegions) {
-  return create(location, name, resultTypes, operands,
-                DictionaryAttr::get(location.getContext(), attributes),
-                successors, numRegions);
-}
-
 /// Create a new Operation from operation state.
 Operation *Operation::create(const OperationState &state) {
   return create(state.location, state.name, state.types, state.operands,
@@ -43,11 +33,11 @@ Operation *Operation::create(const OperationState &state) {
 /// Create a new Operation with the specific fields.
 Operation *Operation::create(Location location, OperationName name,
                              TypeRange resultTypes, ValueRange operands,
-                             DictionaryAttr attributes, BlockRange successors,
+                             NamedAttrList &&attributes, BlockRange successors,
                              RegionRange regions) {
   unsigned numRegions = regions.size();
-  Operation *op = create(location, name, resultTypes, operands, attributes,
-                         successors, numRegions);
+  Operation *op = create(location, name, resultTypes, operands,
+                         std::move(attributes), successors, numRegions);
   for (unsigned i = 0; i < numRegions; ++i)
     if (regions[i])
       op->getRegion(i).takeBody(*regions[i]);
@@ -58,7 +48,7 @@ Operation *Operation::create(Location location, OperationName name,
 /// unnecessarily uniquing a list of attributes.
 Operation *Operation::create(Location location, OperationName name,
                              TypeRange resultTypes, ValueRange operands,
-                             DictionaryAttr attributes, BlockRange successors,
+                             NamedAttrList &&attributes, BlockRange successors,
                              unsigned numRegions) {
   assert(llvm::all_of(resultTypes, [](Type t) { return t; }) &&
          "unexpected null result type");
@@ -88,9 +78,9 @@ Operation *Operation::create(Location location, OperationName name,
   void *rawMem = mallocMem + prefixByteSize;
 
   // Create the new Operation.
-  Operation *op =
-      ::new (rawMem) Operation(location, name, numResults, numSuccessors,
-                               numRegions, attributes, needsOperandStorage);
+  Operation *op = ::new (rawMem) Operation(
+      location, name, numResults, numSuccessors, numRegions,
+      attributes.getDictionary(location.getContext()), needsOperandStorage);
 
   assert((numSuccessors == 0 || op->mightHaveTrait<OpTrait::IsTerminator>()) &&
          "unexpected successors in a non-terminator operation");