diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td --- a/mlir/test/lib/Dialect/Test/TestOps.td +++ b/mlir/test/lib/Dialect/Test/TestOps.td @@ -1374,7 +1374,7 @@ def FormatLiteralOp : TEST_Op<"format_literal_op"> { let assemblyFormat = [{ - `keyword_$.` `->` `:` `,` `=` `<` `>` `(` `)` `[` `]` attr-dict + `keyword_$.` `->` `:` `,` `=` `<` `>` `(` `)` `[` `]` ` ` `(` ` ` `)` ` ` attr-dict }]; } diff --git a/mlir/test/mlir-tblgen/op-format-spec.td b/mlir/test/mlir-tblgen/op-format-spec.td --- a/mlir/test/mlir-tblgen/op-format-spec.td +++ b/mlir/test/mlir-tblgen/op-format-spec.td @@ -309,7 +309,7 @@ }]>; // CHECK-NOT: error def LiteralValid : TestFormat_Op<"literal_valid", [{ - `_` `:` `,` `=` `<` `>` `(` `)` `[` `]` `->` `abc$._` + `_` `:` `,` `=` `<` `>` `(` `)` `[` `]` ` ` `->` `abc$._` attr-dict }]>; @@ -365,6 +365,10 @@ def OptionalInvalidL : TestFormat_Op<"optional_invalid_l", [{ (custom($arg)^)? }]>, Arguments<(ins I64:$arg)>; +// CHECK: error: only variables can be used to anchor an optional group +def OptionalInvalidM : TestFormat_Op<"optional_invalid_m", [{ + (` `^)? +}]>, Arguments<(ins)>; //===----------------------------------------------------------------------===// // Variables @@ -410,26 +414,30 @@ def VariableInvalidI : TestFormat_Op<"variable_invalid_i", [{ (`foo` $attr^)? `:` attr-dict }]>, Arguments<(ins OptionalAttr:$attr)>; -// CHECK: error: region 'region' is already bound +// CHECK: error: format ambiguity caused by `:` literal found after attribute `attr` which does not have a buildable type def VariableInvalidJ : TestFormat_Op<"variable_invalid_j", [{ + $attr ` ` `:` attr-dict +}]>, Arguments<(ins ElementsAttr:$attr)>; +// CHECK: error: region 'region' is already bound +def VariableInvalidK : TestFormat_Op<"variable_invalid_k", [{ $region $region attr-dict }]> { let regions = (region AnyRegion:$region); } // CHECK: error: region 'region' is already bound -def VariableInvalidK : TestFormat_Op<"variable_invalid_K", [{ +def VariableInvalidL : TestFormat_Op<"variable_invalid_l", [{ regions $region attr-dict }]> { let regions = (region AnyRegion:$region); } // CHECK: error: regions can only be used at the top level -def VariableInvalidL : TestFormat_Op<"variable_invalid_l", [{ +def VariableInvalidM : TestFormat_Op<"variable_invalid_m", [{ type($region) }]> { let regions = (region AnyRegion:$region); } // CHECK: error: region #0, named 'region', not found -def VariableInvalidM : TestFormat_Op<"variable_invalid_m", [{ +def VariableInvalidN : TestFormat_Op<"variable_invalid_n", [{ attr-dict }]> { let regions = (region AnyRegion:$region); diff --git a/mlir/test/mlir-tblgen/op-format.mlir b/mlir/test/mlir-tblgen/op-format.mlir --- a/mlir/test/mlir-tblgen/op-format.mlir +++ b/mlir/test/mlir-tblgen/op-format.mlir @@ -5,8 +5,8 @@ // CHECK: %[[MEMREF:.*]] = %memref = "foo.op"() : () -> (memref<1xf64>) -// CHECK: test.format_literal_op keyword_$. -> :, = <> () [] {foo.some_attr} -test.format_literal_op keyword_$. -> :, = <> () [] {foo.some_attr} +// CHECK: test.format_literal_op keyword_$. -> :, = <> () [] ( ) {foo.some_attr} +test.format_literal_op keyword_$. -> :, = <> () [] ( ) {foo.some_attr} // CHECK: test.format_attr_op 10 // CHECK-NOT: {attr diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp --- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp +++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp @@ -58,6 +58,9 @@ /// This element is a literal. Literal, + /// This element is printed as a space. It is ignored by the parser. + Space, + /// This element is an variable value. AttributeVariable, OperandVariable, @@ -292,6 +295,21 @@ }); } +//===----------------------------------------------------------------------===// +// SpaceElement + +namespace { +/// This class represents an instance of a space element. It's a literal that +/// is only printed, but ignored by the parser. +class SpaceElement : public Element { +public: + SpaceElement() : Element{Kind::Space} {} + static bool classof(const Element *element) { + return element->getKind() == Kind::Space; + } +}; +} // end anonymous namespace + //===----------------------------------------------------------------------===// // OptionalElement @@ -1060,6 +1078,10 @@ genLiteralParser(literal->getLiteral(), body); body << ")\n return ::mlir::failure();\n"; + /// Spaces. + } else if (isa(element)) { + // Nothing to parse. + /// Arguments. } else if (auto *attr = dyn_cast(element)) { const NamedAttribute *var = attr->getVar(); @@ -1459,7 +1481,7 @@ return !StringRef("<>(){}[],").contains(value.front()); }; if (shouldEmitSpace && shouldPrintSpaceBeforeLiteral()) - body << " << \" \""; + body << " << ' '"; body << " << \"" << value << "\";\n"; // Insert a space after certain literals. @@ -1468,9 +1490,16 @@ lastWasPunctuation = !(value.front() == '_' || isalpha(value.front())); } -/// Generate the printer for a literal value. `shouldEmitSpace` is true if a -/// space should be emitted before this element. `lastWasPunctuation` is true if -/// the previous element was a punctuation literal. +/// Generate the printer for a space. `shouldEmitSpace` and `lastWasPunctuation` +/// are set to false. +static void genSpacePrinter(OpMethodBody &body, bool &shouldEmitSpace, + bool &lastWasPunctuation) { + body << " p << ' ';\n"; + shouldEmitSpace = false; + lastWasPunctuation = false; +} + +/// Generate the printer for a custom directive. static void genCustomDirectivePrinter(CustomDirective *customDir, OpMethodBody &body) { body << " print" << customDir->getName() << "(p"; @@ -1562,6 +1591,9 @@ return genLiteralPrinter(literal->getLiteral(), body, shouldEmitSpace, lastWasPunctuation); + if (isa(element)) + return genSpacePrinter(body, shouldEmitSpace, lastWasPunctuation); + // Emit an optional group. if (OptionalElement *optional = dyn_cast(element)) { // Emit the check for the presence of the anchor element. @@ -1613,7 +1645,7 @@ // Optionally insert a space before the next element. The AttrDict printer // already adds a space as necessary. if (shouldEmitSpace || !lastWasPunctuation) - body << " p << \" \";\n"; + body << " p << ' ';\n"; lastWasPunctuation = false; shouldEmitSpace = true; @@ -1623,8 +1655,8 @@ // If we are formatting as an enum, symbolize the attribute as a string. if (canFormatEnumAttr(var)) { const EnumAttr &enumAttr = cast(var->attr); - body << " p << \"\\\"\" << " << enumAttr.getSymbolToStringFnName() << "(" - << var->name << "()) << \"\\\"\";\n"; + body << " p << '\"' << " << enumAttr.getSymbolToStringFnName() << "(" + << var->name << "()) << '\"';\n"; return; } @@ -2208,8 +2240,9 @@ for (auto &nextItPair : iteratorStack) { ElementsIterT nextIt = nextItPair.first, nextE = nextItPair.second; for (; nextIt != nextE; ++nextIt) { - // Skip any trailing optional groups or attribute dictionaries. - if (isa(*nextIt) || isa(*nextIt)) + // Skip any trailing spaces, attribute dictionaries, or optional groups. + if (isa(*nextIt) || isa(*nextIt) || + isa(*nextIt)) continue; // We are only interested in `:` literals. @@ -2528,8 +2561,15 @@ Token literalTok = curToken; consumeToken(); - // Check that the parsed literal is valid. StringRef value = literalTok.getSpelling().drop_front().drop_back(); + + // The parsed literal is a space. + if (value.size() == 1 && value.front() == ' ') { + element = std::make_unique(); + return ::mlir::success(); + } + + // Check that the parsed literal is valid. if (!LiteralElement::isValidLiteral(value)) return emitError(literalTok.getLoc(), "expected valid literal"); @@ -2641,10 +2681,11 @@ // a check here. return ::mlir::success(); }) - // Literals, custom directives, and type directives may be used, + // Literals, spaces, custom directives, and type directives may be used, // but they can't anchor the group. - .Case([&](Element *) { + .Case([&](Element *) { if (isAnchor) return emitError(childLoc, "only variables can be used to anchor " "an optional group");