Conversation
Implement #[spirv(workgroup_size)] builtin that provides workgroup dimensions as a constant to compute shaders. The constant is created from the LocalSize execution mode values. See https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#BuiltIn and https://github.com/KhronosGroup/SPIRV-Guide/blob/main/chapters/local_size_and_workgroup_size.md
| None => { | ||
| self.tcx.dcx().span_err( | ||
| attrs.builtin.as_ref().unwrap().span, | ||
| "WorkgroupSize builtin requires LocalSize execution mode to be set (e.g., #[spirv(compute(threads(x, y, z)))])", |
There was a problem hiding this comment.
Specialization constants seem to be supported as well, at least according to specs. Is this a temporary restriction? I'd add a TODO if it is.
|
I think it's fine the way it is, but I do have some design feedback for potential future iterations: I'd much rather want the workgroup size be a constant, similar to what I'm doing rn: pub const LIGHTING_WG_SIZE: u32 = 64;
const_assert_eq!(LIGHTING_WG_SIZE, 64);
#[bindless(compute(threads(64)))]
pub fn lighting_cs(...) { ... }This allows me to access the workgroup size from the CPU code as well to compute the workgroup dimensions: let groups = [
(image_size.x + LIGHTING_WG_SIZE - 1) / LIGHTING_WG_SIZE,
image_size.y,
1,
];And allows you to declare shared memory as a multiple of the workgroup size: pub const DIRECTIONAL_SHADOWS_WG_SIZE: u32 = 64;
const SHARED_SIZE: usize = DIRECTIONAL_SHADOWS_WG_SIZE as usize * 2;
const_assert_eq!(DIRECTIONAL_SHADOWS_WG_SIZE, 64);
#[bindless(compute(threads(64)))]
pub fn directional_shadows(
#[spirv(workgroup)] shared: &[f32; SHARED_SIZE],
) { ... }Instead of this PR, I'd much rather have the proc macro accept a const expr instead of a literal, so we can do this: pub const LIGHTING_WG_SIZE: u32 = 64;
#[bindless(compute(threads(LIGHTING_WG_SIZE)))]
pub fn lighting_cs(...) { ... }Not sure how hard this is to implement. |
|
I tried to do the rust const route for the past week, the problem is that rustc doesn't support constants in attrs currently so I have to do a bunch of hacky stuff with resolution to make it work. I have it working for most cases, but not all. Not sure if it is better to support it with sharp edge cases or not support it at all. |
|
I'll clean up what I have and we can compare and contrast. At the very least we'll need to document as people coming from glsl will expect something like workgroupsize attr to exist. |
|
@LegNeato I wonder if we actually need to parse out the actual value within that macro.... Currently we're emitting this, for which we clearly need to parse the literals: But with Vulkan1.2 we get the new fancy Note how at the point we're writing the We probably should move this design discussion to an issue, I just want to talk about it now in case this has any ramifications for this PR as well. |
|
rustc just changed some attr parsing infra, need to look and see if it better enables this |
|
I just read this: https://github.com/KhronosGroup/SPIRV-Guide/blob/1482521d9826fdbb48854dfff8c46cc4a8b3607e/chapters/local_size_and_workgroup_size.md And while I don't really understand much, it seems that:
While I think A simpler (but less intuitive) way to achieve the same might be to generate a submodule in proc macro, such that this works: #[spirv(compute(threads(256)))]
pub fn entrypoint() {}
// This is the same as `const_assert_eq!()`, but without macros and external crates
const _: () = {
// `entrypoint` is a macro-generated module and has a constant defined in it
assert!(entrypoint::WORKGROUP_SIZE == 256);
}; |
Implement
#[spirv(workgroup_size)]builtin that provides workgroup dimensions as a constant to compute shaders. The constant is created from the LocalSize execution mode values.See https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#BuiltIn and https://github.com/KhronosGroup/SPIRV-Guide/blob/main/chapters/local_size_and_workgroup_size.md