[mlir] Remove the MutableDictionaryAttr class

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.

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

flang/lib/Optimizer/Dialect/FIROps.cpp

@@ -1008,7 +1008,7 @@ 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));

mlir/docs/OpDefinitions.md

@@ -761,7 +761,7 @@ declarative parameter to `print` method argument is detailed below:
     -   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.
 

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

@@ -962,7 +962,7 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func",
     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 = [{

mlir/include/mlir/IR/BuiltinAttributes.h

@@ -1472,75 +1472,6 @@ auto ElementsAttr::getValues() const -> iterator_range<T> {
   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 {

mlir/include/mlir/IR/BuiltinOps.td

@@ -77,7 +77,7 @@ def FuncOp : Builtin_Op<"func", [
   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,
@@ -86,7 +86,7 @@ def FuncOp : Builtin_Op<"func", [
                          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

mlir/include/mlir/IR/FunctionSupport.h

@@ -300,8 +300,9 @@ public:
     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));
   }
@@ -328,8 +329,11 @@ public:
 
   /// 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]);
@@ -343,9 +347,10 @@ public:
                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
@@ -363,8 +368,9 @@ public:
     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));
   }
@@ -391,8 +397,10 @@ public:
 
   /// 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]);
@@ -407,9 +415,10 @@ public:
                   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
@@ -572,17 +581,14 @@ void FunctionLike<ConcreteType>::setArgAttrs(
 
 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
@@ -590,27 +596,26 @@ void FunctionLike<ConcreteType>::setArgAttrs(unsigned index,
 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;
 }
 
 //===----------------------------------------------------------------------===//
@@ -632,17 +637,15 @@ void FunctionLike<ConcreteType>::setResultAttrs(
 }
 
 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
@@ -650,27 +653,26 @@ void FunctionLike<ConcreteType>::setResultAttrs(
 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

mlir/include/mlir/IR/OpDefinition.h

@@ -183,21 +183,20 @@ public:
 
   /// 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()));
   }
 

mlir/include/mlir/IR/Operation.h

@@ -36,12 +36,12 @@ public:
                            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);
@@ -49,7 +49,7 @@ public:
   /// 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 = {});
 
@@ -304,21 +304,19 @@ public:
   // 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); }
@@ -342,19 +340,28 @@ public:
   }
 
   /// 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.
@@ -394,12 +401,12 @@ public:
   /// 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()));
   }
 
   //===--------------------------------------------------------------------===//
@@ -648,7 +655,7 @@ private:
 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.
@@ -731,7 +738,7 @@ private:
   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>;

mlir/include/mlir/IR/OperationSupport.h

@@ -32,7 +32,6 @@ namespace mlir {
 class Dialect;
 class DictionaryAttr;
 class ElementsAttr;
-class MutableDictionaryAttr;
 class Operation;
 struct OperationState;
 class OpAsmParser;
@@ -226,6 +225,7 @@ public:
 
   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 {
@@ -239,13 +239,26 @@ public:
   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);
@@ -285,9 +298,11 @@ public:
   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
@@ -300,7 +315,6 @@ public:
 
   NamedAttrList &operator=(const SmallVectorImpl<NamedAttribute> &rhs);
   operator ArrayRef<NamedAttribute>() const;
-  operator MutableDictionaryAttr() const;
 
 private:
   /// Return whether the attributes are sorted.

mlir/lib/CAPI/IR/IR.cpp

@@ -326,8 +326,7 @@ void mlirOperationSetAttributeByName(MlirOperation op, MlirStringRef name,
 }
 
 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,

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp

@@ -1897,8 +1897,8 @@ struct ConstantOpLowering : public ConvertOpToLLVMPattern<ConstantOp> {
       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());

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

@@ -90,7 +90,7 @@ private:
     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.

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

@@ -1597,7 +1597,7 @@ Block *LLVMFuncOp::addEntryBlock() {
 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));
@@ -1613,7 +1613,7 @@ void LLVMFuncOp::build(OpBuilder &builder, OperationState &result,
          "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);
 }
 

mlir/lib/IR/Attributes.cpp

