[Mlir-commits] [mlir] [mlir][bytecode] Fix D155919 and enable backward-compatibility and ba… (PR #70498)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: Matteo Franciolini (mfrancio)

<details>
<summary>Changes</summary>

…ck-deployment between version 5 and version 6 of the bytecode encoding.

Before D155919, when a dialect was leveraging properties to store attributes with `usePropertiesForAttributes`, the operand segment sizes attribute was emitted in the property section in sorted order together with all the other attributes of an op. After D155919, version 5 of the bytecode was emitting and parsing operand segment sizes after all the properties of an op, breaking backward compatibility and back deployment. This patch fixes the emission ordering and allows to parse bytecode files emitted before (D155919) with version 5 of MLIR bytecode. The patch also enables to emit correctly version 5 of MLIR bytecode.

---
Full diff: https://github.com/llvm/llvm-project/pull/70498.diff


1 Files Affected:

- (modified) mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp (+73-23) 


``````````diff
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 7029c0eac15c392..6deb111d228f0ae 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -156,28 +156,38 @@ static const char *const valueRangeReturnCode = R"(
 )";
 
 /// Read operand/result segment_size from bytecode.
-static const char *const readBytecodeSegmentSize = R"(
-if ($_reader.getBytecodeVersion() < /*kNativePropertiesODSSegmentSize=*/6) {
-  ::mlir::DenseI32ArrayAttr attr;
-  if (::mlir::failed($_reader.readAttribute(attr))) return ::mlir::failure();
-  if (attr.size() > static_cast<int64_t>(sizeof($_storage) / sizeof(int32_t))) {
-    $_reader.emitError("size mismatch for operand/result_segment_size");
-    return ::mlir::failure();
-  }
-  ::llvm::copy(::llvm::ArrayRef<int32_t>(attr), $_storage.begin());
-} else {
+static const char *const readBytecodeSegmentSizeNative = R"(
+if ($_reader.getBytecodeVersion() >= /*kNativePropertiesODSSegmentSize=*/6)
   return $_reader.readSparseArray(::llvm::MutableArrayRef($_storage));
-}
+)";
+
+static const char *const readBytecodeSegmentSizeLegacy = R"(
+  if ($_reader.getBytecodeVersion() < /*kNativePropertiesODSSegmentSize=*/6) {
+    auto &$_storage = prop.$_propName;
+    ::mlir::DenseI32ArrayAttr attr;
+    if (::mlir::failed($_reader.readAttribute(attr))) return ::mlir::failure();
+    if (attr.size() > static_cast<int64_t>(sizeof($_storage) / sizeof(int32_t))) {
+      $_reader.emitError("size mismatch for operand/result_segment_size");
+      return ::mlir::failure();
+    }
+    ::llvm::copy(::llvm::ArrayRef<int32_t>(attr), $_storage.begin());
+  }
 )";
 
 /// Write operand/result segment_size to bytecode.
-static const char *const writeBytecodeSegmentSize = R"(
-if ($_writer.getBytecodeVersion() < /*kNativePropertiesODSSegmentSize=*/6)
-  $_writer.writeAttribute(::mlir::DenseI32ArrayAttr::get(getContext(), $_storage));
-else
+static const char *const writeBytecodeSegmentSizeNative = R"(
+if ($_writer.getBytecodeVersion() >= /*kNativePropertiesODSSegmentSize=*/6)
   $_writer.writeSparseArray(::llvm::ArrayRef($_storage));
 )";
 
+/// Write operand/result segment_size to bytecode.
+static const char *const writeBytecodeSegmentSizeLegacy = R"(
+if ($_writer.getBytecodeVersion() < /*kNativePropertiesODSSegmentSize=*/6) {
+  auto &$_storage = prop.$_propName;
+  $_writer.writeAttribute(::mlir::DenseI32ArrayAttr::get($_ctxt, $_storage));
+}
+)";
+
 /// A header for indicating code sections.
 ///
 /// {0}: Some text, or a class name.
@@ -389,6 +399,14 @@ class OpOrAdaptorHelper {
     return resultSegmentsSize;
   }
 
+  unsigned long getOperandSegmentSizesLegacyIndex() {
+    return operandSegmentSizesLegacyIndex;
+  }
+
+  unsigned long getResultSegmentSizesLegacyIndex() {
+    return resultSegmentSizesLegacyIndex;
+  }
+
 private:
   // Compute the attribute metadata.
   void computeAttrMetadata();
