Repository navigation
Fully enable cache bypass access for Simple/LL/LL128 - #3812
Merged
Merged
Conversation
1 task
mustafabar
reviewed
Mar 5, 2026
Contributor
|
This breaks |
Contributor
Author
Fixed |
thananon
approved these changes
Mar 9, 2026
atulkulk
added a commit
that referenced
this pull request
Mar 13, 2026
Cherry-picked and squashed from PR #3812 (users/wenkaidu/cache_bypass). When RCCL_HAVE_GLOBAL_DWORDX4_BUILTINS is enabled (gfx942/gfx950): - load128/store128 use __builtin_amdgcn_global_{load,store}_b128 with system scope, ensuring cache-bypassing visibility for LL128 P2P buffers - LL protocol loads/stores use DWORDX4 builtins for system-scope access - Sub-16B ld_volatile/st uses __hip_atomic_{load,store} with address_space(1) pointers and __HIP_MEMORY_SCOPE_SYSTEM for correct cross-GPU visibility and global_* instruction emission - DEFINE_ld_st macros split into hip_atomic (DWORDX4) and fallback (nontemporal) variants; fallback uses address_space(1) casts - ncclLLFifoLine gains v4u member for b128 load/store on supported HW - rccl_ptr.h: define RCCL_HAVE_GLOBAL_DWORDX4_BUILTINS 0 for non-device - topo_expl build fix for rccl_ptr.h include path
mustafabar
approved these changes
Mar 16, 2026
atulkulk
approved these changes
Mar 16, 2026
3 of 4 tasks
lfmeadow
added a commit
that referenced
this pull request
Mar 17, 2026
…f-include - Cherry-pick PR #3812 (cache bypass for Simple/LL/LL128): dwordx4 system-scope loads/stores via __builtin_amdgcn_global_{load,store}_b128, hip_atomic ld/st for sub-16B accesses, v4u member in ncclLLFifoLine - Remove bundler target string probe (broken with clang 23 .llvm.offloading format); hardcode hip-amdgcn-amd-amdhsa--<arch> which is stable across all relevant compiler versions - Fix lds_pointer_transform.py self-include bug: skip add_rccl_ptr_include fallback when processing rccl_ptr.h itself - Restore CMAKE_CXX_COMPILER override in do-cmake Made-with: Cursor
Contributor
|
Great! I'm glad this landed. Thanks for the assist @wenkaidu! |
lfmeadow
added a commit
that referenced
this pull request
Mar 18, 2026
…f-include - Cherry-pick PR #3812 (cache bypass for Simple/LL/LL128): dwordx4 system-scope loads/stores via __builtin_amdgcn_global_{load,store}_b128, hip_atomic ld/st for sub-16B accesses, v4u member in ncclLLFifoLine - Remove bundler target string probe (broken with clang 23 .llvm.offloading format); hardcode hip-amdgcn-amd-amdhsa--<arch> which is stable across all relevant compiler versions - Fix lds_pointer_transform.py self-include bug: skip add_rccl_ptr_include fallback when processing rccl_ptr.h itself - Restore CMAKE_CXX_COMPILER override in do-cmake Made-with: Cursor
lfmeadow
added a commit
that referenced
this pull request
Mar 18, 2026
…f-include - Cherry-pick PR #3812 (cache bypass for Simple/LL/LL128): dwordx4 system-scope loads/stores via __builtin_amdgcn_global_{load,store}_b128, hip_atomic ld/st for sub-16B accesses, v4u member in ncclLLFifoLine - Remove bundler target string probe (broken with clang 23 .llvm.offloading format); hardcode hip-amdgcn-amd-amdhsa--<arch> which is stable across all relevant compiler versions - Fix lds_pointer_transform.py self-include bug: skip add_rccl_ptr_include fallback when processing rccl_ptr.h itself - Restore CMAKE_CXX_COMPILER override in do-cmake Made-with: Cursor
6 tasks done
corey-derochie-amd
pushed a commit
that referenced
this pull request
Jul 15, 2026
This PR completes what was left from #2847 and implements cache bypassing access for Simple/LL/LL128 protocols Since compiler only supports __builtin_amdgcn_global_load_b128 and __builtin_amdgcn_global_store_b128 at this time, system scope relaxed atomic is used for other data sizes. There should be minimal performance impact as RCCL communication buffers are still allocated as uncached. The PR does nothing with existing use cases as there is no cache to bypass. Real use case will be "Enable user buffer and graph registration feature": #2758 No observable change in performance with and without the PR. - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Yiltan <ytemucin@amd.com> Co-authored-by: Bertan Dogancay <111835151+BertanDogancay@users.noreply.github.com>
i-kosarev
added a commit
that referenced
this pull request
Oct 14, 2026
storeData has called store<> since #9336. When RCCL_HAVE_GLOBAL_DWORDX4_BUILTINS is 0 (compiler without __builtin_amdgcn_global_load_b128, e.g. ROCm 7.0.1, or DWORDX4_INTRINSICS=OFF), store<> falls back to a non-temporal store followed, on gfx950 with ROCM_VERSION < 70002, by a release fence: buffer_wbl2 sc0 sc1 plus s_waitcnt vmcnt(0) lgkmcnt(0) after every user-buffer store. The fence works around the ROCr hipHostMallocUncached bug for the comm FIFO, which storeLL still covers. store<> only writes user buffers, had no callers before #9336, and its builtin path has not carried the fence since #3812. Only gfx950 builds against ROCm < 7.0.2 without the builtins change; every other build preprocesses to the same code. JIRA ID : ROCM-31938 Co-authored-by: Cursor <cursoragent@cursor.com>
1 task done
i-kosarev
added a commit
that referenced
this pull request
Oct 16, 2026
## Motivation Since #9336, the LL `storeData` writes the user output buffer through `store<>`. Without the b128 builtins (`RCCL_HAVE_GLOBAL_DWORDX4_BUILTINS == 0`), `store<>` uses a non-temporal store. On gfx950 with `ROCM_VERSION < 70002` it also issues a release fence (`buffer_wbl2 sc0 sc1` + `s_waitcnt vmcnt(0) lgkmcnt(0)`) after every store. This makes 4 KB–64 KB `ncclBroadcast` on MI355X about 30% slower in RCCL built against ROCm 7.0.1. ## Technical Details - Removes the gfx950 `ROCM_VERSION < 70002` fence from the `store<>` fallback. - The fence is the ROCr `hipHostMallocUncached` workaround for the comm FIFO, and `storeLL` keeps it. `store<>` only writes user buffers, and its builtin path has had no fence since #3812. - Builds against ROCm 7.0.2 or later, and gfx942 builds, compile to the same code as before. ## Issue Tracking JIRA ID: ROCM-31938 ## Test Plan 4 nodes x 8 MI355X, RCCL built against ROCm 7.0.1: - `broadcast_perf` 1K–256K, interleaved with RCCL 2.27.7 and develop. - `all_reduce_perf` and `reduce_perf` 1K–4M. - Correctness with `NCCL_PROTO=LL -d all -u 1`, 8 B–1 MB, for broadcast, all_reduce, all_gather, reduce_scatter and reduce. ## Test Result `broadcast_perf` out-of-place time, ratio vs RCCL 2.27.7: | Size | develop | this PR | | --- | --- | --- | | 8K | 1.23 | 1.08 | | 16K | 1.26 | 1.08 | | 32K | 1.28 | 1.06 | | 64K | 1.31 | 1.02 | | 128K | 0.99 | 1.00 | - `all_reduce` and `reduce`: no change. - Correctness: `#wrong` = 0 for every collective, data type and size. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
This PR completes what was left from #2847 and implements cache bypassing access for Simple/LL/LL128 protocols
Technical Details
Since compiler only supports __builtin_amdgcn_global_load_b128 and __builtin_amdgcn_global_store_b128 at this time, system scope relaxed atomic is used for other data sizes.
JIRA ID
Test Plan
There should be minimal performance impact as RCCL communication buffers are still allocated as uncached. The PR does nothing with existing use cases as there is no cache to bypass. Real use case will be "Enable user buffer and graph registration feature": #2758
Test Result
No observable change in performance with and without the PR.
Submission Checklist