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

Implement specialization constant support in numthreads / local_size #5963

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

juliusikkala
Copy link
Contributor

@juliusikkala juliusikkala commented Dec 30, 2024

Adds support for using SPIR-V specialization constants in numthreads:

[vk::constant_id(0)] const int WG_SIZE = 8;

[shader("compute")]
[numthreads(1, WG_SIZE, 1)]
void main(uint3 tid : SV_DispatchThreadID)
{
}

Also adds support in GLSL mode:

#version 460

layout(constant_id = 0) const uint size = 8;
// The standard local_size_x_id is supported, but also local_size_x = SPECCONSTNAME.
layout(local_size_x_id = 0, local_size_y = size) in;

void main()
{
}

Similar to the local_size_x_id feature from GLSL, this allows users to generate shader permutations with different work group shapes / thread counts after the shader has already been compiled to SPIR-V.

Currently, this works with the SPIR-V (using LocalSizeId execution mode) and GLSL (using local_size_{x,y,z}_id) targets. I'm not sure if there even is anything that could be done for the other targets, other than to just present diagnostics about this feature not being supported there.

@juliusikkala juliusikkala force-pushed the spec-const-numthreads branch from cde9d98 to 1862056 Compare January 5, 2025 11:59
@juliusikkala juliusikkala force-pushed the spec-const-numthreads branch from 1862056 to 1c7b6cc Compare January 6, 2025 12:55
@juliusikkala
Copy link
Contributor Author

I've now tested this in a real Vulkan program in a couple of different contexts, and this feature seems to now work correctly.

@juliusikkala juliusikkala marked this pull request as ready for review January 6, 2025 13:04
@juliusikkala juliusikkala requested a review from a team as a code owner January 6, 2025 13:04
Copy link
Collaborator

@csyonghe csyonghe left a comment

Choose a reason for hiding this comment

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

Can you add a test for this feature?

csyonghe
csyonghe previously approved these changes Jan 9, 2025
Copy link
Collaborator

@csyonghe csyonghe left a comment

Choose a reason for hiding this comment

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

Looks good to me. I wish there is a cleaner way to add this feature, but what is implemented in this PR is probably the easiest path forward. If I understand the code correctly, this implementation won't allow something like [numthreads(specConst+1, 1, 2)], right?

source/slang/slang-ast-modifier.h Show resolved Hide resolved
@csyonghe csyonghe added the pr: non-breaking PRs without breaking changes label Jan 9, 2025
@juliusikkala
Copy link
Contributor Author

It currently doesn't allow doing math on the specialization constant, because I'm fairly sure that that can't be expressed in SPIR-V. I'll fix that compiler warning later today, I've been developing on Linux only so I guess MSVC errors are to be expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: non-breaking PRs without breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants