Skip to content
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

Propose A Better Scheme for Id Allocation Specifically For The Testing Purpose #3512

Open
qingyuanzNV opened this issue Feb 13, 2024 · 2 comments

Comments

@qingyuanzNV
Copy link
Contributor

It's great that we maintain a large repository of shaders in this project to ensure that glslang is composed with high quality code. However, the exact-match model in test result and the counting approach of id allocation strategy are making it difficult to reason about the test result change. Even a really simple change in glslang could lead to an id shift and all relevant test results may be edited entirely.

Could we add a new scheme of id allocation for testing only so that those ids are somewhat more stable, and the test result changes are easier to reason about? For example, we may let "id = opcode * 10000 + offset" so at least ids are stable across different opcodes?

@arcady-lunarg
Copy link
Contributor

I think the concern about spurious diffs is a valid one, and it's been an issue for me as well when trying to review changes that add or remove some instructions at the beginning of a SPIR-V file and thus result in big diffs to test results where the actual change is really small and everything else is just changes in Ids. On the other hand, I don't think it's a great idea to maintain separate paths for Id allocation and technically what you propose goes against the SPIR-V spec recommendation to have the Id space be reasonably compact.
There is a tool in SPIRV-Tools called spirv-diff, which tries to do structural diffing between SPIR-V files, however it's not currently used in our test harness, nor is it really directly usable because of the format the result files, which is not binaries nor is it the standard text format as used by spirv-as and spirv-dis. If you want to integrate this into the test harness somehow though, I think that would be a great patch and would make my life easier as a maintainer.

@qingyuanzNV
Copy link
Contributor Author

To clarify, I'm proposing this scheme only for testing. Those modules shouldn't be generated in real world for end users. Perhaps this could also be done by separately maintained tools instead of directly in glslang.

spirv-diff is also a promising option. Though we have to come up with some approach to make is usable as obviously we can't store diffs as golden files.

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

2 participants