[Mlir-commits] [mlir] f9b8a0b - [mlir] Allow space literals (` `) in assemblyFormat.
Christian Sigg
llvmlistbot at llvm.org
Sun Oct 18 22:25:38 PDT 2020
Author: Christian Sigg
Date: 2020-10-19T07:25:28+02:00
New Revision: f9b8a0b96b47343a025939db588f260ed6d7b4e3
URL: https://github.com/llvm/llvm-project/commit/f9b8a0b96b47343a025939db588f260ed6d7b4e3
DIFF: https://github.com/llvm/llvm-project/commit/f9b8a0b96b47343a025939db588f260ed6d7b4e3.diff
LOG: [mlir] Allow space literals (` `) in assemblyFormat.
Spaces are only printed, not parsed.
Reviewed By: rriddle
Differential Revision: https://reviews.llvm.org/D89585
Added:
Modified:
mlir/test/lib/Dialect/Test/TestOps.td
mlir/test/mlir-tblgen/op-format-spec.td
mlir/test/mlir-tblgen/op-format.mlir
mlir/tools/mlir-tblgen/OpFormatGen.cpp
Removed:
################################################################################
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 048400fcbe81..20ae5b92e742 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -1374,7 +1374,7 @@ def AsmDialectInterfaceOp : TEST_Op<"asm_dialect_interface_op"> {
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
index 0addff9f35fb..566204069b64 100644
--- a/mlir/test/mlir-tblgen/op-format-spec.td
+++ b/mlir/test/mlir-tblgen/op-format-spec.td
@@ -309,7 +309,7 @@ def LiteralInvalidB : TestFormat_Op<"literal_invalid_b", [{
}]>;
// CHECK-NOT: error
def LiteralValid : TestFormat_Op<"literal_valid", [{
- `_` `:` `,` `=` `<` `>` `(` `)` `[` `]` `->` `abc$._`
+ `_` `:` `,` `=` `<` `>` `(` `)` `[` `]` ` ` `->` `abc$._`
attr-dict
}]>;
@@ -365,6 +365,10 @@ def OptionalInvalidK : TestFormat_Op<"optional_invalid_k", [{
def OptionalInvalidL : TestFormat_Op<"optional_invalid_l", [{
(custom<MyDirective>($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 VariableInvalidH : TestFormat_Op<"variable_invalid_h", [{
def VariableInvalidI : TestFormat_Op<"variable_invalid_i", [{
(`foo` $attr^)? `:` attr-dict
}]>, Arguments<(ins OptionalAttr<ElementsAttr>:$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
index 57360e31ebfb..9a710a621490 100644
--- 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
index 9b8f24923240..01acd591b6fe 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -58,6 +58,9 @@ class Element {
/// 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 @@ bool LiteralElement::isValidLiteral(StringRef value) {
});
}
+//===----------------------------------------------------------------------===//
+// 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 @@ void OperationFormat::genElementParser(Element *element, OpMethodBody &body,
genLiteralParser(literal->getLiteral(), body);
body << ")\n return ::mlir::failure();\n";
+ /// Spaces.
+ } else if (isa<SpaceElement>(element)) {
+ // Nothing to parse.
+
/// Arguments.
} else if (auto *attr = dyn_cast<AttributeVariable>(element)) {
const NamedAttribute *var = attr->getVar();
@@ -1459,7 +1481,7 @@ static void genLiteralPrinter(StringRef value, OpMethodBody &body,
return !StringRef("<>(){}[],").contains(value.front());
};
if (shouldEmitSpace && shouldPrintSpaceBeforeLiteral())
- body << " << \" \"";
+ body << " << ' '";
body << " << \"" << value << "\";\n";
// Insert a space after certain literals.
@@ -1468,9 +1490,16 @@ static void genLiteralPrinter(StringRef value, OpMethodBody &body,
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 @@ void OperationFormat::genElementPrinter(Element *element, OpMethodBody &body,
return genLiteralPrinter(literal->getLiteral(), body, shouldEmitSpace,
lastWasPunctuation);
+ if (isa<SpaceElement>(element))
+ return genSpacePrinter(body, shouldEmitSpace, lastWasPunctuation);
+
// Emit an optional group.
if (OptionalElement *optional = dyn_cast<OptionalElement>(element)) {
// Emit the check for the presence of the anchor element.
@@ -1613,7 +1645,7 @@ void OperationFormat::genElementPrinter(Element *element, OpMethodBody &body,
// 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 @@ void OperationFormat::genElementPrinter(Element *element, OpMethodBody &body,
// If we are formatting as an enum, symbolize the attribute as a string.
if (canFormatEnumAttr(var)) {
const EnumAttr &enumAttr = cast<EnumAttr>(var->attr);
- body << " p << \"\\\"\" << " << enumAttr.getSymbolToStringFnName() << "("
- << var->name << "()) << \"\\\"\";\n";
+ body << " p << '\"' << " << enumAttr.getSymbolToStringFnName() << "("
+ << var->name << "()) << '\"';\n";
return;
}
@@ -2208,8 +2240,9 @@ LogicalResult FormatParser::verifyAttributes(
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<AttrDictDirective>(*nextIt) || isa<OptionalElement>(*nextIt))
+ // Skip any trailing spaces, attribute dictionaries, or optional groups.
+ if (isa<SpaceElement>(*nextIt) || isa<AttrDictDirective>(*nextIt) ||
+ isa<OptionalElement>(*nextIt))
continue;
// We are only interested in `:` literals.
@@ -2528,8 +2561,15 @@ LogicalResult FormatParser::parseLiteral(std::unique_ptr<Element> &element) {
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<SpaceElement>();
+ 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 @@ LogicalResult FormatParser::parseOptionalChildElement(
// 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<LiteralElement, CustomDirective, FunctionalTypeDirective,
- OptionalElement, TypeRefDirective, TypeDirective>([&](Element *) {
+ .Case<LiteralElement, SpaceElement, CustomDirective,
+ FunctionalTypeDirective, OptionalElement, TypeRefDirective,
+ TypeDirective>([&](Element *) {
if (isAnchor)
return emitError(childLoc, "only variables can be used to anchor "
"an optional group");
More information about the Mlir-commits
mailing list