Dialects can opt-in to providing custom encodings by implementing the
BytecodeDialectInterface. This interface provides hooks, namely
readAttribute/readType and writeAttribute/writeType, that will be used
by the bytecode reader and writer. These hooks are provided a reader and writer
implementation that can be used to encode various constructs in the underlying
bytecode format. A unique feature of this interface is that dialects may choose
to only encode a subset of their attributes and types in a custom bytecode
format, which can simplify adding new or experimental components that aren't
fully baked.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/include/mlir/Bytecode/BytecodeImplementation.h | ||
---|---|---|
45 | What is the advantage of differentiating between default constructible and not here? Why not always use the FailureOr<T>() from? | |
mlir/lib/IR/BuiltinDialectBytecode.cpp | ||
30–38 | Should the varint for the length, written and read by (read|write)List be documented here as well? |
Nice!
mlir/docs/BytecodeFormat.md | ||
---|---|---|
213 | compatibility Also we may over-promise at the moment here. | |
mlir/include/mlir/Bytecode/BytecodeImplementation.h | ||
31 | Please document a bit more who should subclass it (no one?) and how, where it get involved. | |
mlir/lib/Bytecode/Reader/BytecodeReader.cpp | ||
342 | Nit: "parent" is confusing to me here. | |
mlir/lib/IR/BuiltinDialectBytecode.cpp | ||
30–38 | I don't think so: the [] expresses that this is a list, the encoding of a list is an implementation detail of the list, not part of the schema here. | |
37 | <StringAttr, Attribute> ? I guess StringAttr may not be desirable because it is an ID here and it can be confusing? |
mlir/docs/BytecodeFormat.md | ||
---|---|---|
213 | Yeah, just dropping this part of the sentence for now. I don't really want to promise anything at this point. | |
mlir/include/mlir/Bytecode/BytecodeImplementation.h | ||
45 | We have different APIs that use either LogicalResult+Reference or FailureOr, supporting both is convenient here. Switched to checking the callable type instead of basing this on default constructible. | |
mlir/lib/IR/BuiltinDialectBytecode.cpp | ||
30–38 | Yeah, I didn't want this schema to try to decipher the underlying implementation, just match what API the BytecodeReader/Writer interface provides. E.g. when we support IntegerAttr I want to just say APInt, not whatever specific implementation we use for encoding that. I suppose if we really wanted to make this explicitly differentiated from an array, we could do something like list<blah>. | |
37 | I think StringAttr is fine to use here. |
It looks like this introduces a dependency cycle between MLIRBytecodeReader and MLIRIR, and MLIRBytecodeWriter and MLIRIR, because now MLIRIR uses the BytecodeImplementation.h header. We can work around that in Bazel by making the BytecodeImplementation.h header part of the "IR" target, but that is not nice.
compatibility
Also we may over-promise at the moment here.