-
Notifications
You must be signed in to change notification settings - Fork 441
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
Added required_subgroup_size to PipelineShaderStageCreateInfo #2235
Added required_subgroup_size to PipelineShaderStageCreateInfo #2235
Conversation
#2234 will make some changes to how |
@@ -191,12 +203,14 @@ impl ComputePipeline { | |||
let specialization_info_vk; | |||
let specialization_map_entries_vk: Vec<_>; | |||
let mut specialization_data_vk: Vec<u8>; | |||
let required_subgroup_size_create_info; |
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.
All the variables holding data to be passed to Vulkan end in _vk
. Could you include that as well please?
#2234 has been merged now. |
@charles-r-earp are you still interested in getting this merged? |
Yes |
fbe641c
to
a99f221
Compare
A few points:
|
This is validated:
However, workgroup_size is not available because it's not reflected from the shader, so it's currently None. |
Can you add the necessary reflection then? |
a48cc75
to
ec195e7
Compare
vulkano/src/pipeline/mod.rs
Outdated
if !properties | ||
.required_subgroup_size_stages | ||
.unwrap_or_default() | ||
.contains(stages) |
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.
You can use contains_enum(stage)
here and then you don't need to do the conversion above.
It looks good so far, but I noticed that the required_subgroup_size test is causing an overflow when I run it: ---- pipeline::compute::tests::required_subgroup_size stdout ---- It looks like |
While |
|
Nice catch! |
Looks good now, thank you! |
…o-rs#2235) * Added required_subgroup_size to PipelineShaderStageCreateInfo * Added validation errors. * Fixed error msgs / vuids. * ComputeShaderExecution for validating local_size. * WorkgroupSizeId reflection. * contains_enum * Reworked ComputeShaderExecution. * panic msgs. * workgroup size validation * unused import * fixed test deprecated fn * catch workgroup size overflow * EntryPointInfo::local_size docs * comments * typo + error msg
Compute and Graphics pipelines can be created with a required subgroup size.
https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_subgroup_size_control.html
To do
Limitations
For compute, can't validate max_compute_workgroup_subgroups without knowing the workgroup size, which is not currently reflected.
This interacts with additional VK_EXT_subgroup_size_control flags for requiring full subgroups and allowing varying subgroups, but since those aren't currently implemented they were neglected. If implemented, they add additional constraints to validate.