[Mlir-commits] [mlir] [mlir][DLTI] Introduce an operation interface for scoped DLTI queries (PR #232733)

[Christian Sigg]

================

----------------
chsigg wrote:

Hi Mehdi

Following up on @jpienaar's comment: this change breaks bytecode backdeployment (emitting bytecode readable by older MLIR consumers) and introduces a layering issue between `builtin`/`func` and `dlti`.

1. **`FuncDialect` and `GPUDialect` cannot emit v0 bytecode at all:**
   `{Func,GPU}DialectBytecodeInterface::writeVersion` unconditionally call `writer.writeVarInt(1)`. In addition, `{Func,GPUModule}Op::writeProperties` gate `properties.dlti` on `writer.getBytecodeVersion() >= bytecode::kDialectVersioning` rather than on a dialect version (and since `writeProperties` is only called when `bytecodeVersion >= kNativePropertiesEncoding`, that condition is always true).
   Because neither dialect implemented `BytecodeDialectInterface` before #232733, earlier readers fail on any bytecode containing `func.func` or `gpu.module` with:
   `dialect 'func' does not implement the bytecode interface, but found a version entry`
   Even with #233149 or `BytecodeWriterConfig::setDialectVersion`, there is no way to target the v0 encoding for `func` or `gpu`. This breaks backdeployment for producers that serialize `func.func`, such as [Shardy](https://github.com/openxla/shardy/blob/main/shardy/tools/sdy_translate_main.cc) ([`compatibility_test.0_0_0.mlir`](https://github.com/openxla/shardy/blob/main/shardy/dialect/sdy/ir/compatibility_test/compatibility_test.0_0_0.mlir)).

2. **Default `BuiltinDialect` v1 emission (and `BuiltinDialect::setVersion` mismatch):**
   - Producers targeting older bytecode versions via `writerConfig.setDesiredBytecodeVersion(...)` (such as [`stablehlo::serializePortableArtifact`](https://github.com/openxla/stablehlo/blob/main/stablehlo/dialect/Serialization.cpp), which wraps `vhlo.*` ops in a top-level `builtin.module`) now emit `builtin` version 1 and an extra `0x00` property byte on `ModuleOp` even when `properties.dlti` is null. Pre-#232733 readers reject this with `"reading newer builtin dialect version than supported"`, breaking 34 `stablehlo_legalize_to_vhlo.*.mlir` compatibility tests (`0.10.0`–`1.21.0`). Every downstream producer must now manually pin `builtin=0` (and `func=0`) alongside `setDesiredBytecodeVersion` even when `dlti` is never used.
   - Additionally, `BuiltinDialectBytecodeInterface::writeVersion` falls back to `BuiltinDialect::getVersion()` (`BuiltinDialect.td`) when `BytecodeWriterConfig` has no override, whereas `ModuleOp::writeProperties` only checks `writer.getDialectVersion<BuiltinDialect>()`. Calling `builtinDialect->setVersion(BuiltinDialectVersion(0))` therefore omits the version header while still writing `properties.dlti`, producing corrupt bytecode that `ModuleOp::readProperties` misparses.

3. **Layering and extensibility of `builtin.module` and `func.func` properties:**
   `builtin.module` and `func.func` are dialect-agnostic containers, but #232733 hardcodes a dialect-specific `OptionalAttr<DLTIQueryAttr>:$dlti` field into `ModuleOp::Properties` and `FuncOp::Properties` (adding `MLIRDLTIDialect` to `MLIRFuncDialect`'s `LINK_LIBS PUBLIC`, including `mlir/Dialect/DLTI/DLTI.h` in `FuncOps.h`, and splitting `ModuleOp` between a `dlti` property in `MLIRIR` and an external `DLTIQueryOpInterface` model in `MLIRDLTIDialect`, while `ModuleOp::getDataLayoutSpec()` and `getTargetSystemSpec()` still use the attribute dictionary).
   Hardcoding per-dialect fields into `ModuleOp::Properties` and `FuncOp::Properties` does not scale to other dialects that attach module- or function-scoped metadata (OpenMP, LLVM, GPU, etc.), and forces a `builtin`/`func` bytecode version bump every time any dialect adds such metadata.

*Disclaimer: The code analysis and this comment were drafted with AI assistance. I manually checked the claims as well as I could and tried to not make the comment too verbose.*


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