diff --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td --- a/mlir/include/mlir/IR/OpBase.td +++ b/mlir/include/mlir/IR/OpBase.td @@ -2576,6 +2576,10 @@ // Whether this op has a folder. bit hasFolder = 0; + // Whether to let ops implement their custom `readProperties` and + // `writeProperties` methods to emit bytecode. + bit useCustomPropertiesEncoding = 0; + // Op traits. // Note: The list of traits will be uniqued by ODS. list traits = props; diff --git a/mlir/include/mlir/TableGen/Operator.h b/mlir/include/mlir/TableGen/Operator.h --- a/mlir/include/mlir/TableGen/Operator.h +++ b/mlir/include/mlir/TableGen/Operator.h @@ -352,6 +352,10 @@ bool hasFolder() const; + /// Whether to generate the `readProperty`/`writeProperty` methods for + /// bytecode emission. + bool useCustomPropertiesEncoding() const; + private: /// Populates the vectors containing operands, attributes, results and traits. void populateOpStructure(); diff --git a/mlir/lib/TableGen/Operator.cpp b/mlir/lib/TableGen/Operator.cpp --- a/mlir/lib/TableGen/Operator.cpp +++ b/mlir/lib/TableGen/Operator.cpp @@ -854,3 +854,7 @@ } bool Operator::hasFolder() const { return def.getValueAsBit("hasFolder"); } + +bool Operator::useCustomPropertiesEncoding() const { + return def.getValueAsBit("useCustomPropertiesEncoding"); +} diff --git a/mlir/test/Bytecode/versioning/versioned-op-with-native-prop-1.12.mlirbc b/mlir/test/Bytecode/versioning/versioned-op-with-native-prop-1.12.mlirbc new file mode 100644 index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 GIT binary patch literal 0 Hc$@&1 | FileCheck %s --check-prefix=ERR_VERSION_NEGATIVE // ERR_VERSION_NEGATIVE: unsupported version requested -1, must be in range [{{[0-9]+}}, {{[0-9]+}}] -// RUN: not mlir-opt %S/versioned-op-1.12.mlirbc -emit-bytecode \ +// RUN: not mlir-opt %S/versioned-op-with-prop-1.12.mlirbc -emit-bytecode \ // RUN: -emit-bytecode-version=999 2>&1 | FileCheck %s --check-prefix=ERR_VERSION_FUTURE // ERR_VERSION_FUTURE: unsupported version requested 999, must be in range [{{[0-9]+}}, {{[0-9]+}}] diff --git a/mlir/test/Bytecode/versioning/versioned_op.mlir b/mlir/test/Bytecode/versioning/versioned_op.mlir --- a/mlir/test/Bytecode/versioning/versioned_op.mlir +++ b/mlir/test/Bytecode/versioning/versioned_op.mlir @@ -10,7 +10,7 @@ // COM: version: 2.0 // COM: "test.versionedA"() <{dims = 123 : i64, modifier = false}> : () -> () // COM: } -// RUN: mlir-opt %S/versioned-op-2.0.mlirbc 2>&1 | FileCheck %s --check-prefix=CHECK1 +// RUN: mlir-opt %S/versioned-op-with-prop-2.0.mlirbc 2>&1 | FileCheck %s --check-prefix=CHECK1 // CHECK1: "test.versionedA"() <{dims = 123 : i64, modifier = false}> : () -> () //===--------------------------------------------------------------------===// @@ -22,8 +22,8 @@ // COM: version: 1.12 // COM: "test.versionedA"() <{dimensions = 123 : i64}> : () -> () // COM: } -// RUN: mlir-opt %S/versioned-op-1.12.mlirbc 2>&1 | FileCheck %s --check-prefix=CHECK2 -// CHECK2: "test.versionedA"() <{dims = 123 : i64, modifier = false}> : () -> () +// RUN: mlir-opt %S/versioned-op-with-prop-1.12.mlirbc 2>&1 | FileCheck %s --check-prefix=CHECK3 +// CHECK3: "test.versionedA"() <{dims = 123 : i64, modifier = false}> : () -> () //===--------------------------------------------------------------------===// // Test forbidden downgrade diff --git a/mlir/test/Bytecode/versioning/versioned_op_with_native_properties.mlir b/mlir/test/Bytecode/versioning/versioned_op_with_native_properties.mlir new file mode 100644 --- /dev/null +++ b/mlir/test/Bytecode/versioning/versioned_op_with_native_properties.mlir @@ -0,0 +1,28 @@ +// This file contains test cases related to the dialect post-parsing upgrade +// mechanism. +// COM: those tests parse bytecode that was generated before test dialect +// adopted `usePropertiesFromAttributes`. + +//===--------------------------------------------------------------------===// +// Test generic +//===--------------------------------------------------------------------===// + +// COM: bytecode contains +// COM: module { +// COM: version: 2.0 +// COM: test.with_versioned_properties 1 | 2 +// COM: } +// RUN: mlir-opt %S/versioned-op-with-native-prop-2.0.mlirbc 2>&1 | FileCheck %s --check-prefix=CHECK1 +// CHECK1: test.with_versioned_properties 1 | 2 + +//===--------------------------------------------------------------------===// +// Test upgrade +//===--------------------------------------------------------------------===// + +// COM: bytecode contains +// COM: module { +// COM: version: 1.12 + +// COM: } +// RUN: mlir-opt %S/versioned-op-with-native-prop-1.12.mlirbc 2>&1 | FileCheck %s --check-prefix=CHECK3 +// CHECK3: test.with_versioned_properties 1 | 0 diff --git a/mlir/test/lib/Dialect/Test/TestDialect.h b/mlir/test/lib/Dialect/Test/TestDialect.h --- a/mlir/test/lib/Dialect/Test/TestDialect.h +++ b/mlir/test/lib/Dialect/Test/TestDialect.h @@ -98,6 +98,17 @@ return content == rhs.content; } }; +struct VersionedProperties { + // For the sake of testing, assume that this object was associated to version + // 1.2 of the test dialect when having only one int value. In the current + // version 2.0, the property has two values. We also assume that the class is + // upgrade-able if value2 = 0. + int value1; + int value2; + bool operator==(const VersionedProperties &rhs) const { + return value1 == rhs.value1 && value2 == rhs.value2; + } +}; } // namespace test #define GET_OP_CLASSES diff --git a/mlir/test/lib/Dialect/Test/TestDialect.cpp b/mlir/test/lib/Dialect/Test/TestDialect.cpp --- a/mlir/test/lib/Dialect/Test/TestDialect.cpp +++ b/mlir/test/lib/Dialect/Test/TestDialect.cpp @@ -114,6 +114,16 @@ const PropertiesWithCustomPrint &prop); static ParseResult customParseProperties(OpAsmParser &parser, PropertiesWithCustomPrint &prop); +static LogicalResult setPropertiesFromAttribute(VersionedProperties &prop, + Attribute attr, + InFlightDiagnostic *diagnostic); +static DictionaryAttr getPropertiesAsAttribute(MLIRContext *ctx, + const VersionedProperties &prop); +static llvm::hash_code computeHash(const VersionedProperties &prop); +static void customPrintProperties(OpAsmPrinter &p, + const VersionedProperties &prop); +static ParseResult customParseProperties(OpAsmParser &parser, + VersionedProperties &prop); void test::registerTestDialect(DialectRegistry ®istry) { registry.insert(); @@ -1148,6 +1158,54 @@ prop.label = std::make_shared(std::move(label)); return success(); } +static LogicalResult +setPropertiesFromAttribute(VersionedProperties &prop, Attribute attr, + InFlightDiagnostic *diagnostic) { + DictionaryAttr dict = dyn_cast(attr); + if (!dict) { + if (diagnostic) + *diagnostic << "expected DictionaryAttr to set VersionedProperties"; + return failure(); + } + auto value1Attr = dict.getAs("value1"); + if (!value1Attr) { + if (diagnostic) + *diagnostic << "expected IntegerAttr for key `value1`"; + return failure(); + } + auto value2Attr = dict.getAs("value2"); + if (!value2Attr) { + if (diagnostic) + *diagnostic << "expected IntegerAttr for key `value2`"; + return failure(); + } + + prop.value1 = value1Attr.getValue().getSExtValue(); + prop.value2 = value2Attr.getValue().getSExtValue(); + return success(); +} +static DictionaryAttr +getPropertiesAsAttribute(MLIRContext *ctx, const VersionedProperties &prop) { + SmallVector attrs; + Builder b{ctx}; + attrs.push_back(b.getNamedAttr("value1", b.getI32IntegerAttr(prop.value1))); + attrs.push_back(b.getNamedAttr("value2", b.getI32IntegerAttr(prop.value2))); + return b.getDictionaryAttr(attrs); +} +static llvm::hash_code computeHash(const VersionedProperties &prop) { + return llvm::hash_combine(prop.value1, prop.value2); +} +static void customPrintProperties(OpAsmPrinter &p, + const VersionedProperties &prop) { + p << prop.value1 << " | " << prop.value2; +} +static ParseResult customParseProperties(OpAsmParser &parser, + VersionedProperties &prop) { + if (parser.parseInteger(prop.value1) || parser.parseVerticalBar() || + parser.parseInteger(prop.value2)) + return failure(); + return success(); +} static bool parseUsingPropertyInCustom(OpAsmParser &parser, int64_t value[3]) { return parser.parseLSquare() || parser.parseInteger(value[0]) || @@ -1220,6 +1278,69 @@ return MutableOperandRange(getOperation()); } +LogicalResult +TestVersionedOpA::readProperties(::mlir::DialectBytecodeReader &reader, + ::mlir::OperationState &state) { + auto &prop = state.getOrAddProperties(); + if (::mlir::failed(reader.readAttribute(prop.dims))) + return ::mlir::failure(); + + // Check if we have a version. If not, assume we are parsing the current + // version. + auto maybeVersion = reader.getDialectVersion("test"); + if (succeeded(maybeVersion)) { + // If version is less than 2.0, there is no additional attribute to parse. + // We can materialize missing properties post parsing before verification. + const auto *version = + reinterpret_cast(*maybeVersion); + if ((version->major < 2)) { + return success(); + } + } + + if (::mlir::failed(reader.readAttribute(prop.modifier))) + return ::mlir::failure(); + return ::mlir::success(); +} + +void TestVersionedOpA::writeProperties(::mlir::DialectBytecodeWriter &writer) { + auto &prop = getProperties(); + writer.writeAttribute(prop.dims); + writer.writeAttribute(prop.modifier); +} + +::mlir::LogicalResult TestOpWithVersionedProperties::readFromMlirBytecode( + ::mlir::DialectBytecodeReader &reader, test::VersionedProperties &prop) { + uint64_t value1, value2 = 0; + if (failed(reader.readVarInt(value1))) + return failure(); + + // Check if we have a version. If not, assume we are parsing the current + // version. + auto maybeVersion = reader.getDialectVersion("test"); + bool needToParseAnotherInt = true; + if (succeeded(maybeVersion)) { + // If version is less than 2.0, there is no additional attribute to parse. + // We can materialize missing properties post parsing before verification. + const auto *version = + reinterpret_cast(*maybeVersion); + if ((version->major < 2)) + needToParseAnotherInt = false; + } + if (needToParseAnotherInt && failed(reader.readVarInt(value2))) + return failure(); + + prop.value1 = value1; + prop.value2 = value2; + return success(); +} +void TestOpWithVersionedProperties::writeToMlirBytecode( + ::mlir::DialectBytecodeWriter &writer, + const test::VersionedProperties &prop) { + writer.writeVarInt(prop.value1); + writer.writeVarInt(prop.value2); +} + #include "TestOpEnums.cpp.inc" #include "TestOpInterfaces.cpp.inc" #include "TestTypeInterfaces.cpp.inc" diff --git a/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp b/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp --- a/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp +++ b/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp @@ -122,11 +122,10 @@ // Prior version 2.0, the old op supported only a single attribute called // "dimensions". We can perform the upgrade. topLevelOp->walk([](TestVersionedOpA op) { - if (auto dims = op->getAttr("dimensions")) { - op->removeAttr("dimensions"); - op->setAttr("dims", dims); - } - op->setAttr("modifier", BoolAttr::get(op->getContext(), false)); + // Prior version 2.0, `readProperties` did not process the modifier + // attribute. Handle that according to the version here. + auto &prop = op.getProperties(); + prop.modifier = BoolAttr::get(op->getContext(), false); }); return success(); } 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 @@ -785,7 +785,7 @@ let results = (outs AnyTensor); } -def OpWithShapedTypeInferTypeAdaptorInterfaceOp : +def OpWithShapedTypeInferTypeAdaptorInterfaceOp : TEST_Op<"op_with_shaped_type_infer_type_adaptor_if", [InferTensorTypeAdaptorWithReify]> { let arguments = (ins AnyTensor:$operand1, AnyTensor:$operand2); @@ -2601,6 +2601,10 @@ AnyI64Attr:$dims, BoolAttr:$modifier ); + + // Since we use properties to store attributes, we need a custom encoding + // reader/writer to handle versioning. + let useCustomPropertiesEncoding = 1; } def TestVersionedOpB : TEST_Op<"versionedB"> { @@ -2742,6 +2746,51 @@ }]; } +def VersionedProperties : Property<"VersionedProperties"> { + let convertToAttribute = [{ + getPropertiesAsAttribute($_ctxt, $_storage) + }]; + let convertFromAttribute = [{ + return setPropertiesFromAttribute($_storage, $_attr, $_diag); + }]; + let hashProperty = [{ + computeHash($_storage); + }]; +} + +def TestOpWithVersionedProperties : TEST_Op<"with_versioned_properties"> { + let assemblyFormat = "prop-dict attr-dict"; + let arguments = (ins + VersionedProperties:$prop + ); + let extraClassDeclaration = [{ + void printProperties(::mlir::MLIRContext *ctx, ::mlir::OpAsmPrinter &p, + const Properties &prop); + static ::mlir::ParseResult parseProperties(::mlir::OpAsmParser &parser, + ::mlir::OperationState &result); + static ::mlir::LogicalResult readFromMlirBytecode( + ::mlir::DialectBytecodeReader &, + test::VersionedProperties &prop); + static void writeToMlirBytecode( + ::mlir::DialectBytecodeWriter &, + const test::VersionedProperties &prop); + }]; + let extraClassDefinition = [{ + void TestOpWithVersionedProperties::printProperties(::mlir::MLIRContext *ctx, + ::mlir::OpAsmPrinter &p, const Properties &prop) { + customPrintProperties(p, prop.prop); + } + ::mlir::ParseResult TestOpWithVersionedProperties::parseProperties( + ::mlir::OpAsmParser &parser, + ::mlir::OperationState &result) { + Properties &prop = result.getOrAddProperties(); + if (customParseProperties(parser, prop.prop)) + return failure(); + return success(); + } + }]; +} + //===----------------------------------------------------------------------===// // Test Dataflow //===----------------------------------------------------------------------===// @@ -2765,7 +2814,7 @@ def TestStoreWithARegion : TEST_Op<"store_with_a_region", [DeclareOpInterfaceMethods, SingleBlock]> { - let arguments = (ins + let arguments = (ins Arg:$address, BoolAttr:$store_before_region ); diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp --- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp +++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp @@ -498,6 +498,9 @@ namespace { // Helper class to emit a record into the given output stream. class OpEmitter { + using ConstArgument = + llvm::PointerUnion; + public: static void emitDecl(const Operator &op, raw_ostream &os, @@ -525,6 +528,10 @@ // Generates code to manage the properties, if any! void genPropertiesSupport(); + // Generates code to manage the encoding of properties to bytecode. + void + genPropertiesSupportForBytecode(ArrayRef attrOrProperties); + // Generates getters for the attributes. void genAttrGetters(); @@ -1177,8 +1184,6 @@ void OpEmitter::genPropertiesSupport() { if (!emitHelper.hasProperties()) return; - using ConstArgument = - llvm::PointerUnion; SmallVector attrOrProperties; for (const std::pair &it : @@ -1245,21 +1250,6 @@ "getDiag")) ->body(); - auto &readPropertiesMethod = - opClass - .addStaticMethod( - "::mlir::LogicalResult", "readProperties", - MethodParameter("::mlir::DialectBytecodeReader &", "reader"), - MethodParameter("::mlir::OperationState &", "state")) - ->body(); - - auto &writePropertiesMethod = - opClass - .addMethod( - "void", "writeProperties", - MethodParameter("::mlir::DialectBytecodeWriter &", "writer")) - ->body(); - opClass.declare("Properties", "FoldAdaptor::Properties"); // Convert the property to the attribute form. @@ -1542,6 +1532,38 @@ } verifyInherentAttrsMethod << " return ::mlir::success();"; + // Generate methods to interact with bytecode. + genPropertiesSupportForBytecode(attrOrProperties); +} + +void OpEmitter::genPropertiesSupportForBytecode( + ArrayRef attrOrProperties) { + if (op.useCustomPropertiesEncoding()) { + opClass.declareStaticMethod( + "::mlir::LogicalResult", "readProperties", + MethodParameter("::mlir::DialectBytecodeReader &", "reader"), + MethodParameter("::mlir::OperationState &", "state")); + opClass.declareMethod( + "void", "writeProperties", + MethodParameter("::mlir::DialectBytecodeWriter &", "writer")); + return; + } + + auto &readPropertiesMethod = + opClass + .addStaticMethod( + "::mlir::LogicalResult", "readProperties", + MethodParameter("::mlir::DialectBytecodeReader &", "reader"), + MethodParameter("::mlir::OperationState &", "state")) + ->body(); + + auto &writePropertiesMethod = + opClass + .addMethod( + "void", "writeProperties", + MethodParameter("::mlir::DialectBytecodeWriter &", "writer")) + ->body(); + // Populate bytecode serialization logic. readPropertiesMethod << " auto &prop = state.getOrAddProperties(); (void)prop;";