diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp --- a/flang/lib/Optimizer/Dialect/FIROps.cpp +++ b/flang/lib/Optimizer/Dialect/FIROps.cpp @@ -1008,7 +1008,7 @@ StringRef offsetAttr) { Operation *owner = operands.getOwner(); NamedAttribute targetOffsetAttr = - *owner->getMutableAttrDict().getNamed(offsetAttr); + *owner->getAttrDictionary().getNamed(offsetAttr); return getSubOperands( pos, operands, targetOffsetAttr.second.cast(), mlir::MutableOperandRange::OperandSegment(pos, targetOffsetAttr)); diff --git a/mlir/docs/OpDefinitions.md b/mlir/docs/OpDefinitions.md --- a/mlir/docs/OpDefinitions.md +++ b/mlir/docs/OpDefinitions.md @@ -761,7 +761,7 @@ - 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. diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td @@ -962,7 +962,7 @@ OpBuilderDAG<(ins "StringRef":$name, "LLVMType":$type, CArg<"Linkage", "Linkage::External">:$linkage, CArg<"ArrayRef", "{}">:$attrs, - CArg<"ArrayRef", "{}">:$argAttrs)> + CArg<"ArrayRef", "{}">:$argAttrs)> ]; let extraClassDeclaration = [{ diff --git a/mlir/include/mlir/IR/BuiltinAttributes.h b/mlir/include/mlir/IR/BuiltinAttributes.h --- a/mlir/include/mlir/IR/BuiltinAttributes.h +++ b/mlir/include/mlir/IR/BuiltinAttributes.h @@ -1472,75 +1472,6 @@ 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 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 getAttrs() const; - - /// Replace the held attributes with ones provided in 'newAttrs'. - void setAttrs(ArrayRef 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 getNamed(StringRef name) const; - Optional 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 { diff --git a/mlir/include/mlir/IR/BuiltinOps.td b/mlir/include/mlir/IR/BuiltinOps.td --- a/mlir/include/mlir/IR/BuiltinOps.td +++ b/mlir/include/mlir/IR/BuiltinOps.td @@ -77,7 +77,7 @@ let builders = [OpBuilderDAG<(ins "StringRef":$name, "FunctionType":$type, CArg<"ArrayRef", "{}">:$attrs, - CArg<"ArrayRef", "{}">:$argAttrs) + CArg<"ArrayRef", "{}">:$argAttrs) >]; let extraClassDeclaration = [{ static FuncOp create(Location location, StringRef name, FunctionType type, @@ -86,7 +86,7 @@ iterator_range attrs); static FuncOp create(Location location, StringRef name, FunctionType type, ArrayRef attrs, - ArrayRef argAttrs); + ArrayRef 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 diff --git a/mlir/include/mlir/IR/FunctionSupport.h b/mlir/include/mlir/IR/FunctionSupport.h --- a/mlir/include/mlir/IR/FunctionSupport.h +++ b/mlir/include/mlir/IR/FunctionSupport.h @@ -300,8 +300,9 @@ return ::mlir::impl::getArgAttrs(this->getOperation(), index); } - /// Return all argument attributes of this function. - void getAllArgAttrs(SmallVectorImpl &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 &result) { for (unsigned i = 0, e = getNumArguments(); i != e; ++i) result.emplace_back(getArgAttrDict(i)); } @@ -328,8 +329,11 @@ /// Set the attributes held by the argument at 'index'. void setArgAttrs(unsigned index, ArrayRef attributes); - void setArgAttrs(unsigned index, MutableDictionaryAttr attributes); - void setAllArgAttrs(ArrayRef 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 attributes) { assert(attributes.size() == getNumArguments()); for (unsigned i = 0, e = attributes.size(); i != e; ++i) setArgAttrs(i, attributes[i]); @@ -343,9 +347,10 @@ 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 @@ return ::mlir::impl::getResultAttrs(this->getOperation(), index); } - /// Return all result attributes of this function. - void getAllResultAttrs(SmallVectorImpl &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 &result) { for (unsigned i = 0, e = getNumResults(); i != e; ++i) result.emplace_back(getResultAttrDict(i)); } @@ -391,8 +397,10 @@ /// Set the attributes held by the result at 'index'. void setResultAttrs(unsigned index, ArrayRef attributes); - void setResultAttrs(unsigned index, MutableDictionaryAttr attributes); - void setAllResultAttrs(ArrayRef 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 attributes) { assert(attributes.size() == getNumResults()); for (unsigned i = 0, e = attributes.size(); i != e; ++i) setResultAttrs(i, attributes[i]); @@ -407,9 +415,10 @@ 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 @@ template void FunctionLike::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 @@ template void FunctionLike::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 -MutableDictionaryAttr::RemoveResult -FunctionLike::removeArgAttr(unsigned index, Identifier name) { +Attribute FunctionLike::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 @@ } template -void FunctionLike::setResultAttrs( - unsigned index, MutableDictionaryAttr attributes) { +void FunctionLike::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 @@ template void FunctionLike::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 -MutableDictionaryAttr::RemoveResult -FunctionLike::removeResultAttr(unsigned index, Identifier name) { +Attribute FunctionLike::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 diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h --- a/mlir/include/mlir/IR/OpDefinition.h +++ b/mlir/include/mlir/IR/OpDefinition.h @@ -183,21 +183,20 @@ /// Set the attributes held by this operation. void setAttrs(ArrayRef 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 void setDialectAttrs(DialectAttrs &&attrs) { - state->setDialectAttrs(std::move(attrs)); + state->setDialectAttrs(std::forward(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())); } diff --git a/mlir/include/mlir/IR/Operation.h b/mlir/include/mlir/IR/Operation.h --- a/mlir/include/mlir/IR/Operation.h +++ b/mlir/include/mlir/IR/Operation.h @@ -36,12 +36,12 @@ ArrayRef 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 @@ /// 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 @@ // the lifetime of an operation. /// Return all of the attributes on this operation. - ArrayRef getAttrs() { return attrs.getAttrs(); } + ArrayRef 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 newAttrs) { attrs = newAttrs; } + void setAttrs(DictionaryAttr newAttrs) { + assert(newAttrs && "expected valid attribute dictionary"); + attrs = newAttrs; + } + void setAttrs(ArrayRef 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 @@ } /// 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 @@ /// Set the dialect attributes for this operation, and preserve all dependent. template void setDialectAttrs(DialectAttrT &&dialectAttrs) { - SmallVector 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: 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 @@ 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; diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h --- a/mlir/include/mlir/IR/OperationSupport.h +++ b/mlir/include/mlir/IR/OperationSupport.h @@ -32,7 +32,6 @@ class Dialect; class DictionaryAttr; class ElementsAttr; -class MutableDictionaryAttr; class Operation; struct OperationState; class OpAsmParser; @@ -226,6 +225,7 @@ NamedAttrList() : dictionarySorted({}, true) {} NamedAttrList(ArrayRef attributes); + NamedAttrList(DictionaryAttr attributes); NamedAttrList(const_iterator in_start, const_iterator in_end); bool operator!=(const NamedAttrList &other) const { @@ -239,13 +239,26 @@ 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 newAttributes); + template 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 + 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 @@ Optional 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 @@ NamedAttrList &operator=(const SmallVectorImpl &rhs); operator ArrayRef() const; - operator MutableDictionaryAttr() const; private: /// Return whether the attributes are sorted. diff --git a/mlir/lib/CAPI/IR/IR.cpp b/mlir/lib/CAPI/IR/IR.cpp --- a/mlir/lib/CAPI/IR/IR.cpp +++ b/mlir/lib/CAPI/IR/IR.cpp @@ -326,8 +326,7 @@ } 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, diff --git a/mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp b/mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp --- a/mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp +++ b/mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp @@ -1897,8 +1897,8 @@ 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( op, type.cast(), symbolRef.getValue(), attrs.getAttrs()); diff --git a/mlir/lib/Dialect/GPU/Transforms/AsyncRegionRewriter.cpp b/mlir/lib/Dialect/GPU/Transforms/AsyncRegionRewriter.cpp --- a/mlir/lib/Dialect/GPU/Transforms/AsyncRegionRewriter.cpp +++ b/mlir/lib/Dialect/GPU/Transforms/AsyncRegionRewriter.cpp @@ -90,7 +90,7 @@ 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. diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp @@ -1597,7 +1597,7 @@ void LLVMFuncOp::build(OpBuilder &builder, OperationState &result, StringRef name, LLVMType type, LLVM::Linkage linkage, ArrayRef attrs, - ArrayRef argAttrs) { + ArrayRef argAttrs) { result.addRegion(); result.addAttribute(SymbolTable::getSymbolAttrName(), builder.getStringAttr(name)); @@ -1613,7 +1613,7 @@ "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); } diff --git a/mlir/lib/IR/Attributes.cpp b/mlir/lib/IR/Attributes.cpp --- a/mlir/lib/IR/Attributes.cpp +++ b/mlir/lib/IR/Attributes.cpp @@ -35,7 +35,7 @@ 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 { diff --git a/mlir/lib/IR/BuiltinAttributes.cpp b/mlir/lib/IR/BuiltinAttributes.cpp --- a/mlir/lib/IR/BuiltinAttributes.cpp +++ b/mlir/lib/IR/BuiltinAttributes.cpp @@ -1461,107 +1461,3 @@ {&*std::next(sparseIndexValues.begin(), i * rank), rank})); return flatSparseIndices; } - -//===----------------------------------------------------------------------===// -// MutableDictionaryAttr -//===----------------------------------------------------------------------===// - -MutableDictionaryAttr::MutableDictionaryAttr( - ArrayRef 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 MutableDictionaryAttr::getAttrs() const { - return attrs ? attrs.getValue() : llvm::None; -} - -/// Replace the held attributes with ones provided in 'newAttrs'. -void MutableDictionaryAttr::setAttrs(ArrayRef 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 MutableDictionaryAttr::getNamed(StringRef name) const { - return attrs ? attrs.getNamed(name) : Optional(); -} -Optional -MutableDictionaryAttr::getNamed(Identifier name) const { - return attrs ? attrs.getNamed(name) : Optional(); -} - -/// 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 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 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 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 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; -} diff --git a/mlir/lib/IR/BuiltinDialect.cpp b/mlir/lib/IR/BuiltinDialect.cpp --- a/mlir/lib/IR/BuiltinDialect.cpp +++ b/mlir/lib/IR/BuiltinDialect.cpp @@ -85,7 +85,7 @@ } FuncOp FuncOp::create(Location location, StringRef name, FunctionType type, ArrayRef attrs, - ArrayRef argAttrs) { + ArrayRef argAttrs) { FuncOp func = create(location, name, type, attrs); func.setAllArgAttrs(argAttrs); return func; @@ -93,7 +93,7 @@ void FuncOp::build(OpBuilder &builder, OperationState &state, StringRef name, FunctionType type, ArrayRef attrs, - ArrayRef argAttrs) { + ArrayRef argAttrs) { state.addAttribute(SymbolTable::getSymbolAttrName(), builder.getStringAttr(name)); state.addAttribute(getTypeAttrName(), TypeAttr::get(type)); @@ -105,7 +105,7 @@ 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); } diff --git a/mlir/lib/IR/FunctionSupport.cpp b/mlir/lib/IR/FunctionSupport.cpp --- a/mlir/lib/IR/FunctionSupport.cpp +++ b/mlir/lib/IR/FunctionSupport.cpp @@ -43,7 +43,7 @@ SmallString<8> nameBuf; // Collect arg attrs to set. - SmallVector newArgAttrs; + SmallVector newArgAttrs; iterateIndicesExcept(originalNumArgs, argIndices, [&](unsigned i) { newArgAttrs.emplace_back(getArgAttrDict(op, i)); }); @@ -58,11 +58,10 @@ // 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 @@ SmallString<8> nameBuf; // Collect result attrs to set. - SmallVector newResultAttrs; + SmallVector newResultAttrs; iterateIndicesExcept(originalNumResults, resultIndices, [&](unsigned i) { newResultAttrs.emplace_back(getResultAttrDict(op, i)); }); @@ -94,10 +93,9 @@ // 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); } } diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp --- a/mlir/lib/IR/Operation.cpp +++ b/mlir/lib/IR/Operation.cpp @@ -76,20 +76,22 @@ ArrayRef 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 @@ 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(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()) { diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp --- a/mlir/lib/IR/OperationSupport.cpp +++ b/mlir/lib/IR/OperationSupport.cpp @@ -26,6 +26,12 @@ assign(attributes.begin(), attributes.end()); } +NamedAttrList::NamedAttrList(DictionaryAttr attributes) + : NamedAttrList(attributes ? attributes.getValue() + : ArrayRef()) { + dictionarySorted.setPointerAndInt(attributes, true); +} + NamedAttrList::NamedAttrList(const_iterator in_start, const_iterator in_end) { assign(in_start, in_end); } @@ -52,35 +58,11 @@ return dictionarySorted.getPointer().cast(); } -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 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{in_start, in_end}, attrs); @@ -136,26 +118,28 @@ /// 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 @@ // - Operation Name // - Attributes llvm::hash_code hash = - llvm::hash_combine(op->getName(), op->getMutableAttrDict()); + llvm::hash_combine(op->getName(), op->getAttrDictionary()); // - Result Types ArrayRef resultTypes = op->getResultTypes(); @@ -597,7 +581,7 @@ 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 lhsResultTypes = lhs->getResultTypes(); diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp --- a/mlir/lib/IR/SymbolTable.cpp +++ b/mlir/lib/IR/SymbolTable.cpp @@ -456,8 +456,8 @@ Operation *op, function_ref)> 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 diff --git a/mlir/lib/Pass/IRPrinting.cpp b/mlir/lib/Pass/IRPrinting.cpp --- a/mlir/lib/Pass/IRPrinting.cpp +++ b/mlir/lib/Pass/IRPrinting.cpp @@ -32,7 +32,7 @@ // - Operation pointer addDataToHash(hasher, op); // - Attributes - addDataToHash(hasher, op->getMutableAttrDict()); + addDataToHash(hasher, op->getAttrDictionary()); // - Blocks in Regions for (Region ®ion : op->getRegions()) { for (Block &block : region) { diff --git a/mlir/lib/Target/SPIRV/Deserialization.cpp b/mlir/lib/Target/SPIRV/Deserialization.cpp --- a/mlir/lib/Target/SPIRV/Deserialization.cpp +++ b/mlir/lib/Target/SPIRV/Deserialization.cpp @@ -542,7 +542,7 @@ DenseMap debugInfoMap; // Result to decorations mapping. - DenseMap decorations; + DenseMap decorations; // Result to type decorations. DenseMap typeDecorations; diff --git a/mlir/lib/Transforms/SCCP.cpp b/mlir/lib/Transforms/SCCP.cpp --- a/mlir/lib/Transforms/SCCP.cpp +++ b/mlir/lib/Transforms/SCCP.cpp @@ -525,7 +525,7 @@ // in-place. The constant passed in may not correspond to the real runtime // value, so in-place updates are not allowed. SmallVector 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. diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp --- a/mlir/lib/Transforms/Utils/DialectConversion.cpp +++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp @@ -557,7 +557,7 @@ 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 @@ private: Operation *op; LocationAttr loc; - MutableDictionaryAttr attrs; + DictionaryAttr attrs; SmallVector operands; SmallVector successors; }; diff --git a/mlir/test/lib/Dialect/Test/TestDialect.cpp b/mlir/test/lib/Dialect/Test/TestDialect.cpp --- a/mlir/test/lib/Dialect/Test/TestDialect.cpp +++ b/mlir/test/lib/Dialect/Test/TestDialect.cpp @@ -414,8 +414,8 @@ } 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. diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp --- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp +++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp @@ -910,7 +910,7 @@ "range.first, range.second"; if (attrSizedOperands) body << ", ::mlir::MutableOperandRange::OperandSegment(" << i - << "u, *getOperation()->getMutableAttrDict().getNamed(" + << "u, *getOperation()->getAttrDictionary().getNamed(" "\"operand_segment_sizes\"))"; body << ");\n"; } diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp --- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp +++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp @@ -1482,7 +1482,7 @@ { 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 @@ body << attr->getVar()->name << "Attr()"; } else if (isa(¶m)) { - // 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(¶m)) { body << operand->getVar()->name << "()";