Skip to content

Commit 0ee4875

Browse files
GleasonKjoker-eph
authored andcommitted
[mlir][bytecode] Error if requested bytecode version is unsupported
Currently desired bytecode version is clamped to the maximum. This allows requesting bytecode versions that do not exist. We have added callsite validation for this in StableHLO to ensure we don't pass an invalid version number, probably better if this is managed upstream. If a user wants to use the current version, then omitting `setDesiredBytecodeVersion` is the best way to do that (as opposed to providing a large number). Adding this check will also properly error on older version numbers as we increment the minimum supported version. Silently claming on minimum version would likely lead to unintentional forward incompatibilities. Separately, due to bytecode version being `int64_t` and using methods to read/write uints, we can generate payloads with invalid version numbers: ``` mlir-opt file.mlir --emit-bytecode --emit-bytecode-version=-1 | mlir-opt <stdin>:0:0: error: bytecode version 18446744073709551615 is newer than the current version 5 ``` This is fixed with version bounds checking as well. Reviewed By: mehdi_amini Differential Revision: https://reviews.llvm.org/D151838
1 parent 21dfaf6 commit 0ee4875

File tree

3 files changed

+22
-7
lines changed

3 files changed

+22
-7
lines changed

mlir/include/mlir/Bytecode/BytecodeWriter.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,9 @@ class BytecodeWriterConfig {
4040
/// Return an instance of the internal implementation.
4141
const Impl &getImpl() const { return *impl; }
4242

43-
/// Set the desired bytecode version to emit. This function clamps the version
44-
/// to the existing version if larger than existing. The desired version may
45-
/// not be used depending on the features used and the actual version required
46-
/// is returned by bytecode writer entry point.
43+
/// Set the desired bytecode version to emit. This method does not validate
44+
/// the desired version. The bytecode writer entry point will return failure
45+
/// if it cannot emit the desired version.
4746
void setDesiredBytecodeVersion(int64_t bytecodeVersion);
4847

4948
/// Get the set desired bytecode version to emit.

mlir/lib/Bytecode/Writer/BytecodeWriter.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,7 @@ void BytecodeWriterConfig::attachResourcePrinter(
6565
}
6666

6767
void BytecodeWriterConfig::setDesiredBytecodeVersion(int64_t bytecodeVersion) {
68-
// Clamp to current version.
69-
impl->bytecodeVersion =
70-
std::min<int64_t>(bytecodeVersion, bytecode::kVersion);
68+
impl->bytecodeVersion = bytecodeVersion;
7169
}
7270

7371
int64_t BytecodeWriterConfig::getDesiredBytecodeVersion() const {
@@ -630,6 +628,13 @@ LogicalResult BytecodeWriter::write(Operation *rootOp, raw_ostream &os) {
630628
emitter.emitString("ML\xefR");
631629

632630
// Emit the bytecode version.
631+
if (config.bytecodeVersion < bytecode::kMinSupportedVersion ||
632+
config.bytecodeVersion > bytecode::kVersion)
633+
return rootOp->emitError()
634+
<< "unsupported version requested " << config.bytecodeVersion
635+
<< ", must be in range ["
636+
<< static_cast<int64_t>(bytecode::kMinSupportedVersion) << ", "
637+
<< static_cast<int64_t>(bytecode::kVersion) << ']';
633638
emitter.emitVarInt(config.bytecodeVersion);
634639

635640
// Emit the producer.

mlir/test/Bytecode/versioning/versioned_bytecode.mlir

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,14 @@
1212
// RUN: mlir-opt %S/versioned-op-1.12.mlirbc -o %t.2 && \
1313
// RUN: diff %t.1 %t.2
1414

15+
//===--------------------------------------------------------------------===//
16+
// Test invalid versions
17+
//===--------------------------------------------------------------------===//
18+
19+
// RUN: not mlir-opt %S/versioned-op-1.12.mlirbc -emit-bytecode \
20+
// RUN: -emit-bytecode-version=-1 2>&1 | FileCheck %s --check-prefix=ERR_VERSION_NEGATIVE
21+
// ERR_VERSION_NEGATIVE: unsupported version requested -1, must be in range [{{[0-9]+}}, {{[0-9]+}}]
22+
23+
// RUN: not mlir-opt %S/versioned-op-1.12.mlirbc -emit-bytecode \
24+
// RUN: -emit-bytecode-version=999 2>&1 | FileCheck %s --check-prefix=ERR_VERSION_FUTURE
25+
// ERR_VERSION_FUTURE: unsupported version requested 999, must be in range [{{[0-9]+}}, {{[0-9]+}}]

0 commit comments

Comments
 (0)