-
Notifications
You must be signed in to change notification settings - Fork 219
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
[SPIR-V 1.1] SPIRVReader: Add OpSizeOf support #2853
Conversation
The `OpSizeOf` instruction was added in SPIR-V 1.1, but not supported yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Do we some day expect to have forward translation as well?
; TODO: reenable spirv-val when OpSizeOf is supported. | ||
; R/U/N: spirv-val %t.spv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there already an issue in SPIRV-Tools?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, spirv-val doesn't know much about OpSizeOf
yet. I'm preparing a PR to fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just an issue reported to SPIR-V Tolls would have been good enough for now, but that's great your fix is already in progress :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, the fix is here: KhronosGroup/SPIRV-Tools#5879
(but even if that lands soon, we'll need to wait for the next SPIRV-Tools release before we can re-enable this check in CI.)
Some day hopefully. For now I'm focusing on the consumption side only. |
The
OpSizeOf
instruction was added in SPIR-V 1.1, but not supported yet.