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] Add support vk::ext_execution_mode #4086

Merged
merged 2 commits into from
Nov 18, 2021

Conversation

jiaolu
Copy link
Contributor

@jiaolu jiaolu commented Nov 17, 2021

[[vk::ext_storage_class(uint storage_class)]]
this is part of PRs for the
#3919

@AppVeyorBot
Copy link

@jaebaek jaebaek self-requested a review November 17, 2021 20:49
@jaebaek jaebaek added the spirv Work related to SPIR-V label Nov 17, 2021
Copy link
Collaborator

@jaebaek jaebaek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

I realize that the spec I wrote about vk::ext_execution_mode(uint execution_mode); is wrong. I forgot to mention the extra literal parameters (Thank you for catching it!).

Your implementation looks fine. I will update the spec based on your implementation soon.

// RUN: %dxc -T ps_6_0 -E main -spirv -Vd

//CHECK: {{%\w+}} = OpVariable {{%\w+}} RayPayloadNV
//CHECK: {{%\w+}} = OpVariable {{%\w+}} CrossWorkgroup
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check the type of variable as well?
We want to make sure that RayPayloadNV is used for payload but it is not used for foo and vice versa. I am just worried in the future some developers can update the code and break your implementation.

tools/clang/lib/SPIRV/SpirvEmitter.cpp Show resolved Hide resolved
[[vk::ext_storage_class(uint storage_class)]]
this is part of PRs for the
microsoft#3919
@AppVeyorBot
Copy link

Copy link
Collaborator

@jaebaek jaebaek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks again for your contribution!

@jaebaek jaebaek merged commit 552b11b into microsoft:master Nov 18, 2021
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
Development

Successfully merging this pull request may close these issues.

3 participants