Add shader validation tests on subgroup-size-control - Part I#4641
Add shader validation tests on subgroup-size-control - Part I#4641Jiawei-Shao wants to merge 3 commits into
subgroup-size-control - Part I#4641Conversation
This patch adds the first part of the shader validation tests on the extension `subgroup-size-control`: - Enabling `subgroup_size_control` must also enable `subgroups`. - The WGSL attribute `@subgroup_size` must be used when `subgroup_size_control` is enabled. - `@subgroup_size` can only be used in the compute stage issue: gpuweb#4640
|
Results for build job (at 382124f): +webgpu:shader,validation,extension,subgroup_size_control:enable_subgroup_size_control_requires_subgroups:* - 2 cases, 2 subcases (~1/case)
+webgpu:shader,validation,extension,subgroup_size_control:use_subgroup_size_attribute_requires_subgroup_size_control_extension_enabled:* - 2 cases, 2 subcases (~1/case)
+webgpu:shader,validation,extension,subgroup_size_control:subgroup_size_attribute_only_valid_in_compute_stage:* - 3 cases, 3 subcases (~1/case)
-TOTAL: 281247 cases, 2322858 subcases
+TOTAL: 281254 cases, 2322865 subcases |
|
PTAL, thanks! |
| * Returns a subgroup size value that is valid for use in the @subgroup_size | ||
| * attribute on the current adapter. | ||
| * | ||
| * On Intel gen-12lp, subgroupMinSize may be 8 in fragment stages, which is below the allowed range |
There was a problem hiding this comment.
You're saying adapterInfo.subgroupMinSize is 8 on these devices, but 8 is not actually a valid subgroup size in compute? That's kind of weird, it requires applications to do weird stuff like this to get the right subgroup size. I would have thought subgroupMinSize/subgroupMaxSize are just for compute, especially now that we're adding subgroup-size-control which only applies to compute.
I understand we're saying not all values between subgroupMinSize and subgroupMaxSize are valid to create all pipelines with, but surely they should all work for a trivial compute pipeline? Maybe we can't technically specify that, but I think we should try to test it unless there are known counterexamples.
There was a problem hiding this comment.
I believe it is true that on these devices, the subgroup size is 8 for fragment shaders but that is never a valid subgroup size for a compute shader. This is the reason we decided to say that it would be a spurious failure if you requested a subgroup size that is not supported (otherwise we have to add a second set of limits as previously proposed, which we decided against).
So no, unfortunately not all sizes between min and max will work even for trivial compute pipelines.
There was a problem hiding this comment.
You're saying adapterInfo.subgroupMinSize is 8 on these devices, but 8 is not actually a valid subgroup size in compute?
Yes that's the case on Intel D3D12 Gen12 GPUs. D3D12 only requires the value used for [WaveSize] must be between [WaveLaneCountMin, WaveLaneCountMax], while it never means [WaveLaneCountMin, WaveLaneCountMax] has the same concept as minSubgroupSize and maxSubgroupSize in Vulkan.
That's why in the initial proposal there were another two members in adapterInfo (minComputeExplicitSubgroupSize and maxComputeExplicitSubgroupSize) that show the actual range we can use for @subgroup_size.
Maybe we can't technically specify that, but I think we should try to test it unless there are known counterexamples.
I am afraid we can only test it per vendor and architecture, which makes the test doesn't look so good, but I have to do so as we have dropped the query on the range of the subgroup sizes that can be used for @subgroup_size.
This patch adds the first part of the shader validation tests on the extension
subgroup-size-control:subgroup_size_controlmust also enablesubgroups.@subgroup_sizecannot used whensubgroup_size_controlis not enabled.@subgroup_sizecan only be used in the compute stageIssue: #4640
Requirements for PR author:
.unimplemented()./** documented */and new helper files are found inhelper_index.txt.Requirements for reviewer sign-off:
When landing this PR, be sure to make any necessary issue status updates.