[Mlir-commits] [mlir] d2f3e5f - [mlir] Add support for non-identifier attribute names.
River Riddle
llvmlistbot at llvm.org
Wed Mar 11 13:26:04 PDT 2020
Author: River Riddle
Date: 2020-03-11T13:22:33-07:00
New Revision: d2f3e5f204cfb06e1862476ade175ccf0bb7ca0f
URL: https://github.com/llvm/llvm-project/commit/d2f3e5f204cfb06e1862476ade175ccf0bb7ca0f
DIFF: https://github.com/llvm/llvm-project/commit/d2f3e5f204cfb06e1862476ade175ccf0bb7ca0f.diff
LOG: [mlir] Add support for non-identifier attribute names.
Summary: In some situations the name of the attribute is not representable as a bare-identifier, this revision adds support for those cases by formatting the name as a string instead. This has the added benefit of removing the identifier regex from the verifier.
Differential Revision: https://reviews.llvm.org/D75973
Added:
Modified:
mlir/docs/LangRef.md
mlir/lib/Analysis/Verifier.cpp
mlir/lib/IR/AsmPrinter.cpp
mlir/lib/Parser/Parser.cpp
mlir/test/IR/parser.mlir
Removed:
################################################################################
diff --git a/mlir/docs/LangRef.md b/mlir/docs/LangRef.md
index 61a37b6d447d..d59e552e32c7 100644
--- a/mlir/docs/LangRef.md
+++ b/mlir/docs/LangRef.md
@@ -1156,7 +1156,8 @@ attribute-dict ::= `{` `}`
attribute-entry ::= dialect-attribute-entry | dependent-attribute-entry
dialect-attribute-entry ::= dialect-namespace `.` bare-id `=` attribute-value
dependent-attribute-entry ::= dependent-attribute-name `=` attribute-value
-dependent-attribute-name ::= (letter|[_]) (letter|digit|[_$])*
+dependent-attribute-name ::= ((letter|[_]) (letter|digit|[_$])*)
+ | string-literal
```
Attributes are the mechanism for specifying constant data on operations in
diff --git a/mlir/lib/Analysis/Verifier.cpp b/mlir/lib/Analysis/Verifier.cpp
index 37e2486569a7..161feb426c2f 100644
--- a/mlir/lib/Analysis/Verifier.cpp
+++ b/mlir/lib/Analysis/Verifier.cpp
@@ -40,8 +40,7 @@ namespace {
/// This class encapsulates all the state used to verify an operation region.
class OperationVerifier {
public:
- explicit OperationVerifier(MLIRContext *ctx)
- : ctx(ctx), identifierRegex("^[a-zA-Z_][a-zA-Z_0-9\\.\\$]*$") {}
+ explicit OperationVerifier(MLIRContext *ctx) : ctx(ctx) {}
/// Verify the given operation.
LogicalResult verify(Operation &op);
@@ -53,9 +52,6 @@ class OperationVerifier {
return ctx->getRegisteredDialect(dialectNamePair.first);
}
- /// Returns if the given string is valid to use as an identifier name.
- bool isValidName(StringRef name) { return identifierRegex.match(name); }
-
private:
/// Verify the given potentially nested region or block.
LogicalResult verifyRegion(Region ®ion);
@@ -82,9 +78,6 @@ class OperationVerifier {
/// Dominance information for this operation, when checking dominance.
DominanceInfo *domInfo = nullptr;
- /// Regex checker for attribute names.
- llvm::Regex identifierRegex;
-
/// Mapping between dialect namespace and if that dialect supports
/// unregistered operations.
llvm::StringMap<bool> dialectAllowsUnknownOps;
@@ -172,9 +165,6 @@ LogicalResult OperationVerifier::verifyOperation(Operation &op) {
/// Verify that all of the attributes are okay.
for (auto attr : op.getAttrs()) {
- if (!identifierRegex.match(attr.first))
- return op.emitError("invalid attribute name '") << attr.first << "'";
-
// Check for any optional dialect specific attributes.
if (!attr.first.strref().contains('.'))
continue;
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index d24bb505c53f..776792ec2f17 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -892,6 +892,7 @@ class ModulePrinter {
void printOptionalAttrDict(ArrayRef<NamedAttribute> attrs,
ArrayRef<StringRef> elidedAttrs = {},
bool withKeyword = false);
+ void printNamedAttribute(NamedAttribute attr);
void printTrailingLocation(Location loc);
void printLocationInternal(LocationAttr loc, bool pretty = false);
@@ -1244,16 +1245,7 @@ void ModulePrinter::printAttribute(Attribute attr,
case StandardAttributes::Dictionary:
os << '{';
interleaveComma(attr.cast<DictionaryAttr>().getValue(),
- [&](NamedAttribute attr) {
- os << attr.first;
-
- // The value of a UnitAttr is elided within a dictionary.
- if (attr.second.isa<UnitAttr>())
- return;
-
- os << " = ";
- printAttribute(attr.second);
- });
+ [&](NamedAttribute attr) { printNamedAttribute(attr); });
os << '}';
break;
case StandardAttributes::Integer: {
@@ -1618,17 +1610,26 @@ void ModulePrinter::printOptionalAttrDict(ArrayRef<NamedAttribute> attrs,
// Otherwise, print them all out in braces.
os << " {";
- interleaveComma(filteredAttrs, [&](NamedAttribute attr) {
+ interleaveComma(filteredAttrs,
+ [&](NamedAttribute attr) { printNamedAttribute(attr); });
+ os << '}';
+}
+
+void ModulePrinter::printNamedAttribute(NamedAttribute attr) {
+ if (isBareIdentifier(attr.first)) {
os << attr.first;
+ } else {
+ os << '"';
+ printEscapedString(attr.first.strref(), os);
+ os << '"';
+ }
- // Pretty printing elides the attribute value for unit attributes.
- if (attr.second.isa<UnitAttr>())
- return;
+ // Pretty printing elides the attribute value for unit attributes.
+ if (attr.second.isa<UnitAttr>())
+ return;
- os << " = ";
- printAttribute(attr.second);
- });
- os << '}';
+ os << " = ";
+ printAttribute(attr.second);
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp
index 209668adc6f3..d987fd0d8add 100644
--- a/mlir/lib/Parser/Parser.cpp
+++ b/mlir/lib/Parser/Parser.cpp
@@ -1642,7 +1642,7 @@ Attribute Parser::parseAttribute(Type type) {
///
/// attribute-dict ::= `{` `}`
/// | `{` attribute-entry (`,` attribute-entry)* `}`
-/// attribute-entry ::= bare-id `=` attribute-value
+/// attribute-entry ::= (bare-id | string-literal) `=` attribute-value
///
ParseResult
Parser::parseAttributeDict(SmallVectorImpl<NamedAttribute> &attributes) {
@@ -1650,17 +1650,21 @@ Parser::parseAttributeDict(SmallVectorImpl<NamedAttribute> &attributes) {
return failure();
auto parseElt = [&]() -> ParseResult {
- // We allow keywords as attribute names.
- if (getToken().isNot(Token::bare_identifier, Token::inttype) &&
- !getToken().isKeyword())
+ // The name of an attribute can either be a bare identifier, or a string.
+ Optional<Identifier> nameId;
+ if (getToken().is(Token::string))
+ nameId = builder.getIdentifier(getToken().getStringValue());
+ else if (getToken().isAny(Token::bare_identifier, Token::inttype) ||
+ getToken().isKeyword())
+ nameId = builder.getIdentifier(getTokenSpelling());
+ else
return emitError("expected attribute name");
- Identifier nameId = builder.getIdentifier(getTokenSpelling());
consumeToken();
// Try to parse the '=' for the attribute value.
if (!consumeIf(Token::equal)) {
// If there is no '=', we treat this as a unit attribute.
- attributes.push_back({nameId, builder.getUnitAttr()});
+ attributes.push_back({*nameId, builder.getUnitAttr()});
return success();
}
@@ -1668,7 +1672,7 @@ Parser::parseAttributeDict(SmallVectorImpl<NamedAttribute> &attributes) {
if (!attr)
return failure();
- attributes.push_back({nameId, attr});
+ attributes.push_back({*nameId, attr});
return success();
};
diff --git a/mlir/test/IR/parser.mlir b/mlir/test/IR/parser.mlir
index 4a8caf7d3d37..a6dc9b6617b3 100644
--- a/mlir/test/IR/parser.mlir
+++ b/mlir/test/IR/parser.mlir
@@ -1163,6 +1163,10 @@ func @"\"_string_symbol_reference\""() {
return
}
+// CHECK-LABEL: func @string_attr_name
+// CHECK-SAME: {"0 . 0", nested = {"0 . 0"}}
+func @string_attr_name() attributes {"0 . 0", nested = {"0 . 0"}}
+
// CHECK-LABEL: func @nested_reference
// CHECK: ref = @some_symbol::@some_nested_symbol
func @nested_reference() attributes {test.ref = @some_symbol::@some_nested_symbol }
More information about the Mlir-commits
mailing list