[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

Author: River707

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

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 &region);
@@ -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;

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);
 }
 
 //===----------------------------------------------------------------------===//

mlir/lib/Parser/Parser.cpp

@@ -1642,33 +1642,37 @@ 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) {
   if (parseToken(Token::l_brace, "expected '{' in attribute dictionary"))
     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();
     }
 
     auto attr = parseAttribute();
     if (!attr)
       return failure();
 
-    attributes.push_back({nameId, attr});
+    attributes.push_back({*nameId, attr});
     return success();
   };
 

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 }