[mlir] Remove the MutableDictionaryAttr class

[rriddle]

Summary

This class used to serve a few useful purposes:

• Allowed containing a null DictionaryAttr
• Provided some simple mutable API around a DictionaryAttr

The first of which is no longer an issue now that there is much better caching support for attributes in general, and a cache in the context for empty dictionaries. The second results in more trouble than it's worth because it mutates the internal dictionary on every action, leading to a potentially large number of dictionary copies. NamedAttrList is a much better alternative for the second use case, and should be modified as needed to better fit it's usage as a DictionaryAttrBuilder.

[jpienaar]

Thanks!

mlir/include/mlir/IR/FunctionSupport.h

304

Is result a nullptr or just empty/not modified?

372

Same as above

flang/lib/Optimizer/Dialect/FIROps.cpp

return allArgs.slice(start, (*(ranges.begin() + pos)).getZExtValue(),
                        std::forward<AdditionalArgs>(additionalArgs)...);
 }
 
 static mlir::MutableOperandRange
 getMutableSuccessorOperands(unsigned pos, mlir::MutableOperandRange operands,
                             StringRef offsetAttr) {
   Operation *owner = operands.getOwner();
   NamedAttribute targetOffsetAttr =
-      *owner->getMutableAttrDict().getNamed(offsetAttr);
+      *owner->getAttrDictionary().getNamed(offsetAttr);
   return getSubOperands(
       pos, operands, targetOffsetAttr.second.cast<DenseIntElementsAttr>(),
       mlir::MutableOperandRange::OperandSegment(pos, targetOffsetAttr));
 }
 
 static unsigned denseElementsSize(mlir::DenseIntElementsAttr attr) {
   return attr.getNumElements();
 }

mlir/docs/OpDefinitions.md

 *   Type Directives
     -   Single: `Type`
     -   Optional: `Type`
     -   Variadic: `TypeRange`
 *   TypeRef Directives
     -   Single: `Type`
     -   Optional: `Type`
     -   Variadic: `TypeRange`
-*   `attr-dict` Directive: `const MutableDictionaryAttr&`
+*   `attr-dict` Directive: `DictionaryAttr`
 
 When a variable is optional, the provided value may be null.
 
 #### Optional Groups
 
 In certain situations operations may have "optional" information, e.g.
 attributes or an empty set of variadic operands. In these situations a section
 of the assembly format can be marked as `optional` based on the presence of this

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

