Bytecode implementation for StableHLO #60
Conversation
|
Adding Mehdi as a reviewer. I think my biggest area of concern is the datatypes chosen to represent data. Just about everything is represented as a Additionally, note the test cases run in |
The (s)varint are the right choice right now for anything stored as integers (up to 64 bits). |
Added round-trip testing for all StableHLO ops in `bytecode.mlir`, and documentation in `bytecode.md`.
| outputBatchDimension: svarint | ||
| outputFeatureDimension: svarint | ||
| outputSpatialDimensions: svarint[] | ||
| } |
There was a problem hiding this comment.
Hmm I don't think that you pushed this update?
| @@ -0,0 +1,695 @@ | |||
| /* Copyright 2022 The StableHLO Authors. | |||
| StablehloBytecode.cpp - StableHLO Bytecode Implementation */ | |||
There was a problem hiding this comment.
Let's use the usual license blurb here?
|
|
||
| /// This enum contains marker codes used to indicate which attribute is | ||
| /// currently being decoded, and how it should be decoded. The order of these | ||
| /// codes should generally be unchanged, as any changes will inevitably break |
There was a problem hiding this comment.
Do you anticipate situations where the order should be changed? (I guess, this is an eventuality that "generally" is preparing the reader for).
docs/bytecode.md
Outdated
| the `strings` command can be used to see what attributes were missed: | ||
|
|
||
| _Note: The following trace is from a previous revision where the `scatter` attribute | ||
| was not implemented. Currently all types/attrs are implemented and log only shows |
There was a problem hiding this comment.
How about we remove this sentence and its accompanying output in the printout below? "The following trace is from a previous revision where the scatter attribute was not implemented". Even without stablehlo.scatter, the output is educational enough?
This PR plays a role in #34. There is still a little more work to do before closing this issue, as documented in
bytecode.mdof this PR.bytecode.mlir:stablehlo-optandstablehlo-opt -emit-bytecode file.mlir | stablehlo-optproduce the same results.stablehlo-bytecodedebug trace to ensure that all attrs/types have bytecoding implemented.bytecode.md.