[SYCL] Add CUDA/HIP hardware acceleration for dot_acc (dp4a) - #22756
[SYCL] Add CUDA/HIP hardware acceleration for dot_acc (dp4a)#22756zjin-lcf wants to merge 1 commit into
Conversation
The `sycl_ext_oneapi_dot_accumulate` `dot_acc` helpers previously always
used a scalar implementation. Lower them to the native dp4a instruction
on hardware that supports it, keeping the scalar path as a portable
fallback (the numerical result is identical):
- NVPTX (CUDA): emit the `dp4a.{s32,u32}.{s32,u32}` instruction on
sm_61+ (Pascal and later), covering all four operand-signedness
combinations.
- AMDGCN (HIP): use `__builtin_amdgcn_sdot4` / `udot4` (v_dot4, gfx906+/
CDNA/RDNA2) and `__builtin_amdgcn_sudot4` (v_dot4_i32_iu8, gfx11/RDNA3+
and gfx12) for the mixed- and, where sdot4 is unavailable, same-sign
cases. Each path is guarded by `__builtin_amdgcn_is_invocable` so
architectures without a given instruction fall back to scalar.
The scalar fallbacks are moved into `detail` (dropping the `Us`/`Uu`
unions that leaked into the public namespace) and now read the packed
words through `int8_t`/`uint8_t` views, which fixes signedness on targets
where plain `char` is unsigned (e.g. AArch64) and avoids type-punning UB.
The `vec` overloads bit-cast directly to the packed 32-bit word.
Add check_device_code tests for the CUDA (sm_90) and HIP (gfx90a, gfx942,
gfx1030, gfx1100, gfx1200) codegen, and update the existing E2E tests'
comments to reflect that they now validate the hardware paths too.
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
could you also upstream this change to llvm-project?
There was a problem hiding this comment.
Done - upstreamed as llvm/llvm-project#213691.
I have also dropped that commit from this PR: it was only here because the branch was accidentally created off my other topic branch, and it is unrelated to dp4a. It is still tracked separately in #22750, so this PR is now purely the dp4a change.
While preparing the upstream patch I found the __NVPTX__ exclusion came from llvm/llvm-project#146814, where it was added to silence the large atomic operation may incur significant performance penalty warning on 32-bit targets. Since NVPTX sets MaxAtomicInlineWidth to the target pointer width, nvptx64 never emits that warning, and libclc no longer builds a 32-bit nvptx target, so the guard is obsolete. The upstream PR therefore drops __NVPTX__ from clc_atomic_def.inc as well, which is the same change intel/llvm carries locally as 5f67a6c (marked "This change will be upstreamed").
|
@zjin-lcf please fix the code formatting |
e489fbc to
75b501e
Compare
|
Thanks for the review, both comments should be addressed now:
The branch was recreated so the PR is now a single dp4a commit. Apart from the formatting, the content is unchanged: ignoring whitespace, |
Summary
Lower the
sycl_ext_oneapi_dot_accumulatedot_acchelpers to the native dp4a instruction on hardware that supports it, keeping the existing scalar implementation as a portable fallback. The numerical result is identical on every path.dp4a.{s32,u32}.{s32,u32}on sm_61+ (Pascal and later) for all four operand-signedness combinations.__builtin_amdgcn_sdot4/udot4(v_dot4, gfx906+/CDNA/RDNA2) and__builtin_amdgcn_sudot4(v_dot4_i32_iu8, gfx11/RDNA3+ and gfx12/RDNA4) for the mixed-sign cases and, wheresdot4is unavailable (RDNA3+ droppedv_dot4_i32_i8), the same-sign signed case. Every path is guarded by__builtin_amdgcn_is_invocable, so architectures lacking a given instruction fall back to scalar.Cleanups
detailand drop theUs/Uuunions that leaked into the publicsycl::ext::oneapinamespace.int8_t/uint8_tviews instead of achar-based union: fixes signedness on targets where plaincharis unsigned (e.g. AArch64) and removes type-punning UB.vec<*8_t, 4>overloads nowsycl::bit_castdirectly to the packed 32-bit word.Test plan
check_device_codecodegen tests:sm_90:dp4a.s32.s32,dp4a.u32.u32,dp4a.s32.u32,dp4a.u32.s32.gfx90a,gfx942,gfx1030(sdot4/udot4) andgfx1100,gfx1200(sudot4).DotProductint + vec tests (all four sign combinations) pass on: