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

[spirv] support non-int type SV_DispatchThreadId #3481

Closed
wants to merge 1 commit into from

Conversation

jaebaek
Copy link
Collaborator

@jaebaek jaebaek commented Feb 19, 2021

Based on the SPIR-V spec, we cannot use types other than a vector
with 3 integer element for %gl_GlobalInvocationID. However, HLSL
SV_DispatchThreadId stage variable allows other types. This
difference results in a SPIR-V validation error when we use float for
SV_DispatchThreadId. This commit fixes the issue.

Fixes #3443

Based on the SPIR-V spec, we cannot use types other than a vector
with 3 integer element for `%gl_GlobalInvocationID`. However, HLSL
`SV_DispatchThreadId` stage variable allows other types. This
difference results in a SPIR-V validation error when we use float for
`SV_DispatchThreadId`. This commit fixes the issue.

Fixes microsoft#3443
@jaebaek jaebaek added the spirv Work related to SPIR-V label Feb 19, 2021
@jaebaek jaebaek requested a review from ehsannas February 19, 2021 21:49
@jaebaek jaebaek self-assigned this Feb 19, 2021
@vcsharma
Copy link
Contributor

Hello @jaebaek , I just thought that I should bring it to your notice that we have a pending change #3043 that is still under review on DXIL side to do more comprehensive type checking for SV semantics. As part of that, we intend to disallow non-integer types for SV_DispatchThreadID. I would also let @tex3d comment on this just in case he has something to add here. Thanks!

@AppVeyorBot
Copy link

@tex3d
Copy link
Contributor

tex3d commented Feb 22, 2021

Yes, float was not really supposed to be a valid type to be used for SV_DispatchThreadID. When the other change is merged, it may simply make this case dead code and make the test in this PR fail, since it would no longer be legal HLSL.

@ehsannas
Copy link
Contributor

Thanks for the heads up @vcsharma @tex3d .

@jaebaek I won't review this then. Please feel free to close it. Thanks!

@jaebaek
Copy link
Collaborator Author

jaebaek commented Feb 24, 2021

@vcsharma @tex3d Thank you for letting me know :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spirv Work related to SPIR-V
Projects
None yet
5 participants