Introduce support for parsing pretty dialect types, currently with a very

    restricted grammar.  This will make certain common types much easier to read.

    This is part tensorflow/mlir#1 of 2, which allows us to accept the new syntax.  Part 2 will
    change the asmprinter to automatically use it when appropriate, which will
    require updating a bunch of tests.

    This is motivated by the EuroLLVM tutorial and cleaning up the LLVM dialect aesthetics a bit more.

--

PiperOrigin-RevId: 242234821

diff --git a/mlir/g3doc/LangRef.md b/mlir/g3doc/LangRef.md
index 17edf8e..a89a490 100644 (file)
--- a/mlir/g3doc/LangRef.md
+++ b/mlir/g3doc/LangRef.md
@@ -193,10 +193,10 @@ used in an MLIR text file but are not persisted as part of the IR - the printer
 will give them anonymous names like `%42`.
 
 MLIR guarantees identifiers never collide with keywords by prefixing identifiers
-with a sigil (e.g. `%`, `#`, `@`, `^`). In certain unambiguous contexts (e.g.
-affine expressions), identifiers are not prefixed, for brevity. New keywords may
-be added to future versions of MLIR without danger of collision with existing
-identifiers.
+with a sigil (e.g. `%`, `#`, `@`, `^`, `!`). In certain unambiguous contexts
+(e.g. affine expressions), identifiers are not prefixed, for brevity. New
+keywords may be added to future versions of MLIR without danger of collision
+with existing identifiers.
 
 The scope of SSA values is defined based on the standard definition of
 [dominance](https://en.wikipedia.org/wiki/Dominator_\(graph_theory\)). Argument
@@ -534,7 +534,8 @@ type-alias ::= '!' alias-name
 
 MLIR supports defining named aliases for types. A type alias is an identifier
 that can be used in the place of the type that it defines. These aliases *must*
-be defined before their uses.
+be defined before their uses. Alias names may not contain a '.', since those
+names are reserved for [dialect-specific types](#dialect-specific-types).
 
 Example:
 
@@ -2217,9 +2218,15 @@ system. These extensions fit within the same type system as described in the
 
 ``` {.ebnf}
 dialect-type ::= '!' dialect-namespace '<' '"' type-specific-data '"' '>'
+dialect-type ::= '!' alias-name pretty-dialect-type-body?
+
+pretty-dialect-type-body ::= '<' pretty-dialect-type-contents+ '>'
+pretty-dialect-type-contents ::= pretty-dialect-type-body
+                               | '[0-9a-zA-Z.-]+'
+
 ```
 
-Example:
+Dialect types can be specified in a verbose form, e.g. like this:
 
 ```mlir {.mlir}
 // LLVM type that wraps around llvm IR types.
@@ -2227,8 +2234,31 @@ Example:
 
 // Tensor flow string type.
 !tf<"string">
+
+// Complex type
+!foo<"something<abcd>">
+
+// Even more complex type
+!foo<"something<a%%123^^^>>>">
+```
+
+Dialect types that are simple enough can use the pretty format, which is a
+lighter weight syntax that is equivalent to the above forms:
+
+```mlir {.mlir}
+// Tensor flow string type.
+!tf.string
+
+// Complex type
+!foo.something<abcd>
 ```
 
+Sufficiently complex dialect types are required to use the verbose form for
+generality. For example, the more complex type shown above wouldn't be valid in
+the lighter syntax: `!foo.something<a%%123^^^>>>` because it contains characters
+that are not allowed in the lighter syntax, as well as unbalanced `<>`
+characters.
+
 ### TensorFlow types
 
 The TensorFlow dialect in MLIR defines several extended types:

diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp
index c07cabe..95a760e 100644 (file)
--- a/mlir/lib/Parser/Parser.cpp
+++ b/mlir/lib/Parser/Parser.cpp
@@ -184,6 +184,7 @@ public:
   ParseResult parseDimensionListRanked(SmallVectorImpl<int64_t> &dimensions,
                                        bool allowDynamic);
   Type parseExtendedType();
+  ParseResult parsePrettyDialectTypeName(StringRef &prettyName);
   Type parseTensorType();
   Type parseComplexType();
   Type parseTupleType();
@@ -480,10 +481,51 @@ Parser::parseDimensionListRanked(SmallVectorImpl<int64_t> &dimensions,
   return ParseSuccess;
 }
 
+/// Parse the body of a pretty dialect type, which starts and ends with <>'s,
+/// and may be recursive.  Return with the 'prettyName' StringRef encompasing
+/// the entire pretty name.
+///
+///   pretty-dialect-type-body ::= '<' pretty-dialect-type-contents+ '>'
+///   pretty-dialect-type-contents ::= pretty-dialect-type-body
+///                                  | '[0-9a-zA-Z.-]+'
+///
+ParseResult Parser::parsePrettyDialectTypeName(StringRef &prettyName) {
+  consumeToken(Token::less);
+
+  while (1) {
+    if (getToken().is(Token::greater)) {
+      auto *start = prettyName.begin();
+      // Update the size of the covered range to include all the tokens we have
+      // skipped over.
+      unsigned length = getTokenSpelling().end() - start;
+      prettyName = StringRef(start, length);
+      consumeToken(Token::greater);
+      return ParseSuccess;
+    }
+
+    if (getToken().is(Token::less)) {
+      if (parsePrettyDialectTypeName(prettyName))
+        return ParseFailure;
+      continue;
+    }
+
+    // Check to see if the token contains simple characters.
+    bool isSimple = true;
+    for (auto c : getTokenSpelling())
+      isSimple &= (isalpha(c) || isdigit(c) || c == '.' || c == '-');
+
+    if (!isSimple || getToken().is(Token::eof))
+      return emitError("expected simple name in pretty dialect type");
+
+    consumeToken();
+  }
+}
+
 /// Parse an extended type.
 ///
 ///   extended-type ::= (dialect-type | type-alias)
 ///   dialect-type  ::= `!` dialect-namespace `<` '"' type-data '"' `>`
+///   dialect-type  ::= `!` alias-name pretty-dialect-type-body?
 ///   type-alias    ::= `!` alias-name
 ///
 Type Parser::parseExtendedType() {
@@ -491,10 +533,12 @@ Type Parser::parseExtendedType() {
 
   // Parse the dialect namespace.
   StringRef identifier = getTokenSpelling().drop_front();
+  auto loc = getToken().getLoc();
   consumeToken(Token::exclamation_identifier);
 
-  // If there is not a '<' token, we are parsing a type alias.
-  if (getToken().isNot(Token::less)) {
+  // If there is no '<' token following this, and if the typename contains no
+  // dot, then we are parsing a type alias.
+  if (getToken().isNot(Token::less) && !identifier.contains('.')) {
     // Check for an alias for this type.
     auto aliasIt = state.typeAliasDefinitions.find(identifier);
     if (aliasIt == state.typeAliasDefinitions.end())
@@ -503,38 +547,56 @@ Type Parser::parseExtendedType() {
     return aliasIt->second;
   }
 
-  // Otherwise, we are parsing a dialect-specific type.
-
-  // Consume the '<'.
-  if (parseToken(Token::less, "expected '<' in dialect type"))
-    return nullptr;
-
-  // Parse the type specific data.
-  if (getToken().isNot(Token::string))
-    return (emitError("expected string literal type data in dialect type"),
-            nullptr);
+  // Otherwise, we are parsing a dialect-specific type.  If the name contains a
+  // dot, then this is the "pretty" form.  If not, it is the verbose form that
+  // looks like <"...">.
+  std::string typeData;
+  auto dialectName = identifier;
 
-  auto typeData = getToken().getStringValue();
-  auto loc = getEncodedSourceLocation(getToken().getLoc());
-  consumeToken(Token::string);
+  // Handle the verbose form, where "identifier" is a simple dialect name.
+  if (!identifier.contains('.')) {
+    // Consume the '<'.
+    if (parseToken(Token::less, "expected '<' in dialect type"))
+      return nullptr;
 
-  Type result;
+    // Parse the type specific data.
+    if (getToken().isNot(Token::string))
+      return (emitError("expected string literal type data in dialect type"),
+              nullptr);
+    typeData = getToken().getStringValue();
+    loc = getToken().getLoc();
+    consumeToken(Token::string);
 
-  // If we found a registered dialect, then ask it to parse the type.
-  if (auto *dialect = state.context->getRegisteredDialect(identifier)) {
-    result = dialect->parseType(typeData, loc);
-    if (!result)
+    // Consume the '>'.
+    if (parseToken(Token::greater, "expected '>' in dialect type"))
       return nullptr;
   } else {
-    // Otherwise, form a new opaque type.
-    result = OpaqueType::getChecked(Identifier::get(identifier, state.context),
-                                    typeData, state.context, loc);
+    // Ok, the dialect name is the part of the identifier before the dot, the
+    // part after the dot is the dialect's type, or the start thereof.
+    auto dotHalves = identifier.split('.');
+    dialectName = dotHalves.first;
+    auto prettyName = dotHalves.second;
+
+    // If the dialect's type is followed immediately by a <, then lex the body
+    // of it into prettyName.
+    if (getToken().is(Token::less) &&
+        prettyName.bytes_end() == getTokenSpelling().bytes_begin()) {
+      if (parsePrettyDialectTypeName(prettyName) == ParseFailure)
+        return nullptr;
+    }
+
+    typeData = prettyName.str();
   }
 
-  // Consume the '>'.
-  if (parseToken(Token::greater, "expected '>' in dialect type"))
-    return nullptr;
-  return result;
+  auto encodedLoc = getEncodedSourceLocation(loc);
+
+  // If we found a registered dialect, then ask it to parse the type.
+  if (auto *dialect = state.context->getRegisteredDialect(dialectName))
+    return dialect->parseType(typeData, encodedLoc);
+
+  // Otherwise, form a new opaque type.
+  return OpaqueType::getChecked(Identifier::get(dialectName, state.context),
+                                typeData, state.context, encodedLoc);
 }
 
 /// Parse a tensor type.
@@ -3548,6 +3610,11 @@ ParseResult ModuleParser::parseTypeAliasDef() {
   if (getState().typeAliasDefinitions.count(aliasName) > 0)
     return emitError("redefinition of type alias id '" + aliasName + "'");
 
+  // Make sure this isn't invading the dialect type namespace.
+  if (aliasName.contains('.'))
+    return emitError("type names with a '.' are reserved for "
+                     "dialect-defined names");
+
   consumeToken(Token::exclamation_identifier);
 
   // Parse the '=' and 'type'.

diff --git a/mlir/test/IR/invalid.mlir b/mlir/test/IR/invalid.mlir
index ca2a79b..eb97289 100644 (file)
--- a/mlir/test/IR/invalid.mlir
+++ b/mlir/test/IR/invalid.mlir
@@ -787,6 +787,11 @@ func @type_alias_unknown(!unknown_alias) -> () { // expected-error {{undefined t
 
 // -----
 
+// expected-error @+1 {{type names with a '.' are reserved for dialect-defined names}}
+!foo.bar = i32
+
+// -----
+
 !missing_eq_alias type i32 // expected-error {{expected '=' in type alias definition}}
 
 // -----
@@ -922,8 +927,8 @@ func @invalid_nested_dominance() {
 
 // -----
 
-// expected-error @+1 {{invalid dialect namespace 'invalid.dialect'}}
-func @invalid_unknown_type_dialect_name() -> !invalid.dialect<"">
+// expected-error @+1 {{expected simple name in pretty dialect type}}
+func @invalid_unknown_type_dialect_name() -> !invalid.dialect<!x@#!@#>
 
 // -----
 

diff --git a/mlir/test/IR/parser.mlir b/mlir/test/IR/parser.mlir
index 37aabb3..a27c9d3 100644 (file)
--- a/mlir/test/IR/parser.mlir
+++ b/mlir/test/IR/parser.mlir
@@ -850,3 +850,17 @@ func @pretty_form_multi_result() -> (i16, i16) {
   %quot, %rem = "foo_div"() : () -> (i16, i16)
   return %quot, %rem : i16, i16
 }
+
+// CHECK-LABEL: func @pretty_dialect_type()
+func @pretty_dialect_type() {
+
+  // CHECK: %0 = "foo.unknown_op"() : () -> !foo<"simpletype">
+  %0 = "foo.unknown_op"() : () -> !foo.simpletype
+
+  // CHECK: %1 = "foo.unknown_op"() : () -> !foo<"complextype<abcd>">
+  %1 = "foo.unknown_op"() : () -> !foo.complextype<abcd>
+
+  // CHECK: %2 = "foo.unknown_op"() : () -> !foo<"complextype<abcd<f32>>">
+  %2 = "foo.unknown_op"() : () -> !foo.complextype<abcd<f32>>
+  return
+}