diff --git a/mlir/lib/Bytecode/Writer/BytecodeWriter.cpp b/mlir/lib/Bytecode/Writer/BytecodeWriter.cpp index 6097f0eda469..9493a6c19a10 100644 --- a/mlir/lib/Bytecode/Writer/BytecodeWriter.cpp +++ b/mlir/lib/Bytecode/Writer/BytecodeWriter.cpp @@ -962,9 +962,12 @@ LogicalResult BytecodeWriter::writeOp(EncodingEmitter &emitter, Operation *op) { DictionaryAttr attrs = op->getDiscardableAttrDictionary(); // Allow deployment to version getPropertiesStorage()) { attrs = op->getAttrDictionary(); + } if (!attrs.empty()) { opEncodingMask |= bytecode::OpEncodingMask::kHasAttrs; emitter.emitVarInt(numberingState.getNumber(attrs)); diff --git a/mlir/lib/Bytecode/Writer/IRNumbering.cpp b/mlir/lib/Bytecode/Writer/IRNumbering.cpp index 036a9477cce6..a306010698f2 100644 --- a/mlir/lib/Bytecode/Writer/IRNumbering.cpp +++ b/mlir/lib/Bytecode/Writer/IRNumbering.cpp @@ -425,9 +425,10 @@ void IRNumberingState::number(Operation &op) { // Only number the operation's dictionary if it isn't empty. DictionaryAttr dictAttr = op.getDiscardableAttrDictionary(); - // Prior to version 5 we need to number also the merged dictionnary - // containing both the inherent and discardable attribute. - if (config.getDesiredBytecodeVersion() < 5) + // Prior to version 5, or when properties are not used, we need to number also + // the merged dictionary containing both the inherent and discardable + // attribute. + if (config.getDesiredBytecodeVersion() < 5 || !op.getPropertiesStorage()) dictAttr = op.getAttrDictionary(); if (!dictAttr.empty()) number(dictAttr); diff --git a/mlir/unittests/Bytecode/BytecodeTest.cpp b/mlir/unittests/Bytecode/BytecodeTest.cpp index db748b475a7a..76ff1a8194db 100644 --- a/mlir/unittests/Bytecode/BytecodeTest.cpp +++ b/mlir/unittests/Bytecode/BytecodeTest.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "mlir/Bytecode/BytecodeReader.h" #include "mlir/Bytecode/BytecodeWriter.h" #include "mlir/IR/AsmState.h" #include "mlir/IR/BuiltinAttributes.h" @@ -15,6 +16,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/Endian.h" +#include "llvm/Support/MemoryBufferRef.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -86,3 +88,60 @@ TEST(Bytecode, MultiModuleWithResource) { checkResourceAttribute(*module); checkResourceAttribute(*roundTripModule); } + +namespace { +/// A custom operation for the purpose of showcasing how discardable attributes +/// are handled in absence of properties. +class OpWithoutProperties : public Op { +public: + // Begin boilerplate. + MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(OpWithoutProperties) + using Op::Op; + static ArrayRef getAttributeNames() { + static StringRef attributeNames[] = {StringRef("inherent_attr")}; + return ArrayRef(attributeNames); + }; + static StringRef getOperationName() { + return "test_op_properties.op_without_properties"; + } + // End boilerplate. +}; + +// A trivial supporting dialect to register the above operation. +class TestOpPropertiesDialect : public Dialect { +public: + MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestOpPropertiesDialect) + static constexpr StringLiteral getDialectNamespace() { + return StringLiteral("test_op_properties"); + } + explicit TestOpPropertiesDialect(MLIRContext *context) + : Dialect(getDialectNamespace(), context, + TypeID::get()) { + addOperations(); + } +}; +} // namespace + +constexpr StringLiteral withoutPropertiesAttrsSrc = R"mlir( + "test_op_properties.op_without_properties"() + {inherent_attr = 42, other_attr = 56} : () -> () +)mlir"; + +TEST(Bytecode, OpWithoutProperties) { + MLIRContext context; + context.getOrLoadDialect(); + ParserConfig config(&context); + OwningOpRef op = + parseSourceString(withoutPropertiesAttrsSrc, config); + + std::string bytecode; + llvm::raw_string_ostream os(bytecode); + ASSERT_TRUE(succeeded(writeBytecodeToFile(op.get(), os))); + std::unique_ptr block = std::make_unique(); + ASSERT_TRUE(succeeded(readBytecodeFile( + llvm::MemoryBufferRef(os.str(), "string-buffer"), block.get(), config))); + Operation *roundtripped = &block->front(); + EXPECT_EQ(roundtripped->getAttrs().size(), 2u); + EXPECT_TRUE(roundtripped->getInherentAttr("inherent_attr") != std::nullopt); + EXPECT_TRUE(roundtripped->getDiscardableAttr("other_attr") != Attribute()); +}