def LLVM_LLVMFuncOp : LLVM_Op<"func",
   let regions = (region AnyRegion:$body);
 
   let skipDefaultBuilders = 1;
 
   let builders = [
     OpBuilderDAG<(ins "StringRef":$name, "LLVMType":$type,
       CArg<"Linkage", "Linkage::External">:$linkage,
       CArg<"ArrayRef<NamedAttribute>", "{}">:$attrs,
-      CArg<"ArrayRef<MutableDictionaryAttr>", "{}">:$argAttrs)>
+      CArg<"ArrayRef<DictionaryAttr>", "{}">:$argAttrs)>
   ];
 
   let extraClassDeclaration = [{
     // Add an entry block to an empty function, and set up the block arguments
     // to match the signature of the function.
     Block *addEntryBlock();
 
     LLVMType getType() {

mlir/include/mlir/IR/BuiltinAttributes.h

auto ElementsAttr::getValues() const -> iterator_range<T> {
   if (SparseElementsAttr sparseAttr = dyn_cast<SparseElementsAttr>()) {
     auto values = sparseAttr.getValues<T>();
     return {iterator<T>(*this, values.begin()),
             iterator<T>(*this, values.end())};
   }
   llvm_unreachable("unexpected attribute kind");
 }
 
-//===----------------------------------------------------------------------===//
-// MutableDictionaryAttr
-//===----------------------------------------------------------------------===//
-
-/// A MutableDictionaryAttr is a mutable wrapper around a DictionaryAttr. It
-/// provides additional interfaces for adding, removing, replacing attributes
-/// within a DictionaryAttr.
-///
-/// We assume there will be relatively few attributes on a given operation
-/// (maybe a dozen or so, but not hundreds or thousands) so we use linear
-/// searches for everything.
-class MutableDictionaryAttr {
-public:
-  MutableDictionaryAttr(DictionaryAttr attrs = nullptr)
-      : attrs((attrs && !attrs.empty()) ? attrs : nullptr) {}
-  MutableDictionaryAttr(ArrayRef<NamedAttribute> attributes);
-
-  bool operator!=(const MutableDictionaryAttr &other) const {
-    return !(*this == other);
-  }
-  bool operator==(const MutableDictionaryAttr &other) const {
-    return attrs == other.attrs;
-  }
-
-  /// Return the underlying dictionary attribute.
-  DictionaryAttr getDictionary(MLIRContext *context) const;
-
-  /// Return the underlying dictionary attribute or null if there are no
-  /// attributes within this dictionary.
-  DictionaryAttr getDictionaryOrNull() const { return attrs; }
-
-  /// Return all of the attributes on this operation.
-  ArrayRef<NamedAttribute> getAttrs() const;
-
-  /// Replace the held attributes with ones provided in 'newAttrs'.
-  void setAttrs(ArrayRef<NamedAttribute> attributes);
-
-  /// Return the specified attribute if present, null otherwise.
-  Attribute get(StringRef name) const;
-  Attribute get(Identifier name) const;
-
-  /// Return the specified named attribute if present, None otherwise.
-  Optional<NamedAttribute> getNamed(StringRef name) const;
-  Optional<NamedAttribute> getNamed(Identifier name) const;
-
-  /// If the an attribute exists with the specified name, change it to the new
-  /// value.  Otherwise, add a new attribute with the specified name/value.
-  void set(Identifier name, Attribute value);
-
-  enum class RemoveResult { Removed, NotFound };
-
-  /// Remove the attribute with the specified name if it exists.  The return
-  /// value indicates whether the attribute was present or not.
-  RemoveResult remove(Identifier name);
-
-  bool empty() const { return attrs == nullptr; }
-
-private:
-  friend ::llvm::hash_code hash_value(const MutableDictionaryAttr &arg);
-
-  DictionaryAttr attrs;
-};
-
-inline ::llvm::hash_code hash_value(const MutableDictionaryAttr &arg) {
-  if (!arg.attrs)
-    return ::llvm::hash_value((void *)nullptr);
-  return hash_value(arg.attrs);
-}
-
 } // end namespace mlir.
 
 namespace llvm {
 
 template <>
 struct PointerLikeTypeTraits<mlir::SymbolRefAttr>
     : public PointerLikeTypeTraits<mlir::Attribute> {
   static inline mlir::SymbolRefAttr getFromVoidPointer(void *ptr) {
     return PointerLikeTypeTraits<mlir::Attribute>::getFromVoidPointer(ptr)
         .cast<mlir::SymbolRefAttr>();
   }
 };
 
 } // namespace llvm
 
 #endif // MLIR_IR_BUILTINATTRIBUTES_H

mlir/include/mlir/IR/BuiltinOps.td

]> {
   let arguments = (ins SymbolNameAttr:$sym_name,
                        TypeAttr:$type,
                        OptionalAttr<StrAttr>:$sym_visibility);
   let regions = (region AnyRegion:$body);
 
   let builders = [OpBuilderDAG<(ins
     "StringRef":$name, "FunctionType":$type,
     CArg<"ArrayRef<NamedAttribute>", "{}">:$attrs,
-    CArg<"ArrayRef<MutableDictionaryAttr>", "{}">:$argAttrs)
+    CArg<"ArrayRef<DictionaryAttr>", "{}">:$argAttrs)
   >];
   let extraClassDeclaration = [{
     static FuncOp create(Location location, StringRef name, FunctionType type,
                          ArrayRef<NamedAttribute> attrs = {});
     static FuncOp create(Location location, StringRef name, FunctionType type,
                          iterator_range<dialect_attr_iterator> attrs);
     static FuncOp create(Location location, StringRef name, FunctionType type,
                          ArrayRef<NamedAttribute> attrs,
-                         ArrayRef<MutableDictionaryAttr> argAttrs);
+                         ArrayRef<DictionaryAttr> argAttrs);
 
     /// Create a deep copy of this function and all of its blocks, remapping any
     /// operands that use values outside of the function using the map that is
     /// provided (leaving them alone if no entry is present). If the mapper
     /// contains entries for function arguments, these arguments are not
     /// included in the new function. Replaces references to cloned sub-values
     /// with the corresponding value that is copied, and adds those mappings to
     /// the mapper.

mlir/include/mlir/IR/FunctionSupport.h

public:
   /// argument attribute dictionaries are optional, and will generally only
   /// exist if they are non-empty.
 
   /// Return all of the attributes for the argument at 'index'.
   ArrayRef<NamedAttribute> getArgAttrs(unsigned index) {
     return ::mlir::impl::getArgAttrs(this->getOperation(), index);
   }
 
-  /// Return all argument attributes of this function.
-  void getAllArgAttrs(SmallVectorImpl<MutableDictionaryAttr> &result) {
+  /// Return all argument attributes of this function. If an argument does not
+  /// have any attributes, the corresponding entry in `result` is nullptr.
+  void getAllArgAttrs(SmallVectorImpl<DictionaryAttr> &result) {
     for (unsigned i = 0, e = getNumArguments(); i != e; ++i)
       result.emplace_back(getArgAttrDict(i));
   }
 
   /// Return the specified attribute, if present, for the argument at 'index',
   /// null otherwise.
   Attribute getArgAttr(unsigned index, Identifier name) {
     auto argDict = getArgAttrDict(index);
public:
   }
   template <typename AttrClass>
   AttrClass getArgAttrOfType(unsigned index, StringRef name) {
     return getArgAttr(index, name).template dyn_cast_or_null<AttrClass>();
   }
 
   /// Set the attributes held by the argument at 'index'.
   void setArgAttrs(unsigned index, ArrayRef<NamedAttribute> attributes);
-  void setArgAttrs(unsigned index, MutableDictionaryAttr attributes);
-  void setAllArgAttrs(ArrayRef<MutableDictionaryAttr> attributes) {
+
+  /// Set the attributes held by the argument at 'index'. `attributes` may be
+  /// null, in which case any existing argument attributes are removed.
+  void setArgAttrs(unsigned index, DictionaryAttr attributes);
+  void setAllArgAttrs(ArrayRef<DictionaryAttr> attributes) {
     assert(attributes.size() == getNumArguments());
     for (unsigned i = 0, e = attributes.size(); i != e; ++i)
       setArgAttrs(i, attributes[i]);
   }
 
   /// If the an attribute exists with the specified name, change it to the new
   /// value. Otherwise, add a new attribute with the specified name/value.
   void setArgAttr(unsigned index, Identifier name, Attribute value);
   void setArgAttr(unsigned index, StringRef name, Attribute value) {
     setArgAttr(index, Identifier::get(name, this->getOperation()->getContext()),
                value);
   }
 
-  /// Remove the attribute 'name' from the argument at 'index'.
-  MutableDictionaryAttr::RemoveResult removeArgAttr(unsigned index,
-                                                    Identifier name);
+  /// Remove the attribute 'name' from the argument at 'index'. Return the
+  /// attribute that was erased, or nullptr if there was no attribute with such
+  /// name.
+  Attribute removeArgAttr(unsigned index, Identifier name);
 
   //===--------------------------------------------------------------------===//
   // Result Attributes
   //===--------------------------------------------------------------------===//
 
   /// FunctionLike operations allow for attaching attributes to each of the
   /// respective function results. These result attributes are stored as
   /// DictionaryAttrs in the main operation attribute dictionary. The name of
   /// these entries is `result` followed by the index of the result. These
   /// result attribute dictionaries are optional, and will generally only
   /// exist if they are non-empty.
 
   /// Return all of the attributes for the result at 'index'.
   ArrayRef<NamedAttribute> getResultAttrs(unsigned index) {
     return ::mlir::impl::getResultAttrs(this->getOperation(), index);
   }
 
-  /// Return all result attributes of this function.
-  void getAllResultAttrs(SmallVectorImpl<MutableDictionaryAttr> &result) {
+  /// Return all result attributes of this function. If a result does not have
+  /// any attributes, the corresponding entry in `result` is nullptr.
+  void getAllResultAttrs(SmallVectorImpl<DictionaryAttr> &result) {
     for (unsigned i = 0, e = getNumResults(); i != e; ++i)
       result.emplace_back(getResultAttrDict(i));
   }
 
   /// Return the specified attribute, if present, for the result at 'index',
   /// null otherwise.
   Attribute getResultAttr(unsigned index, Identifier name) {
     auto argDict = getResultAttrDict(index);
public:
   }
   template <typename AttrClass>
   AttrClass getResultAttrOfType(unsigned index, StringRef name) {
     return getResultAttr(index, name).template dyn_cast_or_null<AttrClass>();
   }
 
   /// Set the attributes held by the result at 'index'.
   void setResultAttrs(unsigned index, ArrayRef<NamedAttribute> attributes);
-  void setResultAttrs(unsigned index, MutableDictionaryAttr attributes);
-  void setAllResultAttrs(ArrayRef<MutableDictionaryAttr> attributes) {
+  /// Set the attributes held by the result at 'index'. `attributes` may be
+  /// null, in which case any existing argument attributes are removed.
+  void setResultAttrs(unsigned index, DictionaryAttr attributes);
+  void setAllResultAttrs(ArrayRef<DictionaryAttr> attributes) {
     assert(attributes.size() == getNumResults());
     for (unsigned i = 0, e = attributes.size(); i != e; ++i)
       setResultAttrs(i, attributes[i]);
   }
 
   /// If the an attribute exists with the specified name, change it to the new
   /// value. Otherwise, add a new attribute with the specified name/value.
   void setResultAttr(unsigned index, Identifier name, Attribute value);
   void setResultAttr(unsigned index, StringRef name, Attribute value) {
     setResultAttr(index,
                   Identifier::get(name, this->getOperation()->getContext()),
                   value);
   }
 
-  /// Remove the attribute 'name' from the result at 'index'.
-  MutableDictionaryAttr::RemoveResult removeResultAttr(unsigned index,
-                                                       Identifier name);
+  /// Remove the attribute 'name' from the result at 'index'. Return the
+  /// attribute that was erased, or nullptr if there was no attribute with such
+  /// name.
+  Attribute removeResultAttr(unsigned index, Identifier name);
 
 protected:
   /// Returns the attribute entry name for the set of argument attributes at
   /// 'index'.
   static StringRef getArgAttrName(unsigned index, SmallVectorImpl<char> &out) {
     return ::mlir::impl::getArgAttrName(index, out);
   }
 
void FunctionLike<ConcreteType>::setArgAttrs(
   if (attributes.empty())
     return (void)static_cast<ConcreteType *>(this)->removeAttr(nameOut);
   Operation *op = this->getOperation();
   op->setAttr(nameOut, DictionaryAttr::get(attributes, op->getContext()));
 }
 
 template <typename ConcreteType>
 void FunctionLike<ConcreteType>::setArgAttrs(unsigned index,
-                                             MutableDictionaryAttr attributes) {
+                                             DictionaryAttr attributes) {
   assert(index < getNumArguments() && "invalid argument number");
   SmallString<8> nameOut;
-  if (attributes.getAttrs().empty()) {
+  if (!attributes || attributes.empty())
     this->getOperation()->removeAttr(getArgAttrName(index, nameOut));
-  } else {
-    auto newAttr = attributes.getDictionary(
-        attributes.getAttrs().front().second.getContext());
+  else
     return this->getOperation()->setAttr(getArgAttrName(index, nameOut),
-                                         newAttr);
-  }
+                                         attributes);
 }
 
 /// If the an attribute exists with the specified name, change it to the new
 /// value. Otherwise, add a new attribute with the specified name/value.
 template <typename ConcreteType>
 void FunctionLike<ConcreteType>::setArgAttr(unsigned index, Identifier name,
                                             Attribute value) {
-  auto curAttr = getArgAttrDict(index);
-  MutableDictionaryAttr attrDict(curAttr);
-  attrDict.set(name, value);
+  NamedAttrList attributes(getArgAttrDict(index));
+  Attribute oldValue = attributes.set(name, value);
 
   // If the attribute changed, then set the new arg attribute list.
-  if (curAttr != attrDict.getDictionary(value.getContext()))
-    setArgAttrs(index, attrDict);
+  if (value != oldValue)
+    setArgAttrs(index, attributes.getDictionary(value.getContext()));
 }
 
 /// Remove the attribute 'name' from the argument at 'index'.
 template <typename ConcreteType>
-MutableDictionaryAttr::RemoveResult
-FunctionLike<ConcreteType>::removeArgAttr(unsigned index, Identifier name) {
+Attribute FunctionLike<ConcreteType>::removeArgAttr(unsigned index,
+                                                    Identifier name) {
   // Build an attribute list and remove the attribute at 'name'.
-  MutableDictionaryAttr attrDict(getArgAttrDict(index));
-  auto result = attrDict.remove(name);
+  NamedAttrList attributes(getArgAttrDict(index));
+  Attribute removedAttr = attributes.erase(name);
 
   // If the attribute was removed, then update the argument dictionary.
-  if (result == MutableDictionaryAttr::RemoveResult::Removed)
-    setArgAttrs(index, attrDict);
-  return result;
+  if (removedAttr)
+    setArgAttrs(index, attributes.getDictionary(removedAttr.getContext()));
+  return removedAttr;
 }
 
 //===----------------------------------------------------------------------===//
 // Function Result Attribute.
 //===----------------------------------------------------------------------===//
 
 /// Set the attributes held by the result at 'index'.
 template <typename ConcreteType>
 void FunctionLike<ConcreteType>::setResultAttrs(
     unsigned index, ArrayRef<NamedAttribute> attributes) {
   assert(index < getNumResults() && "invalid result number");
   SmallString<8> nameOut;
   getResultAttrName(index, nameOut);
 
   if (attributes.empty())
     return (void)this->getOperation()->removeAttr(nameOut);
   Operation *op = this->getOperation();
   op->setAttr(nameOut, DictionaryAttr::get(attributes, op->getContext()));
 }
 
 template <typename ConcreteType>
-void FunctionLike<ConcreteType>::setResultAttrs(
-    unsigned index, MutableDictionaryAttr attributes) {
+void FunctionLike<ConcreteType>::setResultAttrs(unsigned index,
+                                                DictionaryAttr attributes) {
   assert(index < getNumResults() && "invalid result number");
   SmallString<8> nameOut;
-  if (attributes.empty()) {
+  if (!attributes || attributes.empty())
     this->getOperation()->removeAttr(getResultAttrName(index, nameOut));
-  } else {
-    auto newAttr = attributes.getDictionary(this->getOperation()->getContext());
-    return this->getOperation()->setAttr(getResultAttrName(index, nameOut),
-                                         newAttr);
-  }
+  else
+    this->getOperation()->setAttr(getResultAttrName(index, nameOut),
+                                  attributes);
 }
 
 /// If the an attribute exists with the specified name, change it to the new
 /// value. Otherwise, add a new attribute with the specified name/value.
 template <typename ConcreteType>
 void FunctionLike<ConcreteType>::setResultAttr(unsigned index, Identifier name,
                                                Attribute value) {
-  auto curAttr = getResultAttrDict(index);
-  MutableDictionaryAttr attrDict(curAttr);
-  attrDict.set(name, value);
+  NamedAttrList attributes(getResultAttrDict(index));
+  Attribute oldAttr = attributes.set(name, value);
 
   // If the attribute changed, then set the new arg attribute list.
-  if (curAttr != attrDict.getDictionary(value.getContext()))
-    setResultAttrs(index, attrDict);
+  if (oldAttr != value)
+    setResultAttrs(index, attributes.getDictionary(value.getContext()));
 }
 
 /// Remove the attribute 'name' from the result at 'index'.
 template <typename ConcreteType>
-MutableDictionaryAttr::RemoveResult
-FunctionLike<ConcreteType>::removeResultAttr(unsigned index, Identifier name) {
+Attribute FunctionLike<ConcreteType>::removeResultAttr(unsigned index,
+                                                       Identifier name) {
   // Build an attribute list and remove the attribute at 'name'.
-  MutableDictionaryAttr attrDict(getResultAttrDict(index));
-  auto result = attrDict.remove(name);
+  NamedAttrList attributes(getResultAttrDict(index));
+  Attribute removedAttr = attributes.erase(name);
 
   // If the attribute was removed, then update the result dictionary.
-  if (result == MutableDictionaryAttr::RemoveResult::Removed)
-    setResultAttrs(index, attrDict);
-  return result;
+  if (removedAttr)
+    setResultAttrs(index, attributes.getDictionary(removedAttr.getContext()));
+  return removedAttr;
 }
 
 } // end namespace OpTrait
 
 } // end namespace mlir
 
 #endif // MLIR_IR_FUNCTIONSUPPORT_H

mlir/include/mlir/IR/OpDefinition.h

void setAttr(Identifier name, Attribute value) {
     state->setAttr(name, value);
   }
   void setAttr(StringRef name, Attribute value) {
     setAttr(Identifier::get(name, getContext()), value);
   }
 
   /// Set the attributes held by this operation.
   void setAttrs(ArrayRef<NamedAttribute> attributes) {
-    state->setAttrs(attributes);
+    state->setAttrs(DictionaryAttr::get(attributes, getContext()));
   }
-  void setAttrs(MutableDictionaryAttr newAttrs) { state->setAttrs(newAttrs); }
+  void setAttrs(DictionaryAttr newAttrs) { state->setAttrs(newAttrs); }
 
   /// Set the dialect attributes for this operation, and preserve all dependent.
   template <typename DialectAttrs> void setDialectAttrs(DialectAttrs &&attrs) {
-    state->setDialectAttrs(std::move(attrs));
+    state->setDialectAttrs(std::forward<DialectAttrs>(attrs));
   }
 
-  /// Remove the attribute with the specified name if it exists.  The return
-  /// value indicates whether the attribute was present or not.
-  MutableDictionaryAttr::RemoveResult removeAttr(Identifier name) {
-    return state->removeAttr(name);
-  }
-  MutableDictionaryAttr::RemoveResult removeAttr(StringRef name) {
+  /// Remove the attribute with the specified name if it exists. Return the
+  /// attribute that was erased, or nullptr if there was no attribute with such
+  /// name.
+  Attribute removeAttr(Identifier name) { return state->removeAttr(name); }
+  Attribute removeAttr(StringRef name) {
     return state->removeAttr(Identifier::get(name, getContext()));
   }
 
   /// Return true if there are no users of any results of this operation.
   bool use_empty() { return state->use_empty(); }
 
   /// Remove this operation from its parent block and delete it.
   void erase() { state->erase(); }

mlir/include/mlir/IR/Operation.h

: public llvm::ilist_node_with_parent<Operation, Block>,
                                     detail::OperandStorage> {
 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 MutableDictionaryAttr to avoid
+  /// 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,
-                           MutableDictionaryAttr attributes,
-                           BlockRange successors, unsigned numRegions);
+                           DictionaryAttr 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,
-                           MutableDictionaryAttr attributes,
+                           DictionaryAttr 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 null.
public:
   // Attributes
   //===--------------------------------------------------------------------===//
 
   // Operations may optionally carry a list of attributes that associate
   // constants to names.  Attributes may be dynamically added and removed over
   // the lifetime of an operation.
 
   /// Return all of the attributes on this operation.
-  ArrayRef<NamedAttribute> getAttrs() { return attrs.getAttrs(); }
+  ArrayRef<NamedAttribute> getAttrs() { return attrs.getValue(); }
 
   /// Return all of the attributes on this operation as a DictionaryAttr.
-  DictionaryAttr getAttrDictionary() {
-    return attrs.getDictionary(getContext());
-  }
-
-  /// Return mutable container of all the attributes on this operation.
-  MutableDictionaryAttr &getMutableAttrDict() { return attrs; }
+  DictionaryAttr getAttrDictionary() { return attrs; }
 
   /// Set the attribute dictionary on this operation.
-  /// Using a MutableDictionaryAttr is more efficient as it does not require new
-  /// uniquing in the MLIRContext.
-  void setAttrs(MutableDictionaryAttr newAttrs) { attrs = newAttrs; }
-  void setAttrs(ArrayRef<NamedAttribute> newAttrs) { attrs = newAttrs; }
+  void setAttrs(DictionaryAttr newAttrs) {
+    assert(newAttrs && "expected valid attribute dictionary");
+    attrs = newAttrs;
+  }
+  void setAttrs(ArrayRef<NamedAttribute> newAttrs) {
+    setAttrs(DictionaryAttr::get(newAttrs, getContext()));
+  }
 
   /// Return the specified attribute if present, null otherwise.
   Attribute getAttr(Identifier name) { return attrs.get(name); }
   Attribute getAttr(StringRef name) { return attrs.get(name); }
 
   template <typename AttrClass> AttrClass getAttrOfType(Identifier name) {
     return getAttr(name).dyn_cast_or_null<AttrClass>();
   }
   template <typename AttrClass> AttrClass getAttrOfType(StringRef name) {
     return getAttr(name).dyn_cast_or_null<AttrClass>();
   }
 
   /// Return true if the operation has an attribute with the provided name,
   /// false otherwise.
   bool hasAttr(Identifier name) { return static_cast<bool>(getAttr(name)); }
   bool hasAttr(StringRef name) { return static_cast<bool>(getAttr(name)); }
   template <typename AttrClass, typename NameT>
   bool hasAttrOfType(NameT &&name) {
     return static_cast<bool>(
         getAttrOfType<AttrClass>(std::forward<NameT>(name)));
   }
 
   /// If the an attribute exists with the specified name, change it to the new
-  /// value.  Otherwise, add a new attribute with the specified name/value.
-  void setAttr(Identifier name, Attribute value) { attrs.set(name, value); }
+  /// value. Otherwise, add a new attribute with the specified name/value.
+  void setAttr(Identifier name, Attribute value) {
+    NamedAttrList attributes(attrs);
+    if (attributes.set(name, value) != value)
+      attrs = attributes.getDictionary(getContext());
+  }
   void setAttr(StringRef name, Attribute value) {
     setAttr(Identifier::get(name, getContext()), value);
   }
 
-  /// Remove the attribute with the specified name if it exists.  The return
-  /// value indicates whether the attribute was present or not.
-  MutableDictionaryAttr::RemoveResult removeAttr(Identifier name) {
-    return attrs.remove(name);
+  /// Remove the attribute with the specified name if it exists. Return the
+  /// attribute that was erased, or nullptr if there was no attribute with such
+  /// name.
+  Attribute removeAttr(Identifier name) {
+    NamedAttrList attributes(attrs);
+    Attribute removedAttr = attributes.erase(name);
+    if (removedAttr)
+      attrs = attributes.getDictionary(getContext());
+    return removedAttr;
   }
-  MutableDictionaryAttr::RemoveResult removeAttr(StringRef name) {
-    return attrs.remove(Identifier::get(name, getContext()));
+  Attribute removeAttr(StringRef name) {
+    return removeAttr(Identifier::get(name, getContext()));
   }
 
   /// A utility iterator that filters out non-dialect attributes.
   class dialect_attr_iterator
       : public llvm::filter_iterator<ArrayRef<NamedAttribute>::iterator,
                                      bool (*)(NamedAttribute)> {
     static bool filter(NamedAttribute attr) {
       // Dialect attributes are prefixed by the dialect name, like operations.
public:
   dialect_attr_iterator dialect_attr_end() {
     auto attrs = getAttrs();
     return dialect_attr_iterator(attrs.end(), attrs.end());
   }
 
   /// Set the dialect attributes for this operation, and preserve all dependent.
   template <typename DialectAttrT>
   void setDialectAttrs(DialectAttrT &&dialectAttrs) {
-    SmallVector<NamedAttribute, 16> attrs;
-    attrs.assign(std::begin(dialectAttrs), std::end(dialectAttrs));
+    NamedAttrList attrs;
+    attrs.append(std::begin(dialectAttrs), std::end(dialectAttrs));
     for (auto attr : getAttrs())
-      if (!attr.first.strref().count('.'))
+      if (!attr.first.strref().contains('.'))
         attrs.push_back(attr);
-    setAttrs(llvm::makeArrayRef(attrs));
+    setAttrs(attrs.getDictionary(getContext()));
   }
 
   //===--------------------------------------------------------------------===//
   // Blocks
   //===--------------------------------------------------------------------===//
 
   /// Returns the number of regions held by this operation.
   unsigned getNumRegions() { return numRegions; }
private:
   void updateOrderIfNecessary();
 
   /// Returns true if this operation has a valid order.
   bool hasValidOrder() { return orderIndex != kInvalidOrderIdx; }
 
 private:
   Operation(Location location, OperationName name, TypeRange resultTypes,
             unsigned numSuccessors, unsigned numRegions,
-            const MutableDictionaryAttr &attributes, bool hasOperandStorage);
+            DictionaryAttr attributes, bool hasOperandStorage);
 
   // Operations are deleted through the destroy() member because they are
   // allocated with malloc.
   ~Operation();
 
   /// Returns the additional size necessary for allocating the given objects
   /// before an Operation in-memory.
   static size_t prefixAllocSize(unsigned numTrailingResults,
private:
   /// can use 'resultType` in an ArrayRef<Type>.
   bool hasSingleResult : 1;
   Type resultType;
 
   /// This holds the name of the operation.
   OperationName name;
 
   /// This holds general named attributes for the operation.
-  MutableDictionaryAttr attrs;
+  DictionaryAttr attrs;
 
   // allow ilist_traits access to 'block' field.
   friend struct llvm::ilist_traits<Operation>;
 
   // allow block to access the 'orderIndex' field.
   friend class Block;
 
   // allow value to access the 'ResultStorage' methods.

mlir/include/mlir/IR/OperationSupport.h

 #include "llvm/Support/PointerLikeTypeTraits.h"
 #include "llvm/Support/TrailingObjects.h"
 #include <memory>
 
 namespace mlir {
 class Dialect;
 class DictionaryAttr;
 class ElementsAttr;
-class MutableDictionaryAttr;
 class Operation;
 struct OperationState;
 class OpAsmParser;
 class OpAsmParserResult;
 class OpAsmPrinter;
 class OperandRange;
 class OpFoldResult;
 class ParseResult;

 public:
   using const_iterator = SmallVectorImpl<NamedAttribute>::const_iterator;
   using const_reference = const NamedAttribute &;
   using reference = NamedAttribute &;
   using size_type = size_t;
 
   NamedAttrList() : dictionarySorted({}, true) {}
   NamedAttrList(ArrayRef<NamedAttribute> attributes);
+  NamedAttrList(DictionaryAttr attributes);
   NamedAttrList(const_iterator in_start, const_iterator in_end);
 
   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.
-  void append(Identifier name, Attribute attr);
+  void append(Identifier name, Attribute attr) {
+    append(NamedAttribute(name, attr));
+  }
+
+  /// Append the given named attribute.
+  void append(NamedAttribute attr) { push_back(attr); }
 
   /// Add an array of named attributes.
-  void append(ArrayRef<NamedAttribute> newAttributes);
+  template <typename RangeT> void append(RangeT &&newAttributes) {
+    append(std::begin(newAttributes), std::end(newAttributes));
+  }
 
   /// Add a range of named attributes.
-  void append(const_iterator in_start, const_iterator in_end);
+  template <typename IteratorT>
+  void append(IteratorT in_start, IteratorT in_end) {
+    // TODO: expand to handle case where values appended are in order & after
+    // end of current list.
+    dictionarySorted.setPointerAndInt(nullptr, false);
+    attrs.append(in_start, in_end);
+  }
 
   /// Replaces the attributes with new list of attributes.
   void assign(const_iterator in_start, const_iterator in_end);
 
   /// Replaces the attributes with new list of attributes.
   void assign(ArrayRef<NamedAttribute> range) {
     append(range.begin(), range.end());
   }
public:
   Attribute get(Identifier name) const;
   Attribute get(StringRef name) const;
 
   /// Return the specified named attribute if present, None otherwise.
   Optional<NamedAttribute> getNamed(StringRef name) const;
   Optional<NamedAttribute> getNamed(Identifier name) const;
 
   /// If the an attribute exists with the specified name, change it to the new
-  /// value.  Otherwise, add a new attribute with the specified name/value.
-  void set(Identifier name, Attribute value);
-  void set(StringRef name, Attribute value);
+  /// value. Otherwise, add a new attribute with the specified name/value.
+  /// Returns the previous attribute value of `name`, or null if no
+  /// attribute previously existed with `name`.
+  Attribute set(Identifier name, Attribute value);
+  Attribute set(StringRef name, Attribute value);
 
   /// Erase the attribute with the given name from the list. Return the
   /// attribute that was erased, or nullptr if there was no attribute with such
   /// name.
   Attribute erase(Identifier name);
   Attribute erase(StringRef name);
 
   const_iterator begin() const { return attrs.begin(); }
   const_iterator end() const { return attrs.end(); }
 
   NamedAttrList &operator=(const SmallVectorImpl<NamedAttribute> &rhs);
   operator ArrayRef<NamedAttribute>() const;
-  operator MutableDictionaryAttr() const;
 
 private:
   /// Return whether the attributes are sorted.
   bool isSorted() const { return dictionarySorted.getInt(); }
 
   /// Erase the attribute at the given iterator position.
   Attribute eraseImpl(SmallVectorImpl<NamedAttribute>::iterator it);
 

mlir/lib/CAPI/IR/IR.cpp

 }
 
 void mlirOperationSetAttributeByName(MlirOperation op, MlirStringRef name,
                                      MlirAttribute attr) {
   unwrap(op)->setAttr(unwrap(name), unwrap(attr));
 }
 
 bool mlirOperationRemoveAttributeByName(MlirOperation op, MlirStringRef name) {
-  auto removeResult = unwrap(op)->removeAttr(unwrap(name));
-  return removeResult == MutableDictionaryAttr::RemoveResult::Removed;
+  return !!unwrap(op)->removeAttr(unwrap(name));
 }
 
 void mlirOperationPrint(MlirOperation op, MlirStringCallback callback,
                         void *userData) {
   detail::CallbackOstream stream(callback, userData);
   unwrap(op)->print(stream);
 }
 

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp

matchAndRewrite(ConstantOp op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const override {
     // If constant refers to a function, convert it to "addressof".
     if (auto symbolRef = op.getValue().dyn_cast<FlatSymbolRefAttr>()) {
       auto type = typeConverter->convertType(op.getResult().getType())
                       .dyn_cast_or_null<LLVM::LLVMType>();
       if (!type)
         return rewriter.notifyMatchFailure(op, "failed to convert result type");
 
-      MutableDictionaryAttr attrs(op.getAttrs());
-      attrs.remove(rewriter.getIdentifier("value"));
+      NamedAttrList attrs(op->getAttrDictionary());
+      attrs.erase("value");
       rewriter.replaceOpWithNewOp<LLVM::AddressOfOp>(
           op, type.cast<LLVM::LLVMType>(), symbolRef.getValue(),
           attrs.getAttrs());
       return success();
     }
 
     // Calling into other scopes (non-flat reference) is not supported in LLVM.
     if (op.getValue().isa<SymbolRefAttr>())

mlir/lib/Dialect/GPU/Transforms/AsyncRegionRewriter.cpp

LogicalResult rewriteAsyncOp(gpu::AsyncOpInterface asyncOp) {
     asyncOp.addAsyncDependency(currentToken);
 
     // Clone the op to return a token in addition to the other results.
     SmallVector<Type, 1> resultTypes;
     resultTypes.reserve(1 + op->getNumResults());
     copy(op->getResultTypes(), std::back_inserter(resultTypes));
     resultTypes.push_back(tokenType);
     auto *newOp = Operation::create(op->getLoc(), op->getName(), resultTypes,
-                                    op->getOperands(), op->getMutableAttrDict(),
+                                    op->getOperands(), op->getAttrDictionary(),
                                     op->getSuccessors());
 
     // Replace the op with the async clone.
     auto results = newOp->getResults();
     currentToken = results.back();
     builder.insert(newOp);
     op->replaceAllUsesWith(results.drop_back());
     op->erase();

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

Block *LLVMFuncOp::addEntryBlock() {
   for (unsigned i = 0, e = type.getFunctionNumParams(); i < e; ++i)
     entry->addArgument(type.getFunctionParamType(i));
   return entry;
 }
 
 void LLVMFuncOp::build(OpBuilder &builder, OperationState &result,
                        StringRef name, LLVMType type, LLVM::Linkage linkage,
                        ArrayRef<NamedAttribute> attrs,
-                       ArrayRef<MutableDictionaryAttr> argAttrs) {
+                       ArrayRef<DictionaryAttr> argAttrs) {
   result.addRegion();
   result.addAttribute(SymbolTable::getSymbolAttrName(),
                       builder.getStringAttr(name));
   result.addAttribute("type", TypeAttr::get(type));
   result.addAttribute(getLinkageAttrName(),
                       builder.getI64IntegerAttr(static_cast<int64_t>(linkage)));
   result.attributes.append(attrs.begin(), attrs.end());
   if (argAttrs.empty())
     return;
 
   unsigned numInputs = type.getFunctionNumParams();
   assert(numInputs == argAttrs.size() &&
          "expected as many argument attribute lists as arguments");
   SmallString<8> argAttrName;
   for (unsigned i = 0; i < numInputs; ++i)
-    if (auto argDict = argAttrs[i].getDictionary(builder.getContext()))
+    if (DictionaryAttr argDict = argAttrs[i])
       result.addAttribute(getArgAttrName(i, argAttrName), argDict);
 }
 
 // Builds an LLVM function type from the given lists of input and output types.
 // Returns a null type if any of the types provided are non-LLVM types, or if
 // there is more than one output type.
 static Type buildLLVMFunctionType(OpAsmParser &parser, llvm::SMLoc loc,
                                   ArrayRef<Type> inputs, ArrayRef<Type> outputs,

mlir/lib/IR/Attributes.cpp

 //===----------------------------------------------------------------------===//
 // Attribute
 //===----------------------------------------------------------------------===//
 
 /// Return the type of this attribute.
 Type Attribute::getType() const { return impl->getType(); }
 
 /// Return the context this attribute belongs to.
-MLIRContext *Attribute::getContext() const { return getType().getContext(); }
+MLIRContext *Attribute::getContext() const { return getDialect().getContext(); }
 
 /// Get the dialect this attribute is registered to.
 Dialect &Attribute::getDialect() const {
   return impl->getAbstractAttribute().getDialect();
 }
 
 //===----------------------------------------------------------------------===//
 // NamedAttribute

mlir/lib/IR/BuiltinAttributes.cpp

std::vector<ptrdiff_t> SparseElementsAttr::getFlattenedSparseIndices() const {
   // Otherwise, reinterpret each index as an ArrayRef when flattening.
   auto numSparseIndices = sparseIndices.getType().getDimSize(0);
   size_t rank = getType().getRank();
   for (size_t i = 0, e = numSparseIndices; i != e; ++i)
     flatSparseIndices.push_back(getFlattenedIndex(
         {&*std::next(sparseIndexValues.begin(), i * rank), rank}));
   return flatSparseIndices;
 }
-
-//===----------------------------------------------------------------------===//
-// MutableDictionaryAttr
-//===----------------------------------------------------------------------===//
-
-MutableDictionaryAttr::MutableDictionaryAttr(
-    ArrayRef<NamedAttribute> attributes) {
-  setAttrs(attributes);
-}
-
-/// Return the underlying dictionary attribute.
-DictionaryAttr
-MutableDictionaryAttr::getDictionary(MLIRContext *context) const {
-  // Construct empty DictionaryAttr if needed.
-  if (!attrs)
-    return DictionaryAttr::get({}, context);
-  return attrs;
-}
-
-ArrayRef<NamedAttribute> MutableDictionaryAttr::getAttrs() const {
-  return attrs ? attrs.getValue() : llvm::None;
-}
-
-/// Replace the held attributes with ones provided in 'newAttrs'.
-void MutableDictionaryAttr::setAttrs(ArrayRef<NamedAttribute> attributes) {
-  // Don't create an attribute list if there are no attributes.
-  if (attributes.empty())
-    attrs = nullptr;
-  else
-    attrs = DictionaryAttr::get(attributes, attributes[0].second.getContext());
-}
-
-/// Return the specified attribute if present, null otherwise.
-Attribute MutableDictionaryAttr::get(StringRef name) const {
-  return attrs ? attrs.get(name) : nullptr;
-}
-
-/// Return the specified attribute if present, null otherwise.
-Attribute MutableDictionaryAttr::get(Identifier name) const {
-  return attrs ? attrs.get(name) : nullptr;
-}
-
-/// Return the specified named attribute if present, None otherwise.
-Optional<NamedAttribute> MutableDictionaryAttr::getNamed(StringRef name) const {
-  return attrs ? attrs.getNamed(name) : Optional<NamedAttribute>();
-}
-Optional<NamedAttribute>
-MutableDictionaryAttr::getNamed(Identifier name) const {
-  return attrs ? attrs.getNamed(name) : Optional<NamedAttribute>();
-}
-
-/// If the an attribute exists with the specified name, change it to the new
-/// value.  Otherwise, add a new attribute with the specified name/value.
-void MutableDictionaryAttr::set(Identifier name, Attribute value) {
-  assert(value && "attributes may never be null");
-
-  // Look for an existing value for the given name, and set it in-place.
-  ArrayRef<NamedAttribute> values = getAttrs();
-  const auto *it = llvm::find_if(
-      values, [name](NamedAttribute attr) { return attr.first == name; });
-  if (it != values.end()) {
-    // Bail out early if the value is the same as what we already have.
-    if (it->second == value)
-      return;
-
-    SmallVector<NamedAttribute, 8> newAttrs(values.begin(), values.end());
-    newAttrs[it - values.begin()].second = value;
-    attrs = DictionaryAttr::getWithSorted(newAttrs, value.getContext());
-    return;
-  }
-
-  // Otherwise, insert the new attribute into its sorted position.
-  it = llvm::lower_bound(values, name);
-  SmallVector<NamedAttribute, 8> newAttrs;
-  newAttrs.reserve(values.size() + 1);
-  newAttrs.append(values.begin(), it);
-  newAttrs.push_back({name, value});
-  newAttrs.append(it, values.end());
-  attrs = DictionaryAttr::getWithSorted(newAttrs, value.getContext());
-}
-
-/// Remove the attribute with the specified name if it exists.  The return
-/// value indicates whether the attribute was present or not.
-auto MutableDictionaryAttr::remove(Identifier name) -> RemoveResult {
-  auto origAttrs = getAttrs();
-  for (unsigned i = 0, e = origAttrs.size(); i != e; ++i) {
-    if (origAttrs[i].first == name) {
-      // Handle the simple case of removing the only attribute in the list.
-      if (e == 1) {
-        attrs = nullptr;
-        return RemoveResult::Removed;
-      }
-
-      SmallVector<NamedAttribute, 8> newAttrs;
-      newAttrs.reserve(origAttrs.size() - 1);
-      newAttrs.append(origAttrs.begin(), origAttrs.begin() + i);
-      newAttrs.append(origAttrs.begin() + i + 1, origAttrs.end());
-      attrs = DictionaryAttr::getWithSorted(newAttrs,
-                                            newAttrs[0].second.getContext());
-      return RemoveResult::Removed;
-    }
-  }
-  return RemoveResult::NotFound;
-}

mlir/lib/IR/BuiltinDialect.cpp

 }
 FuncOp FuncOp::create(Location location, StringRef name, FunctionType type,
                       iterator_range<dialect_attr_iterator> attrs) {
   SmallVector<NamedAttribute, 8> attrRef(attrs);
   return create(location, name, type, llvm::makeArrayRef(attrRef));
 }
 FuncOp FuncOp::create(Location location, StringRef name, FunctionType type,
                       ArrayRef<NamedAttribute> attrs,
-                      ArrayRef<MutableDictionaryAttr> argAttrs) {
+                      ArrayRef<DictionaryAttr> argAttrs) {
   FuncOp func = create(location, name, type, attrs);
   func.setAllArgAttrs(argAttrs);
   return func;
 }
 
 void FuncOp::build(OpBuilder &builder, OperationState &state, StringRef name,
                    FunctionType type, ArrayRef<NamedAttribute> attrs,
-                   ArrayRef<MutableDictionaryAttr> argAttrs) {
+                   ArrayRef<DictionaryAttr> argAttrs) {
   state.addAttribute(SymbolTable::getSymbolAttrName(),
                      builder.getStringAttr(name));
   state.addAttribute(getTypeAttrName(), TypeAttr::get(type));
   state.attributes.append(attrs.begin(), attrs.end());
   state.addRegion();
 
   if (argAttrs.empty())
     return;
   assert(type.getNumInputs() == argAttrs.size());
   SmallString<8> argAttrName;
   for (unsigned i = 0, e = type.getNumInputs(); i != e; ++i)
-    if (auto argDict = argAttrs[i].getDictionary(builder.getContext()))
+    if (DictionaryAttr argDict = argAttrs[i])
       state.addAttribute(getArgAttrName(i, argAttrName), argDict);
 }
 
 static ParseResult parseFuncOp(OpAsmParser &parser, OperationState &result) {
   auto buildFuncType = [](Builder &builder, ArrayRef<Type> argTypes,
                           ArrayRef<Type> results, impl::VariadicFlag,
                           std::string &) {
     return builder.getFunctionType(argTypes, results);

mlir/lib/IR/FunctionSupport.cpp

void mlir::impl::eraseFunctionArguments(Operation *op,
   // There are 3 things that need to be updated:
   // - Function type.
   // - Arg attrs.
   // - Block arguments of entry block.
   Block &entry = op->getRegion(0).front();
   SmallString<8> nameBuf;
 
   // Collect arg attrs to set.
-  SmallVector<MutableDictionaryAttr, 4> newArgAttrs;
+  SmallVector<DictionaryAttr, 4> newArgAttrs;
   iterateIndicesExcept(originalNumArgs, argIndices, [&](unsigned i) {
     newArgAttrs.emplace_back(getArgAttrDict(op, i));
   });
 
   // Remove any arg attrs that are no longer needed.
   for (unsigned i = newArgAttrs.size(), e = originalNumArgs; i < e; ++i)
     op->removeAttr(getArgAttrName(i, nameBuf));
 
   // Set the function type.
   op->setAttr(getTypeAttrName(), TypeAttr::get(newType));
 
   // Set the new arg attrs, or remove them if empty.
   for (unsigned i = 0, e = newArgAttrs.size(); i != e; ++i) {
     auto nameAttr = getArgAttrName(i, nameBuf);
-    auto argAttr = newArgAttrs[i];
-    if (argAttr.empty())
-      op->removeAttr(nameAttr);
+    if (newArgAttrs[i] && !newArgAttrs[i].empty())
+      op->setAttr(nameAttr, newArgAttrs[i]);
     else
-      op->setAttr(nameAttr, argAttr.getDictionary(op->getContext()));
+      op->removeAttr(nameAttr);
   }
 
   // Update the entry block's arguments.
   entry.eraseArguments(argIndices);
 }
 
 void mlir::impl::eraseFunctionResults(Operation *op,
                                       ArrayRef<unsigned> resultIndices,
                                       unsigned originalNumResults,
                                       Type newType) {
   // There are 2 things that need to be updated:
   // - Function type.
   // - Result attrs.
   SmallString<8> nameBuf;
 
   // Collect result attrs to set.
-  SmallVector<MutableDictionaryAttr, 4> newResultAttrs;
+  SmallVector<DictionaryAttr, 4> newResultAttrs;
   iterateIndicesExcept(originalNumResults, resultIndices, [&](unsigned i) {
     newResultAttrs.emplace_back(getResultAttrDict(op, i));
   });
 
   // Remove any result attrs that are no longer needed.
   for (unsigned i = newResultAttrs.size(), e = originalNumResults; i < e; ++i)
     op->removeAttr(getResultAttrName(i, nameBuf));
 
   // Set the function type.
   op->setAttr(getTypeAttrName(), TypeAttr::get(newType));
 
   // Set the new result attrs, or remove them if empty.
   for (unsigned i = 0, e = newResultAttrs.size(); i != e; ++i) {
     auto nameAttr = getResultAttrName(i, nameBuf);
-    auto resultAttr = newResultAttrs[i];
-    if (resultAttr.empty())
-      op->removeAttr(nameAttr);
+    if (newResultAttrs[i] && !newResultAttrs[i].empty())
+      op->setAttr(nameAttr, newResultAttrs[i]);
     else
-      op->setAttr(nameAttr, resultAttr.getDictionary(op->getContext()));
+      op->removeAttr(nameAttr);
   }
 }

mlir/lib/IR/Operation.cpp

 //===----------------------------------------------------------------------===//
 
 /// 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,
-                MutableDictionaryAttr(attributes), successors, numRegions);
+                DictionaryAttr::get(attributes, location.getContext()),
+                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, state.successors, state.regions);
+                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,
-                             MutableDictionaryAttr attributes,
-                             BlockRange successors, RegionRange regions) {
+                             DictionaryAttr attributes, BlockRange successors,
+                             RegionRange regions) {
   unsigned numRegions = regions.size();
   Operation *op = create(location, name, resultTypes, operands, 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 MutableDictionaryAttr to avoid
+/// 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,
-                             MutableDictionaryAttr attributes,
-                             BlockRange successors, unsigned numRegions) {
+                             DictionaryAttr attributes, BlockRange successors,
+                             unsigned numRegions) {
   // 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();
   unsigned numOperands = operands.size();
 
   // If the operation is known to have no operands, don't allocate an operand
   // storage.
Operation *Operation::create(Location location, OperationName name,
   for (unsigned i = 0; i != numSuccessors; ++i)
     new (&blockOperands[i]) BlockOperand(op, successors[i]);
 
   return op;
 }
 
 Operation::Operation(Location location, OperationName name,
                      TypeRange resultTypes, unsigned numSuccessors,
-                     unsigned numRegions,
-                     const MutableDictionaryAttr &attributes,
+                     unsigned numRegions, DictionaryAttr attributes,
                      bool hasOperandStorage)
     : location(location), numSuccs(numSuccessors), numRegions(numRegions),
       hasOperandStorage(hasOperandStorage), hasSingleResult(false), name(name),
       attrs(attributes) {
+  assert(attributes && "unexpected null attribute dictionary");
   assert(llvm::all_of(resultTypes, [](Type t) { return t; }) &&
          "unexpected null result type");
   if (!resultTypes.empty()) {
     // If there is a single result it is stored in-place, otherwise use a tuple.
     hasSingleResult = resultTypes.size() == 1;
     if (hasSingleResult)
       resultType = resultTypes.front();
     else

mlir/lib/IR/OperationSupport.cpp

 //===----------------------------------------------------------------------===//
 // NamedAttrList
 //===----------------------------------------------------------------------===//
 
 NamedAttrList::NamedAttrList(ArrayRef<NamedAttribute> attributes) {
   assign(attributes.begin(), attributes.end());
 }
 
+NamedAttrList::NamedAttrList(DictionaryAttr attributes)
+    : NamedAttrList(attributes ? attributes.getValue()
+                               : ArrayRef<NamedAttribute>()) {
+  dictionarySorted.setPointerAndInt(attributes, true);
+}
+
 NamedAttrList::NamedAttrList(const_iterator in_start, const_iterator in_end) {
   assign(in_start, in_end);
 }
 
 ArrayRef<NamedAttribute> NamedAttrList::getAttrs() const { return attrs; }
 
 Optional<NamedAttribute> NamedAttrList::findDuplicate() const {
   Optional<NamedAttribute> duplicate =
if (!isSorted()) {
     DictionaryAttr::sortInPlace(attrs);
     dictionarySorted.setPointerAndInt(nullptr, true);
   }
   if (!dictionarySorted.getPointer())
     dictionarySorted.setPointer(DictionaryAttr::getWithSorted(attrs, context));
   return dictionarySorted.getPointer().cast<DictionaryAttr>();
 }
 
-NamedAttrList::operator MutableDictionaryAttr() const {
-  if (attrs.empty())
-    return MutableDictionaryAttr();
-  return getDictionary(attrs.front().second.getContext());
-}
-
 /// Add an attribute with the specified name.
 void NamedAttrList::append(StringRef name, Attribute attr) {
   append(Identifier::get(name, attr.getContext()), attr);
 }
 
-/// Add an attribute with the specified name.
-void NamedAttrList::append(Identifier name, Attribute attr) {
-  push_back({name, attr});
-}
-
-/// Add an array of named attributes.
-void NamedAttrList::append(ArrayRef<NamedAttribute> newAttributes) {
-  append(newAttributes.begin(), newAttributes.end());
-}
-
-/// Add a range of named attributes.
-void NamedAttrList::append(const_iterator in_start, const_iterator in_end) {
-  // TODO: expand to handle case where values appended are in order & after
-  // end of current list.
-  dictionarySorted.setPointerAndInt(nullptr, false);
-  attrs.append(in_start, in_end);
-}
-
 /// Replaces the attributes with new list of attributes.
 void NamedAttrList::assign(const_iterator in_start, const_iterator in_end) {
   DictionaryAttr::sort(ArrayRef<NamedAttribute>{in_start, in_end}, attrs);
   dictionarySorted.setPointerAndInt(nullptr, true);
 }
 
 void NamedAttrList::push_back(NamedAttribute newAttribute) {
   if (isSorted())

 }
 Optional<NamedAttribute> NamedAttrList::getNamed(Identifier name) const {
   auto *it = findAttr(attrs, name, isSorted());
   return it != attrs.end() ? *it : Optional<NamedAttribute>();
 }
 
 /// If the an attribute exists with the specified name, change it to the new
 /// value.  Otherwise, add a new attribute with the specified name/value.
-void NamedAttrList::set(Identifier name, Attribute value) {
+Attribute NamedAttrList::set(Identifier name, Attribute value) {
   assert(value && "attributes may never be null");
 
   // Look for an existing value for the given name, and set it in-place.
   auto *it = findAttr(attrs, name, isSorted());
   if (it != attrs.end()) {
-    // Bail out early if the value is the same as what we already have.
-    if (it->second == value)
-      return;
+    // Only update if the value is different from the existing.
+    Attribute oldValue = it->second;
+    if (oldValue != value) {
       dictionarySorted.setPointer(nullptr);
       it->second = value;
-    return;
+    }
+    return oldValue;
   }
 
   // Otherwise, insert the new attribute into its sorted position.
   it = llvm::lower_bound(attrs, name);
   dictionarySorted.setPointer(nullptr);
   attrs.insert(it, {name, value});
+  return Attribute();
 }
-void NamedAttrList::set(StringRef name, Attribute value) {
+Attribute NamedAttrList::set(StringRef name, Attribute value) {
   assert(value && "setting null attribute not supported");
   return set(mlir::Identifier::get(name, value.getContext()), value);
 }
 
 Attribute
 NamedAttrList::eraseImpl(SmallVectorImpl<NamedAttribute>::iterator it) {
   if (it == attrs.end())
     return nullptr;

 // Operation Equivalency
 //===----------------------------------------------------------------------===//
 
 llvm::hash_code OperationEquivalence::computeHash(Operation *op, Flags flags) {
   // Hash operations based upon their:
   //   - Operation Name
   //   - Attributes
   llvm::hash_code hash =
-      llvm::hash_combine(op->getName(), op->getMutableAttrDict());
+      llvm::hash_combine(op->getName(), op->getAttrDictionary());
 
   //   - Result Types
   ArrayRef<Type> resultTypes = op->getResultTypes();
   switch (resultTypes.size()) {
   case 0:
     // We don't need to add anything to the hash.
     break;
   case 1:
bool OperationEquivalence::isEquivalentTo(Operation *lhs, Operation *rhs,
 
   // Compare the operation name.
   if (lhs->getName() != rhs->getName())
     return false;
   // Check operand counts.
   if (lhs->getNumOperands() != rhs->getNumOperands())
     return false;
   // Compare attributes.
-  if (lhs->getMutableAttrDict() != rhs->getMutableAttrDict())
+  if (lhs->getAttrDictionary() != rhs->getAttrDictionary())
     return false;
   // Compare result types.
   ArrayRef<Type> lhsResultTypes = lhs->getResultTypes();
   ArrayRef<Type> rhsResultTypes = rhs->getResultTypes();
   if (lhsResultTypes.size() != rhsResultTypes.size())
     return false;
   switch (lhsResultTypes.size()) {
   case 0:

mlir/lib/IR/SymbolTable.cpp

 /// May have the following access chain:
 ///
 ///     [0, 0, 1]
 ///
 static WalkResult walkSymbolRefs(
     Operation *op,
     function_ref<WalkResult(SymbolTable::SymbolUse, ArrayRef<int>)> callback) {
   // Check to see if the operation has any attributes.
-  DictionaryAttr attrDict = op->getMutableAttrDict().getDictionaryOrNull();
-  if (!attrDict)
+  DictionaryAttr attrDict = op->getAttrDictionary();
+  if (attrDict.empty())
     return WalkResult::advance();
 
   // A worklist of a container attribute and the current index into the held
   // attribute list.
   SmallVector<Attribute, 1> attrWorklist(1, attrDict);
   SmallVector<int, 1> curAccessChain(1, /*Value=*/-1);
 
   // Process the symbol references within the given nested attribute range.

mlir/lib/Pass/IRPrinting.cpp

public:
   OperationFingerPrint(Operation *topOp) {
     llvm::SHA1 hasher;
 
     // Hash each of the operations based upon their mutable bits:
     topOp->walk([&](Operation *op) {
       //   - Operation pointer
       addDataToHash(hasher, op);
       //   - Attributes
-      addDataToHash(hasher, op->getMutableAttrDict());
+      addDataToHash(hasher, op->getAttrDictionary());
       //   - Blocks in Regions
       for (Region &region : op->getRegions()) {
         for (Block &block : region) {
           addDataToHash(hasher, &block);
           for (BlockArgument arg : block.getArguments())
             addDataToHash(hasher, arg);
         }
       }

mlir/lib/Target/SPIRV/Deserialization.cpp

private:
 
   // Result <id> to name mapping.
   DenseMap<uint32_t, StringRef> nameMap;
 
   // Result <id> to debug info mapping.
   DenseMap<uint32_t, StringRef> debugInfoMap;
 
   // Result <id> to decorations mapping.
-  DenseMap<uint32_t, MutableDictionaryAttr> decorations;
+  DenseMap<uint32_t, NamedAttrList> decorations;
 
   // Result <id> to type decorations.
   DenseMap<uint32_t, uint32_t> typeDecorations;
 
   // Result <id> to member decorations.
   // decorated-struct-type-<id> ->
   //    (struct-member-index -> (decoration -> decoration-operands))
   DenseMap<uint32_t,

mlir/lib/Transforms/SCCP.cpp

void SCCPSolver::visitOperation(Operation *op) {
   auto isOverdefinedFn = [&](Value value) { return isOverdefined(value); };
   if (llvm::all_of(op->getResults(), isOverdefinedFn))
     return;
 
   // Save the original operands and attributes just in case the operation folds
   // in-place. The constant passed in may not correspond to the real runtime
   // value, so in-place updates are not allowed.
   SmallVector<Value, 8> originalOperands(op->getOperands());
-  MutableDictionaryAttr originalAttrs = op->getMutableAttrDict();
+  DictionaryAttr originalAttrs = op->getAttrDictionary();
 
   // Simulate the result of folding this operation to a constant. If folding
   // fails or was an in-place fold, mark the results as overdefined.
   SmallVector<OpFoldResult, 8> foldResults;
   foldResults.reserve(op->getNumResults());
   if (failed(op->fold(operandConstants, foldResults)))
     return markAllOverdefined(op, op->getResults());
 

mlir/lib/Transforms/Utils/DialectConversion.cpp

 
 /// The state of an operation that was updated by a pattern in-place. This
 /// contains all of the necessary information to reconstruct an operation that
 /// was updated in place.
 class OperationTransactionState {
 public:
   OperationTransactionState() = default;
   OperationTransactionState(Operation *op)
-      : op(op), loc(op->getLoc()), attrs(op->getMutableAttrDict()),
+      : op(op), loc(op->getLoc()), attrs(op->getAttrDictionary()),
         operands(op->operand_begin(), op->operand_end()),
         successors(op->successor_begin(), op->successor_end()) {}
 
   /// Discard the transaction state and reset the state of the original
   /// operation.
   void resetOperation() const {
     op->setLoc(loc);
     op->setAttrs(attrs);
     op->setOperands(operands);
     for (auto it : llvm::enumerate(successors))
       op->setSuccessor(it.value(), it.index());
   }
 
   /// Return the original operation of this state.
   Operation *getOperation() const { return op; }
 
 private:
   Operation *op;
   LocationAttr loc;
-  MutableDictionaryAttr attrs;
+  DictionaryAttr attrs;
   SmallVector<Value, 8> operands;
   SmallVector<Block *, 2> successors;
 };
 
 /// This class represents one requested operation replacement via 'replaceOp' or
 /// 'eraseOp`.
 struct OpReplacement {
   OpReplacement() = default;

mlir/test/lib/Dialect/Test/TestDialect.cpp

static void printCustomDirectiveAttributes(OpAsmPrinter &printer, Operation *,
                                            Attribute attribute,
                                            Attribute optAttribute) {
   printer << attribute;
   if (optAttribute)
     printer << ", " << optAttribute;
 }
 
 static void printCustomDirectiveAttrDict(OpAsmPrinter &printer, Operation *op,
-                                         MutableDictionaryAttr attrs) {
-  printer.printOptionalAttrDict(attrs.getAttrs());
+                                         DictionaryAttr attrs) {
+  printer.printOptionalAttrDict(attrs.getValue());
 }
 //===----------------------------------------------------------------------===//
 // Test IsolatedRegionOp - parse passthrough region arguments.
 //===----------------------------------------------------------------------===//
 
 static ParseResult parseIsolatedRegionOp(OpAsmParser &parser,
                                          OperationState &result) {
   OpAsmParser::OperandType argInfo;

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

for (int i = 0, e = op.getNumOperands(); i != e; ++i) {
     auto *m = opClass.addMethodAndPrune("::mlir::MutableOperandRange",
                                         (operand.name + "Mutable").str());
     auto &body = m->body();
     body << "  auto range = getODSOperandIndexAndLength(" << i << ");\n"
          << "  return ::mlir::MutableOperandRange(getOperation(), "
             "range.first, range.second";
     if (attrSizedOperands)
       body << ", ::mlir::MutableOperandRange::OperandSegment(" << i
-           << "u, *getOperation()->getMutableAttrDict().getNamed("
+           << "u, *getOperation()->getAttrDictionary().getNamed("
               "\"operand_segment_sizes\"))";
     body << ");\n";
   }
 }
 
 void OpEmitter::genNamedResultGetters() {
   const int numResults = op.getNumResults();
   const int numVariadicResults = op.getNumVariableLengthResults();

mlir/tools/mlir-tblgen/OpFormatGen.cpp

 /// The code snippet used to generate a printer call for a region of an
 // operation that has the SingleBlockImplicitTerminator trait.
 ///
 /// {0}: The name of the region.
 const char *regionSingleBlockImplicitTerminatorPrinterCode = R"(
   {
     bool printTerminator = true;
     if (auto *term = {0}.empty() ? nullptr : {0}.begin()->getTerminator()) {{
-      printTerminator = !term->getMutableAttrDict().empty() ||
+      printTerminator = !term->getAttrDictionary().empty() ||
                         term->getNumOperands() != 0 ||
                         term->getNumResults() != 0;
     }
     p.printRegion({0}, /*printEntryBlockArgs=*/true,
                   /*printBlockTerminators=*/printTerminator);
   }
 )";
 
static void genCustomDirectivePrinter(CustomDirective *customDir,
                                       OpMethodBody &body) {
   body << "  print" << customDir->getName() << "(p, *this";
   for (Element &param : customDir->getArguments()) {
     body << ", ";
     if (auto *attr = dyn_cast<AttributeVariable>(&param)) {
       body << attr->getVar()->name << "Attr()";
 
     } else if (isa<AttrDictDirective>(&param)) {
-      // Enforce the const-ness since getMutableAttrDict() returns a reference
-      // into the Operations `attr` member.
-      body << "(const "
-              "MutableDictionaryAttr&)getOperation()->getMutableAttrDict()";
+      body << "getOperation()->getAttrDictionary()";
 
     } else if (auto *operand = dyn_cast<OperandVariable>(&param)) {
       body << operand->getVar()->name << "()";
 
     } else if (auto *region = dyn_cast<RegionVariable>(&param)) {
       body << region->getVar()->name << "()";
 
     } else if (auto *successor = dyn_cast<SuccessorVariable>(&param)) {