@@ -406,6 +424,8 @@ class OpOrAdaptorHelper {
   std::string operandSegmentsSizeStorage;
   std::optional<NamedProperty> resultSegmentsSize;
   std::string resultSegmentsSizeStorage;
+  unsigned long operandSegmentSizesLegacyIndex = 0;
+  unsigned long resultSegmentSizesLegacyIndex = 0;
 
   // The number of required attributes.
   unsigned numRequired;
@@ -435,8 +455,8 @@ void OpOrAdaptorHelper::computeAttrMetadata() {
         "::mlir::DenseI32ArrayAttr::get($_ctxt, $_storage)",
         /*convertFromAttributeCall=*/
         "return convertFromAttribute($_storage, $_attr, $_diag);",
-        /*readFromMlirBytecodeCall=*/readBytecodeSegmentSize,
-        /*writeToMlirBytecodeCall=*/writeBytecodeSegmentSize,
+        /*readFromMlirBytecodeCall=*/readBytecodeSegmentSizeNative,
+        /*writeToMlirBytecodeCall=*/writeBytecodeSegmentSizeNative,
         /*hashPropertyCall=*/
         "::llvm::hash_combine_range(std::begin($_storage), "
         "std::end($_storage));",
@@ -478,6 +498,21 @@ void OpOrAdaptorHelper::computeAttrMetadata() {
                return lhs.attrName < rhs.attrName;
              });
 
+  // Store the position of the legacy operand_segment_sizes /
+  // result_segment_sizes so we can emit a backward compatible property readers
+  // and writers.
+  StringRef oldName = StringLiteral("operand_segment_sizes");
+  operandSegmentSizesLegacyIndex =
+      llvm::count_if(sortedAttrMetadata, [&](auto metaData) {
+        return metaData.attrName < oldName;
+      });
+
+  oldName = StringLiteral("result_segment_sizes");
+  resultSegmentSizesLegacyIndex =
+      llvm::count_if(sortedAttrMetadata, [&](auto metaData) {
+        return metaData.attrName < oldName;
+      });
+
   // Compute the subrange bounds for each attribute.
   numRequired = 0;
   for (AttributeMetadata &attr : sortedAttrMetadata) {
@@ -1580,15 +1615,30 @@ void OpEmitter::genPropertiesSupportForBytecode(
   readPropertiesMethod
       << "  auto &prop = state.getOrAddProperties<Properties>(); (void)prop;";
   writePropertiesMethod << "  auto &prop = getProperties(); (void)prop;\n";
-  for (const auto &attrOrProp : attrOrProperties) {
+  for (const auto &item : llvm::enumerate(attrOrProperties)) {
+    auto &attrOrProp = item.value();
+    FmtContext fctx;
+    fctx.addSubst("_reader", "reader")
+        .addSubst("_writer", "writer")
+        .addSubst("_storage", propertyStorage)
+        .addSubst("_ctxt", "this->getContext()");
+    if (emitHelper.getOperandSegmentsSize().has_value() &&
+        item.index() == emitHelper.getOperandSegmentSizesLegacyIndex()) {
+      FmtContext fmtCtxt(fctx);
+      fmtCtxt.addSubst("_propName", operandSegmentAttrName);
+      readPropertiesMethod << tgfmt(readBytecodeSegmentSizeLegacy, &fmtCtxt);
+      writePropertiesMethod << tgfmt(writeBytecodeSegmentSizeLegacy, &fmtCtxt);
+    }
+    if (emitHelper.getResultSegmentsSize().has_value() &&
+        item.index() == emitHelper.getResultSegmentSizesLegacyIndex()) {
+      FmtContext fmtCtxt(fctx);
+      fmtCtxt.addSubst("_propName", operandSegmentAttrName);
+      readPropertiesMethod << tgfmt(readBytecodeSegmentSizeLegacy, &fmtCtxt);
+      writePropertiesMethod << tgfmt(writeBytecodeSegmentSizeLegacy, &fmtCtxt);
+    }
     if (const auto *namedProperty =
             attrOrProp.dyn_cast<const NamedProperty *>()) {
       StringRef name = namedProperty->name;
-      FmtContext fctx;
-      fctx.addSubst("_reader", "reader")
-          .addSubst("_writer", "writer")
-          .addSubst("_storage", propertyStorage)
-          .addSubst("_ctxt", "this->getContext()");
       readPropertiesMethod << formatv(
           R"(
   {{

``````````

</details>


https://github.com/llvm/llvm-project/pull/70498