@@ -35,7 +35,7 @@ void AttributeStorage::setType(Type newType) {
 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 {

mlir/lib/IR/BuiltinAttributes.cpp

@@ -1461,107 +1461,3 @@ std::vector<ptrdiff_t> SparseElementsAttr::getFlattenedSparseIndices() const {
         {&*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

@@ -85,7 +85,7 @@ FuncOp FuncOp::create(Location location, StringRef name, FunctionType type,
 }
 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;
@@ -93,7 +93,7 @@ FuncOp FuncOp::create(Location location, StringRef name, FunctionType type,
 
 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));
@@ -105,7 +105,7 @@ void FuncOp::build(OpBuilder &builder, OperationState &state, StringRef name,
   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);
 }
 

mlir/lib/IR/FunctionSupport.cpp

@@ -43,7 +43,7 @@ void mlir::impl::eraseFunctionArguments(Operation *op,
   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));
   });
@@ -58,11 +58,10 @@ void mlir::impl::eraseFunctionArguments(Operation *op,
   // 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.
@@ -79,7 +78,7 @@ void mlir::impl::eraseFunctionResults(Operation *op,
   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));
   });
@@ -94,10 +93,9 @@ void mlir::impl::eraseFunctionResults(Operation *op,
   // 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

@@ -76,20 +76,22 @@ Operation *Operation::create(Location location, OperationName name,
                              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);
@@ -99,12 +101,12 @@ Operation *Operation::create(Location location, OperationName name,
   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());
@@ -164,12 +166,12 @@ Operation *Operation::create(Location location, OperationName name,
 
 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()) {

mlir/lib/IR/OperationSupport.cpp

@@ -26,6 +26,12 @@ 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);
 }
@@ -52,35 +58,11 @@ DictionaryAttr NamedAttrList::getDictionary(MLIRContext *context) const {
   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);
@@ -136,26 +118,28 @@ Optional<NamedAttribute> NamedAttrList::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 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;
-    dictionarySorted.setPointer(nullptr);
-    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 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);
 }
@@ -555,7 +539,7 @@ llvm::hash_code OperationEquivalence::computeHash(Operation *op, Flags flags) {
   //   - 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();
@@ -597,7 +581,7 @@ bool OperationEquivalence::isEquivalentTo(Operation *lhs, Operation *rhs,
   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();

mlir/lib/IR/SymbolTable.cpp

@@ -456,8 +456,8 @@ 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

mlir/lib/Pass/IRPrinting.cpp

@@ -32,7 +32,7 @@ public:
       //   - 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) {

mlir/lib/Target/SPIRV/Deserialization.cpp

@@ -542,7 +542,7 @@ private:
   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;

mlir/lib/Transforms/SCCP.cpp

@@ -525,7 +525,7 @@ void SCCPSolver::visitOperation(Operation *op) {
   // 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.

mlir/lib/Transforms/Utils/DialectConversion.cpp

@@ -557,7 +557,7 @@ 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()) {}
 
@@ -577,7 +577,7 @@ public:
 private:
   Operation *op;
   LocationAttr loc;
-  MutableDictionaryAttr attrs;
+  DictionaryAttr attrs;
   SmallVector<Value, 8> operands;
   SmallVector<Block *, 2> successors;
 };

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

@@ -414,8 +414,8 @@ static void printCustomDirectiveAttributes(OpAsmPrinter &printer, Operation *,
 }
 
 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.

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

@@ -910,7 +910,7 @@ void OpEmitter::genNamedOperandSetters() {
             "range.first, range.second";
     if (attrSizedOperands)
       body << ", ::mlir::MutableOperandRange::OperandSegment(" << i
-           << "u, *getOperation()->getMutableAttrDict().getNamed("
+           << "u, *getOperation()->getAttrDictionary().getNamed("
               "\"operand_segment_sizes\"))";
     body << ");\n";
   }

mlir/tools/mlir-tblgen/OpFormatGen.cpp

@@ -1482,7 +1482,7 @@ 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;
     }
@@ -1555,10 +1555,7 @@ static void genCustomDirectivePrinter(CustomDirective *customDir,
       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 << "()";