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.

Sign up for GitHub

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

Jump to bottom

opt: add GroupNonUniformPartitionedNV capability to trim pass #5648

Merged
merged 1 commit into from
Apr 18, 2024
Merged

opt: add GroupNonUniformPartitionedNV capability to trim pass #5648

merged 1 commit into from
Apr 18, 2024

Conversation

sudonatalie
Copy link
Collaborator

No description provided.

@sudonatalie sudonatalie requested a review from Keenuts April 18, 2024 16:08
@sudonatalie
Copy link
Collaborator Author

Will be used for the implementation for microsoft/DirectXShaderCompiler#6545

sudonatalie added a commit to sudonatalie/DirectXShaderCompiler that referenced this pull request Apr 18, 2024
@sudonatalie
[SPIR-V] Implement WaveMatch intrinsic function
b2fda3a 
Adds support for the WaveMatch() intrinsic function from Shader Model
6.5 using the OpGroupNonUniformPartitionNV instruction from the
SPV_NV_shader_subgroup_partitioned extension.

Requires KhronosGroup/SPIRV-Tools#5648

Fixes microsoft#6545
Keenuts
Keenuts approved these changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@Keenuts Keenuts left a comment

Choose a reason for hiding this comment

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

Wonder if we need to start thinking about instructions enabled by multiple capabilities. Maybe there is something to do as in "if an capability is core, replace the vendor specific by the core capability"

@sudonatalie sudonatalie mentioned this pull request Apr 18, 2024
Merged
@sudonatalie
Copy link
Collaborator Author

The instructions enabled by this capability aren't and won't be enabled by any others (they have the NV suffix on the instruction too), but that is an optimization I looked into for SPV_KHR_ray_query and SPV_KHR_ray_tracing in the past, since they implement some of the same instructions and capabilities. It might be worth looking into if we find other examples or the hardware overlap gets more divergent.

@sudonatalie sudonatalie enabled auto-merge (squash) April 18, 2024 19:04
@sudonatalie sudonatalie force-pushed the add-GroupNonUniformPartitionedNV branch from 89ae335 to 066c051 Compare April 18, 2024 19:20
@sudonatalie
Copy link
Collaborator Author

Rebasing to see if that makes the bots happy

@sudonatalie sudonatalie merged commit 67a3ed6 into KhronosGroup:main Apr 18, 2024
24 checks passed
@sudonatalie sudonatalie deleted the add-GroupNonUniformPartitionedNV branch April 18, 2024 20:21
@dneto0
Copy link
Collaborator

dneto0 commented Apr 18, 2024

is an optimization I looked into for SPV_KHR_ray_query and SPV_KHR_ray_tracing in the past, since they implement some of the same instructions and capabilities. It might be worth look

What was the failure mode?
Once this landed on main, the continuous integration bots failed on mac bazel, due to 60s timeouts, it looks like:

//:opt_fold_test TIMEOUT in 60.2s
//:val_builtins_test TIMEOUT in 60.2s

@sudonatalie
Copy link
Collaborator Author

@dneto0 The hypothetical optimization of when to apply SPV_KHR_ray_query and SPV_KHR_ray_tracing is a tangent and not related to this change.

The only effect of this PR should be that the capability trimming pass now removes GroupNonUniformPartitionedNV when it's not needed, so I can't imagine it would cause bot timeouts, but if it does appear to be the culprit please feel free to revert.

@sudonatalie
Copy link
Collaborator Author

Oh and the bot failure I saw before rebase was just for an unrelated file's clang-format failure.

sudonatalie added a commit to sudonatalie/DirectXShaderCompiler that referenced this pull request May 6, 2024
@sudonatalie
[SPIR-V] Implement WaveMatch intrinsic function
b7dae6c 
Adds support for the WaveMatch() intrinsic function from Shader Model
6.5 using the OpGroupNonUniformPartitionNV instruction from the
SPV_NV_shader_subgroup_partitioned extension.

Requires KhronosGroup/SPIRV-Tools#5648

Fixes microsoft#6545
sudonatalie added a commit to microsoft/DirectXShaderCompiler that referenced this pull request May 6, 2024
@sudonatalie
[SPIR-V] Implement WaveMatch intrinsic function (#6546)
7db2fc1 
Adds support for the `WaveMatch()` intrinsic function from Shader Model
6.5 using the `OpGroupNonUniformPartitionNV` instruction from the
`SPV_NV_shader_subgroup_partitioned` extension.

SPIRV-Tools bumped to include:
KhronosGroup/SPIRV-Tools#5648

Fixes #6545
Keenuts added a commit to Keenuts/SPIRV-Tools that referenced this pull request Jun 11, 2024
@Keenuts
opt: fix Subgroup* trimming
4e826bb 
PR KhronosGroup#5648 added support for the GroupNonUniformPartitionedNV. But there
was an issue: the opcodes are enabled by multiple capabilities, and the
actual operand is what matters.

Added testing coverage and the implementation to correctly trim a few
NonUniform capabilities.

Signed-off-by: Nathan Gauër <brioche@google.com>
@Keenuts Keenuts mentioned this pull request Jun 11, 2024
Merged
Keenuts added a commit that referenced this pull request Jun 11, 2024
@Keenuts
opt: fix Subgroup* trimming (#5706)
65d30c3 
PR #5648 added support for the GroupNonUniformPartitionedNV. But there
was an issue: the opcodes are enabled by multiple capabilities, and the
actual operand is what matters.

Added testing coverage and the implementation to correctly trim a few
NonUniform capabilities.

Signed-off-by: Nathan Gauër <brioche@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Keenuts Keenuts Keenuts approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sudonatalie @dneto0 @Keenuts