Use symbolic name for previous MLIR Bytecode versions

Reviewed By: jpienaar, burmako

Differential Revision: https://reviews.llvm.org/D151621

2 files changed, 33 insertions, 30 deletions

diff --git a/mlir/lib/Bytecode/Reader/BytecodeReader.cpp b/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
index 4121cfa7230b..177372c68046 100644
--- a/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
+++ b/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
@@ -80,7 +80,7 @@ static bool isSectionOptional(bytecode::Section::ID sectionID, int version) {
   case bytecode::Section::kDialectVersions:
     return true;
   case bytecode::Section::kProperties:
-    return version < 5;
+    return version < bytecode::kNativePropertiesEncoding;
   default:
     llvm_unreachable("unknown section ID");
   }
@@ -492,7 +492,7 @@ struct BytecodeOperationName {
   StringRef name;
 
   /// Whether this operation was registered when the bytecode was produced.
-  /// This flag is populated when bytecode version >=5.
+  /// This flag is populated when bytecode version >=kNativePropertiesEncoding.
   std::optional<bool> wasRegistered;
 };
 } // namespace
@@ -1647,7 +1647,7 @@ LogicalResult BytecodeReader::Impl::parseVersion(EncodingReader &reader) {
                             currentVersion);
   }
   // Override any request to lazy-load if the bytecode version is too old.
-  if (version < 2)
+  if (version < bytecode::kLazyLoading)
     lazyLoading = false;
   return success();
 }
