[libclc][amdgpu] Implement __spirv_GroupNonUniform*Shuffle* builtins - #22665
[libclc][amdgpu] Implement __spirv_GroupNonUniform*Shuffle* builtins#22665zjin-lcf wants to merge 5 commits into
Conversation
The amdgcn libspirv device library declares the SPIR-V group non-uniform
shuffle builtins (__spirv_GroupNonUniformShuffle{,Up,Down,Xor}) via
libspirv/group/group_shuffle*.h but never defines them for the amdgpu
target. The SYCL headers (sycl/detail/spirv.hpp) emit these directly for
sub-group shuffle/permute/scan, and oneDPL work-group algorithms
(inclusive_scan, sort, ...) rely on them. Device linking for
amdgcn-amd-amdhsa therefore fails with e.g.
undefined symbol: __spirv_GroupNonUniformShuffleUp
Unlike SPIR-V targets there is no runtime translation of these
instructions, so the device library must provide them.
Add group/group_shuffle.cl for the amdgpu target, mapping the four
non-uniform shuffle operations onto the existing, validated
__spirv_SubgroupShuffle*INTEL primitives (misc/sub_group_shuffle.cl),
which lower to the hardware ds_bpermute wavefront shuffle. These
primitives derive the wavefront size from __spirv_BuiltInSubgroupMaxSize()
rather than hardcoding it and use only ds_bpermute, so they are correct
across all wave64 CDNA generations (CDNA1 gfx908, CDNA2 gfx90a,
CDNA3 gfx942, CDNA4 gfx950) with no arch-specific code. Only scalar
overloads are defined: the SYCL headers scalarize all vector/marray
shuffles and lower bitcast/generic shuffles onto integer scalars before
reaching the intrinsic, matching the scalar-only __spirv_GroupBroadcast
definitions in group/collectives.cl.
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes device-link failures on amdgcn-amd-amdhsa by providing missing definitions for the SPIR-V group non-uniform shuffle builtins in the AMDGPU libspirv device library, mapping them onto the existing __spirv_SubgroupShuffle*INTEL implementations (which lower to ds_bpermute).
Changes:
- Add
libclc/libspirv/lib/amdgpu/group/group_shuffle.climplementing__spirv_GroupNonUniformShuffle{,Up,Down,Xor}for scalar types. - Register the new source file in the AMDGPU libspirv CMake sources list.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| libclc/libspirv/lib/amdgpu/group/group_shuffle.cl | Implements missing GroupNonUniform shuffle builtins via existing AMDGPU subgroup shuffle primitives. |
| libclc/libspirv/lib/amdgpu/CMakeLists.txt | Adds the new shuffle implementation file to the AMDGPU libspirv build. |
|
@zjin-lcf please fix the code formatting |
Co-authored-by: Cursor <cursoragent@cursor.com>
Apply the final patch from intel#22665 to provide the missing __spirv_GroupNonUniformShuffle{,Up,Down,Xor} definitions for AMDGPU. Upstream-PR: intel#22665 Upstream-Commit: af14d1d Upstream-Commit: d11a322
Apply the final patch from intel#22665 to provide the missing __spirv_GroupNonUniformShuffle{,Up,Down,Xor} definitions for AMDGPU. Upstream-PR: intel#22665 Upstream-Commit: af14d1d Upstream-Commit: d11a322
|
Hi @zjin-lcf , do you plan to continue working on your PRs? |
|
Thanks for the follow-up. Yes, I plan to continue working on this PR. The formatting issue has been fixed and the formatter check now passes. The remaining ARL CI failure is an unrelated XPASS in two bindless-image tests. I do not have permission to rerun the workflow; could a maintainer please rerun the failed job? @Maetveis @kweronsx, could you please review when convenient? |
| __CLC_GROUP_NON_UNIFORM_SHUFFLE(half) | ||
| __CLC_GROUP_NON_UNIFORM_SHUFFLE(float) | ||
| __CLC_GROUP_NON_UNIFORM_SHUFFLE(double) | ||
|
|
There was a problem hiding this comment.
might not related to this PR: is there need or plan to implement vector type value input?
There was a problem hiding this comment.
Implemented in cefdc2e. Correcting my earlier answer: the vector forms are worth defining. Clang declares these builtins for every vector width (AGenTypeNWithBool in SPIRVBuiltins.td) and libspirv declares them for every gentype, so a vector-typed call is valid but would have been an unresolved symbol on amdgcn. The SYCL headers still only emit scalar calls, so nothing regresses.
The vector overloads shuffle element-wise, mirroring the vector __spirv_SubgroupShuffle*INTEL overloads in misc/sub_group_shuffle.cl, and cover widths 2/3/4/8/16 for all eleven gentypes.
Testing: added libclc/test/AMDGPU/group-non-uniform-shuffle.cl, which compiles calls to all four operations for every declared scalar and vector type and then requires llvm-nm -u to report no undefined __spirv_GroupNonUniformShuffle* reference. It fails against a device library without these definitions and passes with them. I also ran the four operations on an MI100 (gfx908) for int2/3/4/8/16, uint4, short8, ushort16, long2, ulong4, half2, float4 and double2, comparing every component against the value expected from the source lane; all matched, and the scalar path still links and runs.
Use the operation's full SPIR-V name to distinguish group non-uniform shuffles from other shuffle implementations. Co-authored-by: Cursor <cursoragent@cursor.com>
…oads Clang declares these builtins for every vector width, so calls with a vector Value operand must resolve for amdgcn too, even though the SYCL headers only ever emit the scalar forms. Add a lit test that fails on any overload libspirv declares but the target does not define. Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
||
| #undef __CLC_GROUP_NON_UNIFORM_SHUFFLE | ||
| #undef __CLC_GROUP_NON_UNIFORM_SHUFFLE_VEC | ||
| #undef __CLC_GROUP_NON_UNIFORM_SHUFFLE_SCALAR |
There was a problem hiding this comment.
following implementation may look simpler (functionally the same) and it automatically take care of #ifdef cl_khr_fp64 #pragma OPENCL EXTENSION cl_khr_fp64 : enable #ifdef cl_khr_fp16 #pragma OPENCL EXTENSION cl_khr_fp16 : enable
group_non_uniform_shuffle.inc
#if __CLC_VECSIZE_OR_1 == 1
_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE
__spirv_GroupNonUniformShuffle(int execution, __CLC_GENTYPE value, uint id) {
...
}
_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE __spirv_GroupNonUniformShuffleXor(
int execution, __CLC_GENTYPE value, uint mask) {
...
}
_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE __spirv_GroupNonUniformShuffleUp(
int execution, __CLC_GENTYPE value, uint delta) {
...
}
_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE __spirv_GroupNonUniformShuffleDown(
int execution, __CLC_GENTYPE value, uint delta) {
...
}
#else
_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE
__spirv_GroupNonUniformShuffle(int execution, __CLC_GENTYPE value, uint id) {
...
}
_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE __spirv_GroupNonUniformShuffleXor(
int execution, __CLC_GENTYPE value, uint mask) {
...
}
_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE __spirv_GroupNonUniformShuffleUp(
int execution, __CLC_GENTYPE value, uint delta) {
...
}
_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE __spirv_GroupNonUniformShuffleDown(
int execution, __CLC_GENTYPE value, uint delta) {
...
}
#endif
group_non_uniform_shuffle.cl:
#define __CLC_BODY "group_non_uniform_shuffle.inc"
#include "clc/integer/gentype.inc"
#define __CLC_BODY "group_non_uniform_shuffle.inc"
#include "clc/math/gentype.inc"
There was a problem hiding this comment.
Implemented in 100a29a. The implementation now uses a shared group_non_uniform_shuffle.inc instantiated through clc/integer/gentype.inc and clc/math/gentype.inc, including their fp16/fp64 handling. Scalar overloads delegate directly to the existing subgroup primitives; vector overloads use the standard libclc union-based element-wise scalarization pattern.
Verification:
- clang-format and
git diff --checkpass - compilation with the libclc amdgcn flags produces all 264 expected definitions and the same 44 subgroup-shuffle dependencies
- Bugbot found no issues
- MI100 (gfx908) runtime validation passed all four operations for int2/3/4/8/16, uint4, short8, ushort16, long2, ulong4, half2, float4, and double2
| // instructions, so they must be provided by the device library. | ||
|
|
||
| #define __CLC_BODY "group_non_uniform_shuffle.inc" | ||
| #include <clc/integer/gentype.inc> |
There was a problem hiding this comment.
use "" quote instead of <> for include, to align with existing include styles in libclc
There was a problem hiding this comment.
Done, switched both gentype.inc includes to quotes in 865ec36.
Follow the standard libclc gentype pattern so scalar and vector shuffle overloads share one implementation while preserving element-wise vector lowering. Co-authored-by: Cursor <cursoragent@cursor.com>
100a29a to
865ec36
Compare
The amdgcn libspirv device library declares the SPIR-V group non-uniform shuffle builtins (__spirv_GroupNonUniformShuffle{,Up,Down,Xor}) via libspirv/group/group_shuffle*.h but never defines them for the amdgpu target. The SYCL headers (sycl/detail/spirv.hpp) emit these directly for sub-group shuffle/permute/scan, and oneDPL work-group algorithms (inclusive_scan, sort, ...) rely on them. Device linking for amdgcn-amd-amdhsa therefore fails with e.g.
undefined symbol: __spirv_GroupNonUniformShuffleUp
Unlike SPIR-V targets there is no runtime translation of these instructions, so the device library must provide them.
Add group/group_shuffle.cl for the amdgpu target, mapping the four non-uniform shuffle operations onto the existing, validated __spirv_SubgroupShuffle*INTEL primitives (misc/sub_group_shuffle.cl), which lower to the hardware ds_bpermute wavefront shuffle. These primitives derive the wavefront size from __spirv_BuiltInSubgroupMaxSize() rather than hardcoding it and use only ds_bpermute, so they are correct across all wave64 CDNA generations (CDNA1 gfx908, CDNA2 gfx90a, CDNA3 gfx942, CDNA4 gfx950) with no arch-specific code. Only scalar overloads are defined: the SYCL headers scalarize all vector/marray shuffles and lower bitcast/generic shuffles onto integer scalars before reaching the intrinsic, matching the scalar-only __spirv_GroupBroadcast definitions in group/collectives.cl.