Page MenuHomePhabricator
Differential D134993 Diff 467872 mlir/tools/mlir-tblgen/OpFormatGen.cpp

Changeset View

Standalone View

View Options

mlir/tools/mlir-tblgen/OpFormatGen.cpp

Show First 20 Lines • Show All 1,076 Lines • ▼ Show 20 Lines if (!var->attr.isOptional()) {
errorMessageOS << "]\");"; errorMessageOS << "]\");";
} }
body << formatv(enumAttrParserCode, var->name, enumAttr.getCppNamespace(), body << formatv(enumAttrParserCode, var->name, enumAttr.getCppNamespace(),
enumAttr.getStringToSymbolFnName(), attrBuilderStr, enumAttr.getStringToSymbolFnName(), attrBuilderStr,
validCaseKeywordsStr, errorMessage); validCaseKeywordsStr, errorMessage);
} }
// Generate the parser for an attribute.
static void genAttrParser(AttributeVariable *attr, MethodBody &body,
FmtContext &attrTypeCtx, bool parseAsOptional) {
const NamedAttribute *var = attr->getVar();
// Check to see if we can parse this as an enum attribute.
if (canFormatEnumAttr(var))
return genEnumAttrParser(var, body, attrTypeCtx);
// Check to see if we should parse this as a symbol name attribute.
if (shouldFormatSymbolNameAttr(var)) {
body << formatv(var->attr.isOptional() ? optionalSymbolNameAttrParserCode
rkayaithAuthorUnsubmitted
Done
ReplyInline Actions

on main genCtx ends up being ignored on this path, should this be parseAsOptional instead?

and similarly for the isOptional() check inside genEnumAttrParser

rkayaith: on main `genCtx` ends up being ignored on this path, should this be `parseAsOptional` instead?
rkayaithAuthorUnsubmitted
Done
ReplyInline Actions

i switched these over to use parseAsOptional; otherwise default-valued symbols and enums break

rkayaith: i switched these over to use `parseAsOptional`; otherwise default-valued symbols and enums break
: symbolNameAttrParserCode,
var->name);
return;
}
// If this attribute has a buildable type, use that when parsing the
// attribute.
std::string attrTypeStr;
if (Optional<StringRef> typeBuilder = attr->getTypeBuilder()) {
llvm::raw_string_ostream os(attrTypeStr);
os << tgfmt(*typeBuilder, &attrTypeCtx);
} else {
attrTypeStr = "::mlir::Type{}";
}
if (parseAsOptional) {
body << formatv(optionalAttrParserCode, var->name, attrTypeStr);
} else {
if (attr->shouldBeQualified() ||
var->attr.getStorageType() == "::mlir::Attribute")
body << formatv(genericAttrParserCode, var->name, attrTypeStr);
else
body << formatv(attrParserCode, var->name, attrTypeStr);
}
}
void OperationFormat::genParser(Operator &op, OpClass &opClass) { void OperationFormat::genParser(Operator &op, OpClass &opClass) {
SmallVector<MethodParameter> paramList; SmallVector<MethodParameter> paramList;
paramList.emplace_back("::mlir::OpAsmParser &", "parser"); paramList.emplace_back("::mlir::OpAsmParser &", "parser");
paramList.emplace_back("::mlir::OperationState &", "result"); paramList.emplace_back("::mlir::OperationState &", "result");
auto *method = opClass.addStaticMethod("::mlir::ParseResult", "parse", auto *method = opClass.addStaticMethod("::mlir::ParseResult", "parse",
std::move(paramList)); std::move(paramList));
auto &body = method->body(); auto &body = method->body();
▲ Show 20 Lines • Show All 55 Lines • ▼ Show 20 Lines if (auto *optional = dyn_cast<OptionalElement>(element)) {
ArrayRef<FormatElement *> thenElements = ArrayRef<FormatElement *> thenElements =
optional->getThenElements(/*parseable=*/true); optional->getThenElements(/*parseable=*/true);
// Generate a special optional parser for the first element to gate the // Generate a special optional parser for the first element to gate the
// parsing of the rest of the elements. // parsing of the rest of the elements.
FormatElement *firstElement = thenElements.front(); FormatElement *firstElement = thenElements.front();
if (auto *attrVar = dyn_cast<AttributeVariable>(firstElement)) { if (auto *attrVar = dyn_cast<AttributeVariable>(firstElement)) {
genElementParser(attrVar, body, attrTypeCtx); genAttrParser(attrVar, body, attrTypeCtx, /*parseAsOptional=*/true);
body << " if (" << attrVar->getVar()->name << "Attr) {\n"; body << " if (" << attrVar->getVar()->name << "Attr) {\n";
} else if (auto *literal = dyn_cast<LiteralElement>(firstElement)) { } else if (auto *literal = dyn_cast<LiteralElement>(firstElement)) {
body << " if (::mlir::succeeded(parser.parseOptional"; body << " if (::mlir::succeeded(parser.parseOptional";
genLiteralParser(literal->getSpelling(), body); genLiteralParser(literal->getSpelling(), body);
body << ")) {\n"; body << ")) {\n";
} else if (auto *opVar = dyn_cast<OperandVariable>(firstElement)) { } else if (auto *opVar = dyn_cast<OperandVariable>(firstElement)) {
genElementParser(opVar, body, attrTypeCtx); genElementParser(opVar, body, attrTypeCtx);
body << " if (!" << opVar->getVar()->name << "Operands.empty()) {\n"; body << " if (!" << opVar->getVar()->name << "Operands.empty()) {\n";
▲ Show 20 Lines • Show All 65 Lines • ▼ Show 20 Lines if (auto *optional = dyn_cast<OptionalElement>(element)) {
genLiteralParser(literal->getSpelling(), body); genLiteralParser(literal->getSpelling(), body);
body << ")\n return ::mlir::failure();\n"; body << ")\n return ::mlir::failure();\n";
/// Whitespaces. /// Whitespaces.
} else if (isa<WhitespaceElement>(element)) { } else if (isa<WhitespaceElement>(element)) {
// Nothing to parse. // Nothing to parse.
/// Arguments. /// Arguments.
} else if (auto *attr = dyn_cast<AttributeVariable>(element)) { } else if (auto *attr = dyn_cast<AttributeVariable>(element)) {
const NamedAttribute *var = attr->getVar(); bool parseAsOptional =
rkayaithAuthorUnsubmitted
Done
ReplyInline Actions

I didn't really understand the existing logic, it seemed odd to me to parse as optional under Normal context, but not under Optional context, but maybe I'm misunderstanding what "context" means here. I switched the use of GenContext so that genContext == Optional whenever the attribute should be optionally parsed (currently this is only for the first element of the then group).

rkayaith: I didn't really understand the existing logic, it seemed odd to me to parse as optional under…
MogballUnsubmitted
Not Done
ReplyInline Actions

GenContext::Optional means the element is inside an optional group, not whether the element should be optionally parsed.

Mogball: `GenContext::Optional` means the element is inside an optional group, not whether the element…
rkayaithAuthorUnsubmitted
Done
ReplyInline Actions

I see, is there supposed to be a difference to how things are parsed in optional groups (other than the first element)? The only place I see genCtx used is this line, and it seems like it's allowing optional attributes to be optionally parsed when outside an optional group, but that just ends up breaking in the printer: https://github.com/llvm/llvm-project/issues/58064

rkayaith: I see, is there supposed to be a difference to how things are parsed in optional groups (other…
MogballUnsubmitted
Not Done
ReplyInline Actions

Not really. Only that certain elements are prohibited in optional groups (e.g. nested optional groups). The issue you linked just looks like a bug to me. I think the feature you are adding is useful but I'd prefer not to fiddle with the gen context

Mogball: Not really. Only that certain elements are prohibited in optional groups (e.g. nested optional…
rkayaithAuthorUnsubmitted
Done
ReplyInline Actions

ok, updated the diff and tried to preserve existing genCtx logic

rkayaith: ok, updated the diff and tried to preserve existing genCtx logic
(genCtx == GenContext::Normal && attr->getVar()->attr.isOptional());
// Check to see if we can parse this as an enum attribute. genAttrParser(attr, body, attrTypeCtx, parseAsOptional);
if (canFormatEnumAttr(var))
return genEnumAttrParser(var, body, attrTypeCtx);
// Check to see if we should parse this as a symbol name attribute.
if (shouldFormatSymbolNameAttr(var)) {
body << formatv(var->attr.isOptional() ? optionalSymbolNameAttrParserCode
: symbolNameAttrParserCode,
var->name);
return;
}
// If this attribute has a buildable type, use that when parsing the
// attribute.
std::string attrTypeStr;
if (Optional<StringRef> typeBuilder = attr->getTypeBuilder()) {
llvm::raw_string_ostream os(attrTypeStr);
os << tgfmt(*typeBuilder, &attrTypeCtx);
} else {
attrTypeStr = "::mlir::Type{}";
}
if (genCtx == GenContext::Normal && var->attr.isOptional()) {
body << formatv(optionalAttrParserCode, var->name, attrTypeStr);
} else {
if (attr->shouldBeQualified() ||
var->attr.getStorageType() == "::mlir::Attribute")
body << formatv(genericAttrParserCode, var->name, attrTypeStr);
else
body << formatv(attrParserCode, var->name, attrTypeStr);
}
} else if (auto *operand = dyn_cast<OperandVariable>(element)) { } else if (auto *operand = dyn_cast<OperandVariable>(element)) {
ArgumentLengthKind lengthKind = getArgumentLengthKind(operand->getVar()); ArgumentLengthKind lengthKind = getArgumentLengthKind(operand->getVar());
StringRef name = operand->getVar()->name; StringRef name = operand->getVar()->name;
if (lengthKind == ArgumentLengthKind::VariadicOfVariadic) if (lengthKind == ArgumentLengthKind::VariadicOfVariadic)
body << llvm::formatv( body << llvm::formatv(
variadicOfVariadicOperandParserCode, name, variadicOfVariadicOperandParserCode, name,
operand->getVar()->constraint.getVariadicOfVariadicSegmentSizeAttr()); operand->getVar()->constraint.getVariadicOfVariadicSegmentSizeAttr());
▲ Show 20 Lines • Show All 588 Lines • ▼ Show 20 Lines TypeSwitch<FormatElement *>(anchor)
body << "!" << name << "().empty()"; body << "!" << name << "().empty()";
}) })
.Case<TypeDirective>([&](TypeDirective *element) { .Case<TypeDirective>([&](TypeDirective *element) {
genOptionalGroupPrinterAnchor(element->getArg(), op, body); genOptionalGroupPrinterAnchor(element->getArg(), op, body);
}) })
.Case<FunctionalTypeDirective>([&](FunctionalTypeDirective *element) { .Case<FunctionalTypeDirective>([&](FunctionalTypeDirective *element) {
genOptionalGroupPrinterAnchor(element->getInputs(), op, body); genOptionalGroupPrinterAnchor(element->getInputs(), op, body);
}) })
.Case<AttributeVariable>([&](AttributeVariable *attr) { .Case<AttributeVariable>([&](AttributeVariable *element) {
body << "(*this)->getAttr(\"" << attr->getVar()->name << "\")"; Attribute attr = element->getVar()->attr;
body << "(*this)->getAttr(\"" << element->getVar()->name << "\")";
if (attr.isOptional())
return; // done
if (attr.hasDefaultValue()) {
// Consider a default-valued attribute as present if it's not the
// default value.
FmtContext fctx;
fctx.withBuilder("::mlir::OpBuilder(getContext())");
body << " != "
<< tgfmt(attr.getConstBuilderTemplate(), &fctx,
attr.getDefaultValue());
return;
}
llvm_unreachable("attribute must be optional or default-valued");
}); });
} }
void collect(FormatElement *element, void collect(FormatElement *element,
SmallVectorImpl<VariableElement *> &variables) { SmallVectorImpl<VariableElement *> &variables) {
TypeSwitch<FormatElement *>(element) TypeSwitch<FormatElement *>(element)
.Case([&](VariableElement *var) { variables.emplace_back(var); }) .Case([&](VariableElement *var) { variables.emplace_back(var); })
.Case([&](CustomDirective *ele) { .Case([&](CustomDirective *ele) {
▲ Show 20 Lines • Show All 1,295 Lines • ▼ Show 20 Lines
LogicalResult OpFormatParser::verifyOptionalGroupElement(SMLoc loc, LogicalResult OpFormatParser::verifyOptionalGroupElement(SMLoc loc,
FormatElement *element, FormatElement *element,
bool isAnchor) { bool isAnchor) {
return TypeSwitch<FormatElement *, LogicalResult>(element) return TypeSwitch<FormatElement *, LogicalResult>(element)
// All attributes can be within the optional group, but only optional // All attributes can be within the optional group, but only optional
// attributes can be the anchor. // attributes can be the anchor.
.Case([&](AttributeVariable *attrEle) { .Case([&](AttributeVariable *attrEle) {
if (isAnchor && !attrEle->getVar()->attr.isOptional()) Attribute attr = attrEle->getVar()->attr;
return emitError(loc, "only optional attributes can be used to " if (isAnchor && !(attr.isOptional() || attr.hasDefaultValue()))
"anchor an optional group"); return emitError(loc, "only optional or default-valued attributes "
"can be used to anchor an optional group");
return success(); return success();
}) })
// Only optional-like(i.e. variadic) operands can be within an optional // Only optional-like(i.e. variadic) operands can be within an optional
// group. // group.
.Case([&](OperandVariable *ele) { .Case([&](OperandVariable *ele) {
if (!ele->getVar()->isVariableLength()) if (!ele->getVar()->isVariableLength())
return emitError(loc, "only variable length operands can be used " return emitError(loc, "only variable length operands can be used "
"within an optional group"); "within an optional group");
▲ Show 20 Lines • Show All 74 Lines • Show Last 20 Lines