@@ -1699,9 +1699,9 @@ BytecodeReader::Impl::parseDialectSection(ArrayRef<uint8_t> sectionData) {
 
   // Parse each of the dialects.
   for (uint64_t i = 0; i < numDialects; ++i) {
-    /// Before version 1, there wasn't any versioning available for dialects,
-    /// and the entryIdx represent the string itself.
-    if (version == 0) {
+    /// Before version kDialectVersioning, there wasn't any versioning available
+    /// for dialects, and the entryIdx represent the string itself.
+    if (version < bytecode::kDialectVersioning) {
       if (failed(stringReader.parseString(sectionReader, dialects[i].name)))
         return failure();
       continue;
@@ -1731,9 +1731,9 @@ BytecodeReader::Impl::parseDialectSection(ArrayRef<uint8_t> sectionData) {
   auto parseOpName = [&](BytecodeDialect *dialect) {
     StringRef opName;
     std::optional<bool> wasRegistered;
-    // Prior to version 5, the information about wheter an op was registered or
-    // not wasn't encoded.
-    if (version < 5) {
+    // Prior to version kNativePropertiesEncoding, the information about wheter
+    // an op was registered or not wasn't encoded.
+    if (version < bytecode::kNativePropertiesEncoding) {
       if (failed(stringReader.parseString(sectionReader, opName)))
         return failure();
     } else {
@@ -1746,9 +1746,9 @@ BytecodeReader::Impl::parseDialectSection(ArrayRef<uint8_t> sectionData) {
     opNames.emplace_back(dialect, opName, wasRegistered);
     return success();
   };
-  // Avoid re-allocation in bytecode version > 3 where the number of ops are
-  // known.
-  if (version > 3) {
+  // Avoid re-allocation in bytecode version >=kElideUnknownBlockArgLocation
+  // where the number of ops are known.
+  if (version >= bytecode::kElideUnknownBlockArgLocation) {
     uint64_t numOps;
     if (failed(sectionReader.parseVarInt(numOps)))
       return failure();
@@ -2078,7 +2078,7 @@ BytecodeReader::Impl::parseRegions(std::vector<RegionReadState> &regionStack,
           RegionReadState childState(*op, &reader, isIsolatedFromAbove);
 
           // Isolated regions are encoded as a section in version 2 and above.
-          if (version >= 2 && isIsolatedFromAbove) {
+          if (version >= bytecode::kLazyLoading && isIsolatedFromAbove) {
             bytecode::Section::ID sectionID;
             ArrayRef<uint8_t> sectionData;
             if (failed(reader.parseSection(sectionID, sectionData)))
@@ -2229,7 +2229,8 @@ BytecodeReader::Impl::parseOpWithoutRegions(EncodingReader &reader,
   /// Parse the use-list orders for the results of the operation. Use-list
   /// orders are available since version 3 of the bytecode.
   std::optional<UseListMapT> resultIdxToUseListMap = std::nullopt;
-  if (version > 2 && (opMask & bytecode::OpEncodingMask::kHasUseListOrders)) {
+  if (version >= bytecode::kUseListOrdering &&
+      (opMask & bytecode::OpEncodingMask::kHasUseListOrders)) {
     size_t numResults = opState.types.size();
     auto parseResult = parseUseListOrderForRange(reader, numResults);
     if (failed(parseResult))
@@ -2316,7 +2317,7 @@ BytecodeReader::Impl::parseBlockHeader(EncodingReader &reader,
     return failure();
 
   // Uselist orders are available since version 3 of the bytecode.
-  if (version < 3)
+  if (version < bytecode::kUseListOrdering)
     return success();
 
   uint8_t hasUseListOrders = 0;
@@ -2357,7 +2358,7 @@ LogicalResult BytecodeReader::Impl::parseBlockArguments(EncodingReader &reader,
   while (numArgs--) {
     Type argType;
     LocationAttr argLoc = unknownLoc;
-    if (version > 3) {
+    if (version >= bytecode::kElideUnknownBlockArgLocation) {
       // Parse the type with hasLoc flag to determine if it has type.
       uint64_t typeIdx;
       bool hasLoc;
diff --git a/mlir/lib/Bytecode/Writer/BytecodeWriter.cpp b/mlir/lib/Bytecode/Writer/BytecodeWriter.cpp
index 3be342b36354..03c7a53a34e5 100644
--- a/mlir/lib/Bytecode/Writer/BytecodeWriter.cpp
+++ b/mlir/lib/Bytecode/Writer/BytecodeWriter.cpp
@@ -657,7 +657,7 @@ LogicalResult BytecodeWriter::write(Operation *rootOp, raw_ostream &os) {
   writeStringSection(emitter);
 
   // Emit the properties section.
-  if (config.bytecodeVersion >= 5)
+  if (config.bytecodeVersion >= bytecode::kNativePropertiesEncoding)
     writePropertiesSection(emitter);
   else if (!propertiesSection.empty())
     return rootOp->emitError(
@@ -708,7 +708,7 @@ void BytecodeWriter::writeDialectSection(EncodingEmitter &emitter) {
     // Write the string section and get the ID.
     size_t nameID = stringSection.insert(dialect.name);
 
-    if (config.bytecodeVersion == 0) {
+    if (config.bytecodeVersion < bytecode::kDialectVersioning) {
       dialectEmitter.emitVarInt(nameID);
       continue;
     }
@@ -732,13 +732,13 @@ void BytecodeWriter::writeDialectSection(EncodingEmitter &emitter) {
                                  std::move(versionEmitter));
   }
 
-  if (config.bytecodeVersion > 3)
+  if (config.bytecodeVersion >= bytecode::kElideUnknownBlockArgLocation)
     dialectEmitter.emitVarInt(size(numberingState.getOpNames()));
 
   // Emit the referenced operation names grouped by dialect.
   auto emitOpName = [&](OpNameNumbering &name) {
     size_t stringId = stringSection.insert(name.name.stripDialect());
-    if (config.bytecodeVersion < 5)
+    if (config.bytecodeVersion < bytecode::kNativePropertiesEncoding)
       dialectEmitter.emitVarInt(stringId);
     else
       dialectEmitter.emitVarIntWithFlag(stringId, name.name.isRegistered());
@@ -826,7 +826,7 @@ LogicalResult BytecodeWriter::writeBlock(EncodingEmitter &emitter,
     emitter.emitVarInt(args.size());
     for (BlockArgument arg : args) {
       Location argLoc = arg.getLoc();
-      if (config.bytecodeVersion > 3) {
+      if (config.bytecodeVersion >= bytecode::kElideUnknownBlockArgLocation) {
         emitter.emitVarIntWithFlag(numberingState.getNumber(arg.getType()),
                                    !isa<UnknownLoc>(argLoc));
         if (!isa<UnknownLoc>(argLoc))
@@ -836,7 +836,7 @@ LogicalResult BytecodeWriter::writeBlock(EncodingEmitter &emitter,
         emitter.emitVarInt(numberingState.getNumber(argLoc));
       }
     }
-    if (config.bytecodeVersion > 2) {
+    if (config.bytecodeVersion >= bytecode::kUseListOrdering) {
       uint64_t maskOffset = emitter.size();
       uint8_t encodingMask = 0;
       emitter.emitByte(0);
@@ -868,9 +868,10 @@ LogicalResult BytecodeWriter::writeOp(EncodingEmitter &emitter, Operation *op) {
 
   // Emit the attributes of this operation.
   DictionaryAttr attrs = op->getDiscardableAttrDictionary();
-  // Allow deployment to version <5 by merging inherent attribute with the
-  // discardable ones. We should fail if there are any conflicts.
-  if (config.bytecodeVersion < 5)
+  // Allow deployment to version <kNativePropertiesEncoding by merging inherent
+  // attribute with the discardable ones. We should fail if there are any
+  // conflicts.
+  if (config.bytecodeVersion < bytecode::kNativePropertiesEncoding)
     attrs = op->getAttrDictionary();
   if (!attrs.empty()) {
     opEncodingMask |= bytecode::OpEncodingMask::kHasAttrs;
@@ -878,8 +879,8 @@ LogicalResult BytecodeWriter::writeOp(EncodingEmitter &emitter, Operation *op) {
   }
 
   // Emit the properties of this operation, for now we still support deployment
-  // to version <5.
-  if (config.bytecodeVersion >= 5) {
+  // to version <kNativePropertiesEncoding.
+  if (config.bytecodeVersion >= bytecode::kNativePropertiesEncoding) {
     std::optional<ssize_t> propertiesId = propertiesSection.emit(op);
     if (propertiesId.has_value()) {
       opEncodingMask |= bytecode::OpEncodingMask::kHasProperties;
@@ -913,7 +914,7 @@ LogicalResult BytecodeWriter::writeOp(EncodingEmitter &emitter, Operation *op) {
 
   // Emit the use-list orders to bytecode, so we can reconstruct the same order
   // at parsing.
-  if (config.bytecodeVersion > 2)
+  if (config.bytecodeVersion >= bytecode::kUseListOrdering)
     writeUseListOrders(emitter, opEncodingMask, ValueRange(op->getResults()));
 
   // Check for regions.
@@ -934,8 +935,9 @@ LogicalResult BytecodeWriter::writeOp(EncodingEmitter &emitter, Operation *op) {
 
     for (Region &region : op->getRegions()) {
       // If the region is not isolated from above, or we are emitting bytecode
-      // targeting version <2, we don't use a section.
-      if (!isIsolatedFromAbove || config.bytecodeVersion < 2) {
+      // targeting version <kLazyLoading, we don't use a section.
+      if (!isIsolatedFromAbove ||
+          config.bytecodeVersion < bytecode::kLazyLoading) {
         if (failed(writeRegion(emitter, &region)))
           return failure();
         continue;