Skip to content

[mlir] Fix the emission of prop-dict when operations have no properties #112851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions mlir/test/IR/properties.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ test.with_nice_properties "foo bar" is -3
// GENERIC-SAME: <{prop = "content for properties"}> : () -> ()
test.with_wrapped_properties <{prop = "content for properties"}>

// CHECK: test.empty_properties
// GENERIC: "test.empty_properties"()
test.empty_properties

// CHECK: test.using_property_in_custom
// CHECK-SAME: [1, 4, 20]{{$}}
// GENERIC: "test.using_property_in_custom"()
Expand Down
5 changes: 5 additions & 0 deletions mlir/test/lib/Dialect/Test/TestOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -3006,6 +3006,11 @@ def TestOpWithWrappedProperties : TEST_Op<"with_wrapped_properties"> {
);
}

def TestOpWithEmptyProperties : TEST_Op<"empty_properties"> {
let assemblyFormat = "prop-dict attr-dict";
let arguments = (ins);
}

def TestOpUsingPropertyInCustom : TEST_Op<"using_property_in_custom"> {
let assemblyFormat = "custom<UsingPropertyInCustom>($prop) attr-dict";
let arguments = (ins IntArrayProperty<"int64_t">:$prop);
Expand Down
2 changes: 1 addition & 1 deletion mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,7 @@ OpEmitter::OpEmitter(const Operator &op,
genFolderDecls();
genTypeInterfaceMethods();
genOpInterfaceMethods();
generateOpFormat(op, opClass);
generateOpFormat(op, opClass, emitHelper.hasProperties());
genSideEffectInterfaceMethods();
}
void OpEmitter::emitDecl(
Expand Down
31 changes: 17 additions & 14 deletions mlir/tools/mlir-tblgen/OpFormatGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,10 +339,8 @@ struct OperationFormat {
Optional
};

OperationFormat(const Operator &op)
: useProperties(op.getDialect().usePropertiesForAttributes() &&
!op.getAttributes().empty()),
opCppClassName(op.getCppClassName()) {
OperationFormat(const Operator &op, bool hasProperties)
: useProperties(hasProperties), opCppClassName(op.getCppClassName()) {
operandTypes.resize(op.getNumOperands(), TypeResolution());
resultTypes.resize(op.getNumResults(), TypeResolution());

Expand Down Expand Up @@ -397,7 +395,7 @@ struct OperationFormat {
/// A flag indicating if this operation has the SingleBlock trait.
bool hasSingleBlockTrait;

/// Indicate whether attribute are stored in properties.
/// Indicate whether we need to use properties for the current operator.
bool useProperties;

/// Indicate whether prop-dict is used in the format
Expand Down Expand Up @@ -1275,8 +1273,8 @@ static void genAttrParser(AttributeVariable *attr, MethodBody &body,
// 'prop-dict' dictionary attr.
static void genParsedAttrPropertiesSetter(OperationFormat &fmt, Operator &op,
OpClass &opClass) {
// Not required unless 'prop-dict' is present.
if (!fmt.hasPropDict)
// Not required unless 'prop-dict' is present or we are not using properties.
if (!fmt.hasPropDict || !fmt.useProperties)
return;

SmallVector<MethodParameter> paramList;
Expand Down Expand Up @@ -1621,8 +1619,10 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
body.unindent() << "}\n";
body.unindent();
} else if (isa<PropDictDirective>(element)) {
body << " if (parseProperties(parser, result))\n"
<< " return ::mlir::failure();\n";
if (useProperties) {
body << " if (parseProperties(parser, result))\n"
<< " return ::mlir::failure();\n";
}
} else if (auto *customDir = dyn_cast<CustomDirective>(element)) {
genCustomDirectiveParser(customDir, body, useProperties, opCppClassName);
} else if (isa<OperandsDirective>(element)) {
Expand Down Expand Up @@ -2047,9 +2047,11 @@ static void genPropDictPrinter(OperationFormat &fmt, Operator &op,
}
}

body << " _odsPrinter << \" \";\n"
<< " printProperties(this->getContext(), _odsPrinter, "
"getProperties(), elidedProps);\n";
if (fmt.useProperties) {
body << " _odsPrinter << \" \";\n"
<< " printProperties(this->getContext(), _odsPrinter, "
"getProperties(), elidedProps);\n";
}
}

/// Generate the printer for the 'attr-dict' directive.
Expand Down Expand Up @@ -3771,7 +3773,8 @@ LogicalResult OpFormatParser::verifyOptionalGroupElement(SMLoc loc,
// Interface
//===----------------------------------------------------------------------===//

void mlir::tblgen::generateOpFormat(const Operator &constOp, OpClass &opClass) {
void mlir::tblgen::generateOpFormat(const Operator &constOp, OpClass &opClass,
bool hasProperties) {
// TODO: Operator doesn't expose all necessary functionality via
// the const interface.
Operator &op = const_cast<Operator &>(constOp);
Expand All @@ -3782,7 +3785,7 @@ void mlir::tblgen::generateOpFormat(const Operator &constOp, OpClass &opClass) {
llvm::SourceMgr mgr;
mgr.AddNewSourceBuffer(
llvm::MemoryBuffer::getMemBuffer(op.getAssemblyFormat()), SMLoc());
OperationFormat format(op);
OperationFormat format(op, hasProperties);
OpFormatParser parser(mgr, format, op);
FailureOr<std::vector<FormatElement *>> elements = parser.parse();
if (failed(elements)) {
Expand Down
3 changes: 2 additions & 1 deletion mlir/tools/mlir-tblgen/OpFormatGen.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ class OpClass;
class Operator;

// Generate the assembly format for the given operator.
void generateOpFormat(const Operator &constOp, OpClass &opClass);
void generateOpFormat(const Operator &constOp, OpClass &opClass,
bool hasProperties);

} // namespace tblgen
} // namespace mlir
Expand Down
Loading