[mlir] Switch create to use NamedAttrList&&

[jpienaar]

Summary

Avoids needing the two parallel functions as NamedAttrList already takes care
of caching/passing through DictionaryAttr. Most cases were already supported
via implicit conversion, but add a few more cases for NamedAttrList that would
have required double implicit conversion.

[rriddle]

mlir/include/mlir/IR/OperationSupport.h

493–499

Let's please avoid this, can we just have a templated range method instead?

[jpienaar]

Use more generic templated constructor for NamedAttrList

mlir/include/mlir/IR/OperationSupport.h

493–499

Like so?

[rriddle]

mlir/include/mlir/IR/OperationSupport.h

493–499

Yep, thanks.

mlir/include/mlir/IR/Operation.h

 class alignas(8) Operation final
     : public llvm::ilist_node_with_parent<Operation, Block>,
       private llvm::TrailingObjects<Operation, detail::OperandStorage,
                                     BlockOperand, Region, OpOperand> {
 public:
   /// 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`.
   static Operation *create(const OperationState &state);
 
   /// 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 = {});
 
   /// The name of an operation is the key identifier for it.
   OperationName getName() { return name; }
 
   /// If this operation has a registered operation description, return it.
   /// Otherwise return None.

mlir/include/mlir/IR/OperationSupport.h

 public:
   using iterator = SmallVectorImpl<NamedAttribute>::iterator;
   using const_iterator = SmallVectorImpl<NamedAttribute>::const_iterator;
   using reference = NamedAttribute &;
   using const_reference = const NamedAttribute &;
   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);
   }
   bool operator==(const NamedAttrList &other) const {
     return attrs == other.attrs;
   }
 
   /// Add an attribute with the specified name.
   void append(StringRef name, Attribute attr);
 
   /// Add an attribute with the specified name.

mlir/lib/IR/Operation.cpp

 #include <numeric>
 
 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,
                 state.attributes.getDictionary(state.getContext()),
                 state.successors, state.regions);
 }
 
 /// 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]);
   return op;
 }
 
 /// Overload of create that takes an existing DictionaryAttr to avoid
 /// 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");
 
   // We only need to allocate additional memory for a subset of results.
   unsigned numTrailingResults = OpResult::getNumTrailing(resultTypes.size());
   unsigned numInlineResults = OpResult::getNumInline(resultTypes.size());
   unsigned numSuccessors = successors.size();
size_t byteSize =
           needsOperandStorage ? 1 : 0, numSuccessors, numRegions, numOperands);
   size_t prefixByteSize = llvm::alignTo(
       Operation::prefixAllocSize(numTrailingResults, numInlineResults),
       alignof(Operation));
   char *mallocMem = reinterpret_cast<char *>(malloc(byteSize + prefixByteSize));
   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");
 
   // Initialize the results.
   auto resultTypeIt = resultTypes.begin();
   for (unsigned i = 0; i < numInlineResults; ++i, ++resultTypeIt)
     new (op->getInlineOpResult(i)) detail::InlineOpResult(*resultTypeIt, i);