-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Structured representation #5
Comments
Please please make this happen! We've started writing the SPIRV->MSL/DXBC/HLSL/GLSL/etc transpiler in Rust, and processing the raw SPIRV modules (that are just a bunch of
I honestly don't see a compelling argument to 2. My choice would certainly be 1), given that:
|
Thanks @kvark! I hear you. I'll look into putting some effort into implementing the structured representation. :) |
@antiagainst please don't forget about this. As we delay, spirv-cross is getting developed further, and it will be harder for us to catch up. |
Hey @kvark, I'm sorry I kept saying that I would like to devote some time to this but didn't really fulfilled. I recently moved onto working on TensorFlow and MLIR, so I'm not sure when I'll have time on this in the near future. But it's definitely on my plate. In the meantime, feel free to contribute! :) |
@antiagainst I'd really like to move this forward. If you don't have time and energy to do this, could you outline a rough plan for contributors to follow when implementing this? Would you also be able to review the changes? |
Hey @kvark, sorry for the late reply. I'm correctly busy with MLIR work so likely won't have the bandwidth here in the near future. But I'd certainly happy to help answering questions and reviewing pull requests. |
Looked at the state of this today. Apparently, most of the infrastructure is already in place:
Do I understand correctly that we only need to get those root-level items (modules, functions, etc) now, after which we can implement conversion to/from the data representation? Or did you plan on working with bytes directly and skip the "mr" layer? |
Firstly, regarding the current existing code for structured representation, it actually deviates from the design proposal as in the original post. Instead of doing trait objects, the existing code just uses memory arena and vectors and indexes into vectors. (I read several blog post regarding how to better model graphs using Rust; that seems the suggested way. But things may change now with the Rust ecosystem. So better suggestions certainly welcome!) Yes structured representation classes for SPIR-V module, function, basic block are missing. We will need to add that. Aside from those, instruction class is half baked. We need to properly wire up instructions so that we can trace an operand back to its original generating instruction. So instead of using Besides, module-level instructions (e.g., OpEntryPoint, OpExecutionModel) are actually "metadata", compared to function-level instructions (e.g., OpFMul), which are real computation. We should consider "fold" these metadata instructions into the module representation itself so that we don't need to go through the same instruction processing mechanism to rediscover these facts. Similarly for decorations, they are also metadata to the decorated instructions. We should also consider folding them into the instruction they decorate. The Regarding the translation path, my original thought is to have binary -> mr -> sr because I think it's easier to bootstrap. Going from binary to mr we can see in the future. |
Thanks for the detailed response!
I think it's working well. If we have, say, enum Foo {
One,
Two(Foo), // NOPE
} We could use A lot of the things you describe are something I was already half-way doing, i.e.
I'm splitting the instruction generator to put some stuff as separate structures (the mode setting ops, other module stuff) and the others as an enum variants (real instructions).
Agreed. Similarly to what #38 is doing.
Great, thank you! I'm glad we are on the same page :) |
Part of #5 Also refines the decoration support with: 1. associating member decorations directly with the members 2. supporting multiple decorations
Yup, a progressive approach SGTM. We can gradually make more and more instructions stand-alone. :) |
I've begun working through rspirv, & will devote my time to moving this forward. |
Currently there is only a plain data representation (DR) implemented in this project. While it introduces little overheads when working with the binary parser and is straightforward for investigating the contents of SPIR-V binaries, it has quite limited functionalities. Instructions are not interconnected; types are not hierarchicalized; metadata (decorations, debug instructions, etc.) are not linked to the targets; etc. It is inconvenient to build future tasks like validation, optimization, and translating from front end languages on top of this DR. So a structured representation (SR) should be introduced, containing:
Type hierarchy
We can either implement it using 1) enums or 2) marker traits with multiple implementing structs.
match
s). Also easy to attach data and methods (for like, getting constants from a type, composing types, etc.) on each type.I'm leaning towards 2. Let's worry about performance later.
Representing SPIR-V instructions
A single flexible instruction struct that can represent all SPIR-V instructions is inferior to a dedicated instruction struct for each SPIR-V instruction: it loses the structure naturally encoded in dedicated instruction struct and will need validation for operand layout correctness, etc. A huge enum containing all possible instructions is ugly. It seems there is no way to go around trait objects, which is avoided by the Rust community to the best. But it's the natural way of fulfilling this kind of task.
There are quite some instructions to implement, but most of them can be generated from the grammar.
Decoration struct
Decorations can have associated data.
Special care should be taken when handling struct types since decorations on struct members cannot penetrate to the underlying base types.
The text was updated successfully, but these errors were encountered: