diff --git a/mlir/docs/LangRef.md b/mlir/docs/LangRef.md --- a/mlir/docs/LangRef.md +++ b/mlir/docs/LangRef.md @@ -1156,7 +1156,8 @@ 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 --- a/mlir/lib/Analysis/Verifier.cpp +++ b/mlir/lib/Analysis/Verifier.cpp @@ -40,8 +40,7 @@ /// 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 @@ 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 @@ /// 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 dialectAllowsUnknownOps; @@ -172,9 +165,6 @@ /// 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 --- a/mlir/lib/IR/AsmPrinter.cpp +++ b/mlir/lib/IR/AsmPrinter.cpp @@ -892,6 +892,7 @@ void printOptionalAttrDict(ArrayRef attrs, ArrayRef elidedAttrs = {}, bool withKeyword = false); + void printNamedAttribute(NamedAttribute attr); void printTrailingLocation(Location loc); void printLocationInternal(LocationAttr loc, bool pretty = false); @@ -1244,16 +1245,7 @@ case StandardAttributes::Dictionary: os << '{'; interleaveComma(attr.cast().getValue(), - [&](NamedAttribute attr) { - os << attr.first; - - // The value of a UnitAttr is elided within a dictionary. - if (attr.second.isa()) - return; - - os << " = "; - printAttribute(attr.second); - }); + [&](NamedAttribute attr) { printNamedAttribute(attr); }); os << '}'; break; case StandardAttributes::Integer: { @@ -1618,17 +1610,26 @@ // 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()) - return; + // Pretty printing elides the attribute value for unit attributes. + if (attr.second.isa()) + return; - os << " = "; - printAttribute(attr.second); - }); - os << '}'; + os << " = "; + printAttribute(attr.second); } //===----------------------------------------------------------------------===// diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp --- a/mlir/lib/Parser/Parser.cpp +++ b/mlir/lib/Parser/Parser.cpp @@ -1642,7 +1642,7 @@ /// /// attribute-dict ::= `{` `}` /// | `{` attribute-entry (`,` attribute-entry)* `}` -/// attribute-entry ::= bare-id `=` attribute-value +/// attribute-entry ::= (bare-id | string-literal) `=` attribute-value /// ParseResult Parser::parseAttributeDict(SmallVectorImpl &attributes) { @@ -1650,17 +1650,21 @@ 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 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 @@ 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 --- a/mlir/test/IR/parser.mlir +++ b/mlir/test/IR/parser.mlir @@ -1163,6 +1163,10 @@ 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 }