[Mlir-commits] [mlir] c298824 - [MLIR] Check for duplicate entries in attribute dictionary during custom parsing
Rahul Joshi
llvmlistbot at llvm.org
Tue Nov 3 16:41:13 PST 2020
Author: Rahul Joshi
Date: 2020-11-03T16:40:46-08:00
New Revision: c298824f9caf407aedeb4958467fb2a18025638d
URL: https://github.com/llvm/llvm-project/commit/c298824f9caf407aedeb4958467fb2a18025638d
DIFF: https://github.com/llvm/llvm-project/commit/c298824f9caf407aedeb4958467fb2a18025638d.diff
LOG: [MLIR] Check for duplicate entries in attribute dictionary during custom parsing
- Verify that attributes parsed using a custom parser do not have duplicates.
- If there are duplicated in the attribute dictionary in the input, they get caught during the
dictionary parsing.
- This check verifies that there is no duplication between the parsed dictionary and any
attributes that might be added by the custom parser (or when the custom parsing code
adds duplicate attributes).
- Fixes https://bugs.llvm.org/show_bug.cgi?id=48025
Differential Revision: https://reviews.llvm.org/D90502
Added:
Modified:
mlir/include/mlir/IR/Attributes.h
mlir/include/mlir/IR/OperationSupport.h
mlir/lib/IR/Attributes.cpp
mlir/lib/IR/OperationSupport.cpp
mlir/lib/Parser/AttributeParser.cpp
mlir/lib/Parser/Parser.cpp
mlir/test/IR/invalid.mlir
Removed:
################################################################################
diff --git a/mlir/include/mlir/IR/Attributes.h b/mlir/include/mlir/IR/Attributes.h
index 4a8cc562cb52..f02470891fcf 100644
--- a/mlir/include/mlir/IR/Attributes.h
+++ b/mlir/include/mlir/IR/Attributes.h
@@ -292,6 +292,12 @@ class DictionaryAttr
/// Requires: uniquely named attributes.
static bool sortInPlace(SmallVectorImpl<NamedAttribute> &array);
+ /// Returns an entry with a duplicate name in `array`, if it exists, else
+ /// returns llvm::None. If `isSorted` is true, the array is assumed to be
+ /// sorted else it will be sorted in place before finding the duplicate entry.
+ static Optional<NamedAttribute>
+ findDuplicate(SmallVectorImpl<NamedAttribute> &array, bool isSorted);
+
private:
/// Return empty dictionary.
static DictionaryAttr getEmpty(MLIRContext *context);
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 563a906ec803..96d6d1194b60 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -262,6 +262,10 @@ class NamedAttrList {
/// Pop last element from list.
void pop_back() { attrs.pop_back(); }
+ /// Returns an entry with a duplicate name the list, if it exists, else
+ /// returns llvm::None.
+ Optional<NamedAttribute> findDuplicate() const;
+
/// Return a dictionary attribute for the underlying dictionary. This will
/// return an empty dictionary attribute if empty rather than null.
DictionaryAttr getDictionary(MLIRContext *context) const;
diff --git a/mlir/lib/IR/Attributes.cpp b/mlir/lib/IR/Attributes.cpp
index 59a66d0c3425..37c4edb7322f 100644
--- a/mlir/lib/IR/Attributes.cpp
+++ b/mlir/lib/IR/Attributes.cpp
@@ -99,8 +99,6 @@ static bool dictionaryAttrSort(ArrayRef<NamedAttribute> value,
storage.assign({value[0]});
break;
case 2: {
- assert(value[0].first != value[1].first &&
- "DictionaryAttr element names must be unique");
bool isSorted = value[0] < value[1];
if (inPlace) {
if (!isSorted)
@@ -122,25 +120,49 @@ static bool dictionaryAttrSort(ArrayRef<NamedAttribute> value,
llvm::array_pod_sort(storage.begin(), storage.end());
value = storage;
}
-
- // Ensure that the attribute elements are unique.
- assert(std::adjacent_find(value.begin(), value.end(),
- [](NamedAttribute l, NamedAttribute r) {
- return l.first == r.first;
- }) == value.end() &&
- "DictionaryAttr element names must be unique");
return !isSorted;
}
return false;
}
+/// Returns an entry with a duplicate name from the given sorted array of named
+/// attributes. Returns llvm::None if all elements have unique names.
+static Optional<NamedAttribute>
+findDuplicateElement(ArrayRef<NamedAttribute> value) {
+ const Optional<NamedAttribute> none{llvm::None};
+ if (value.size() < 2)
+ return none;
+
+ if (value.size() == 2)
+ return value[0].first == value[1].first ? value[0] : none;
+
+ auto it = std::adjacent_find(
+ value.begin(), value.end(),
+ [](NamedAttribute l, NamedAttribute r) { return l.first == r.first; });
+ return it != value.end() ? *it : none;
+}
+
bool DictionaryAttr::sort(ArrayRef<NamedAttribute> value,
SmallVectorImpl<NamedAttribute> &storage) {
- return dictionaryAttrSort</*inPlace=*/false>(value, storage);
+ bool isSorted = dictionaryAttrSort</*inPlace=*/false>(value, storage);
+ assert(!findDuplicateElement(storage) &&
+ "DictionaryAttr element names must be unique");
+ return isSorted;
}
bool DictionaryAttr::sortInPlace(SmallVectorImpl<NamedAttribute> &array) {
- return dictionaryAttrSort</*inPlace=*/true>(array, array);
+ bool isSorted = dictionaryAttrSort</*inPlace=*/true>(array, array);
+ assert(!findDuplicateElement(array) &&
+ "DictionaryAttr element names must be unique");
+ return isSorted;
+}
+
+Optional<NamedAttribute>
+DictionaryAttr::findDuplicate(SmallVectorImpl<NamedAttribute> &array,
+ bool isSorted) {
+ if (!isSorted)
+ dictionaryAttrSort</*inPlace=*/true>(array, array);
+ return findDuplicateElement(array);
}
DictionaryAttr DictionaryAttr::get(ArrayRef<NamedAttribute> value,
@@ -155,7 +177,8 @@ DictionaryAttr DictionaryAttr::get(ArrayRef<NamedAttribute> value,
SmallVector<NamedAttribute, 8> storage;
if (dictionaryAttrSort</*inPlace=*/false>(value, storage))
value = storage;
-
+ assert(!findDuplicateElement(value) &&
+ "DictionaryAttr element names must be unique");
return Base::get(context, value);
}
/// Construct a dictionary with an array of values that is known to already be
@@ -170,10 +193,7 @@ DictionaryAttr DictionaryAttr::getWithSorted(ArrayRef<NamedAttribute> value,
return l.first.strref() < r.first.strref();
}) &&
"expected attribute values to be sorted");
- assert(std::adjacent_find(value.begin(), value.end(),
- [](NamedAttribute l, NamedAttribute r) {
- return l.first == r.first;
- }) == value.end() &&
+ assert(!findDuplicateElement(value) &&
"DictionaryAttr element names must be unique");
return Base::get(context, value);
}
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index 0d72ea5f0ea9..c7aa0e323088 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -32,6 +32,16 @@ NamedAttrList::NamedAttrList(const_iterator in_start, const_iterator in_end) {
ArrayRef<NamedAttribute> NamedAttrList::getAttrs() const { return attrs; }
+Optional<NamedAttribute> NamedAttrList::findDuplicate() const {
+ Optional<NamedAttribute> duplicate =
+ DictionaryAttr::findDuplicate(attrs, isSorted());
+ // DictionaryAttr::findDuplicate will sort the list, so reset the sorted
+ // state.
+ if (!isSorted())
+ dictionarySorted.setPointerAndInt(nullptr, true);
+ return duplicate;
+}
+
DictionaryAttr NamedAttrList::getDictionary(MLIRContext *context) const {
if (!isSorted()) {
DictionaryAttr::sortInPlace(attrs);
diff --git a/mlir/lib/Parser/AttributeParser.cpp b/mlir/lib/Parser/AttributeParser.cpp
index 6234d3b59660..e8f5213768bd 100644
--- a/mlir/lib/Parser/AttributeParser.cpp
+++ b/mlir/lib/Parser/AttributeParser.cpp
@@ -253,7 +253,8 @@ ParseResult Parser::parseAttributeDict(NamedAttrList &attributes) {
else
return emitError("expected attribute name");
if (!seenKeys.insert(*nameId).second)
- return emitError("duplicate key in dictionary attribute");
+ return emitError("duplicate key '")
+ << *nameId << "' in dictionary attribute";
consumeToken();
// Lazy load a dialect in the context if there is a possible namespace.
diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp
index e89972007156..a824687aefb2 100644
--- a/mlir/lib/Parser/Parser.cpp
+++ b/mlir/lib/Parser/Parser.cpp
@@ -846,6 +846,15 @@ class CustomOpAsmParser : public OpAsmParser {
ParseResult parseOperation(OperationState &opState) {
if (opDefinition->parseAssembly(*this, opState))
return failure();
+ // Verify that the parsed attributes does not have duplicate attributes.
+ // This can happen if an attribute set during parsing is also specified in
+ // the attribute dictionary in the assembly, or the attribute is set
+ // multiple during parsing.
+ Optional<NamedAttribute> duplicate = opState.attributes.findDuplicate();
+ if (duplicate)
+ return emitError(getNameLoc(), "attribute '")
+ << duplicate->first
+ << "' occurs more than once in the attribute list";
return success();
}
diff --git a/mlir/test/IR/invalid.mlir b/mlir/test/IR/invalid.mlir
index ae686365dded..c8ed517434b2 100644
--- a/mlir/test/IR/invalid.mlir
+++ b/mlir/test/IR/invalid.mlir
@@ -1513,12 +1513,17 @@ func @really_large_bound() {
// -----
func @duplicate_dictionary_attr_key() {
- // expected-error @+1 {{duplicate key in dictionary attribute}}
+ // expected-error @+1 {{duplicate key 'a' in dictionary attribute}}
"foo.op"() {a, a} : () -> ()
}
// -----
+// expected-error @+1 {{attribute 'attr' occurs more than once in the attribute list}}
+test.format_symbol_name_attr_op @name { attr = "xx" }
+
+// -----
+
func @forward_reference_type_check() -> (i8) {
br ^bb2
More information about the Mlir-commits
mailing list