This is an initial version, currently supports OpString and OpLine
for autogenerated operations during (de)serialization.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Awesome! Thanks a lot for adding this, Denis! Overall LGTM; just a few nits.
| mlir/lib/Dialect/SPIRV/Serialization/Deserializer.cpp | ||
|---|---|---|
| 408 | Add comment to explain what these three elements mean or maybe create a struct for it? | |
| 2040 | OpNoLine should kill this definition. Could you add a process* function for it? It should be trivial. :) | |
| mlir/lib/Dialect/SPIRV/Serialization/Serializer.cpp | ||
| 589 | if (!emitDebugInfo) return; | |
| mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp | ||
| 496 | We have a problem with the generated code size at the moment. Can we turn this into a utilty function in Serializer.cpp and call it here instead of embedding the logc for each op? (I'm also gonna clean up existing duplicated logic in the autogen'ed code soon.) | |
| mlir/lib/Dialect/SPIRV/Serialization/Deserializer.cpp | ||
|---|---|---|
| 2040 | Thanks for review, if I got it right, we have to clear debug line in 3 cases:
| |
Nice! Thanks Denis! I just have a few more nits. Feel free to land directly after addressing them!
| mlir/lib/Dialect/SPIRV/Serialization/Deserializer.cpp | ||
|---|---|---|
| 81 | Actually all zeros might be a triple in real SPIR-V blobs. fileID=0 can map to a %0 = OpString and line=0 and col=0 can mean something related to the whole file. Instead of baking the invalid/empty state in this struct, what about make the debugLine in Deserializer as Optional<DebugLine> so we can use llvm::None to mean no debug info? Then I think we can remove all the functions in this struct. | |
| 249 | Discontinues | |
| 2040 | Yup, correct. Thanks for handling the block end too! | |
| 2438 | Can this function be replaced by op->hasTrait<IsTerminator>()? | |
@antiagainst thanks for review, can you please take a look, to make sure I've updated the patch in right way. Thanks.
| mlir/lib/Dialect/SPIRV/Serialization/Deserializer.cpp | ||
|---|---|---|
| 77 | These constructors can be deleted too? | |
| mlir/lib/Dialect/SPIRV/Serialization/Deserializer.cpp | ||
|---|---|---|
| 71 | struct | |
struct