Skip to content

Reimplement spirv-tools assembler in spirv-tools-rs (or rspirv) #140

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

Closed
Jake-Shadle opened this issue Oct 25, 2020 · 8 comments
Closed

Reimplement spirv-tools assembler in spirv-tools-rs (or rspirv) #140

Jake-Shadle opened this issue Oct 25, 2020 · 8 comments

Comments

@Jake-Shadle
Copy link
Member

Jake-Shadle commented Oct 25, 2020

As part of #117 I added an assembler type that wraps spirv-tools' assembler, however, I ran into a super strange issue that I still haven't found the root cause of.

Basically usage of some of C++'s (i)stringstream would work when being called from eg and example executable that statically linked the spirv-tools library (but not the c++ stdlib), but would, when part of the librustc_codegen_spirv shared library, result in, in the case of std::stringstream bad cast exception with a perfect ordinary uint32_t, or, in the case of std::istringstream fail to parse eg "42" and set the "bad" bit.

It is kind of frustrating as, AFAICT, the inputs would be the exact same (I even hashed them) going through the same calls, with the same options, and then one would work as intended and the other would fail, suggesting some kind of global state or possibly memory stomps? Anway, this only seemed to effect libstdc++, as windows and mac builds worked fine.

I finally just got fed up and replaced most of the number parsing in the assembler, see the diff for what I ended up doing to work around this in most cases (need to support hexadecimals floats still).

Anyway, long story short, the spir-v assembly syntax is relatively straightforward, it shouldn't? be terribly difficult to replace the need for the spirv-tools' assembler, we can even take advantage of their excellent suite of tests.

@Jake-Shadle Jake-Shadle changed the title Reimplement spirv-tools assembler in spirv-tools (or rspirv) Reimplement spirv-tools assembler in spirv-tools-rs (or rspirv) Oct 25, 2020
@Jake-Shadle
Copy link
Member Author

Then again, this is only used for testing, maybe it's not worth any effort.

@Jasper-Bekkers
Copy link
Contributor

@Jake-Shadle rspirv already can assemble a textual representation; https://docs.rs/rspirv/0.7.0/rspirv/binary/trait.Disassemble.html

@Jake-Shadle
Copy link
Member Author

Well, this is for assembling the binary from the text, which seems to also be implemented in rspirv via the Assemble trait, I was kind of confused why spirv-as was being used in the linker test, as presumably that would just be testing that rspirv is doing the correct thing? Or am I just missing something.

@Jake-Shadle
Copy link
Member Author

I was confused, this isn't needed, rspirv should have everything we need.

@khyperia
Copy link
Contributor

wait, @Jasper-Bekkers, that's Disassemble, not assembling (spirv-as)? @Jake-Shadle Is there another thing in rspirv that you found that supports creating a binary from text? I'm super confused, but I guess it doesn't matter if you figured it out :P

@Jake-Shadle
Copy link
Member Author

Yes, I was confused as well by the use of spirv-as in the tests, but AFAICT the actual linking doing can solely use rspirv, no need for spirv-tools for it.

@khyperia
Copy link
Contributor

Ah yeah - the use of spirv-as is so that we can write text format in tests, instead of opaque pre-assembled binaries.

@Jake-Shadle
Copy link
Member Author

So yah, I guess we would want this then, at least for testing, but I'll keep closed for now since I doubt anyone will have time to work on it anytime soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants