Skip to content

fix(rccl): default cheap post-send fence off for gfx950, add force-on override - #10339

Merged
wenkaidu merged 4 commits into
ROCm:developfrom
wenkaidu:rccl/gfx950-cheap-fence-default-off
Aug 19, 2026
Merged

wenkaidu merged 4 commits into
ROCm:developfrom
wenkaidu:rccl/gfx950-cheap-fence-default-off

Conversation

@wenkaidu

Copy link
Copy Markdown
Contributor

Motivation

default cheap post-send fence off for gfx950, add force-on override

Technical Details

The arch-tuned auto path enabled the cheap post-send fence (skipping the full __threadfence_system()) on gfx942, gfx950 and gfx1250. Drop gfx950 from the auto-enable set so it defaults to the full fence, while gfx942/gfx1250 are unchanged.

Extend RCCL_CHEAP_POST_SEND_FENCE_OFF with a new value 2 that force-enables the cheap fence regardless of arch (overriding auto), so gfx950 can be re-enabled when desired. Values remain: 0 = arch-tuned auto, 1 = force off, 2 = force on.

Issue Tracking

JIRA ID AICOMRCCL-1873)

Test Plan

Confirm cheap fence can be turned on/off through env var

Test Result

Passed

Submission Checklist

… override

The arch-tuned auto path enabled the cheap post-send fence (skipping the full
__threadfence_system()) on gfx942, gfx950 and gfx1250. Drop gfx950 from the
auto-enable set so it defaults to the full fence, while gfx942/gfx1250 are
unchanged.

Extend RCCL_CHEAP_POST_SEND_FENCE_OFF with a new value 2 that force-enables the
cheap fence regardless of arch (overriding auto), so gfx950 can be re-enabled
when desired. Values remain: 0 = arch-tuned auto, 1 = force off, 2 = force on.

Co-authored-by: Cursor <cursoragent@cursor.com>
@wenkaidu
wenkaidu requested review from a team August 18, 2026 17:35
@therock-pr-bot

therock-pr-bot Bot commented Aug 18, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

Comment thread projects/rccl/src/init.cc Resolved
Address review feedback on the cheap post-send fence change:
- Extract the decision into rcclComputeCheapPostSendFenceOff(cudaArch, param,
  uncachedMemSupported) declared in rccl_common.h, so the call site in
  initTransportsRank collapses to a single assignment.
- Switch arch detection from IsArchMatch() string matching to the existing
  numeric comm->cudaArch (gfx942 = 940, gfx950 = 950, gfx1250 = 1250).
- Add CheapPostSendFenceTests unit test covering the full matrix (uncached-mem
  gating, arch-tuned auto, force-off, force-on override, other params, unknown
  arch).

Co-authored-by: Cursor <cursoragent@cursor.com>

@alex-breslow-amd alex-breslow-amd left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome! Please verify that test failures aren't from your PR.

@amd-jiali
amd-jiali self-requested a review August 18, 2026 19:37
wenkaidu and others added 2 commits August 18, 2026 15:26
initTransportsRank() uses goto fail for error handling; a local
const bool uncachedMemSupported sat after those labels and failed to
compile. Pass the HIP_UNCACHED_MEMORY flag inline instead.

Co-authored-by: Cursor <cursoragent@cursor.com>
Define rcclComputeCheapPostSendFenceOff inline in rccl_common.h so
rccl-UnitTestsFixtures can exercise it without importing a hidden
librccl symbol, and move CheapPostSendFenceTests there.

Co-authored-by: Cursor <cursoragent@cursor.com>
@wenkaidu
wenkaidu merged commit af691cb into ROCm:develop Aug 19, 2026
113 of 123 checks passed
@wenkaidu
wenkaidu deleted the rccl/gfx950-cheap-fence-default-off branch August 19, 2026 14:16
cmcknigh pushed a commit that referenced this pull request Aug 25, 2026
… override (#10339)

## Motivation

default cheap post-send fence off for gfx950, add force-on override

## Technical Details

The arch-tuned auto path enabled the cheap post-send fence (skipping the
full __threadfence_system()) on gfx942, gfx950 and gfx1250. Drop gfx950
from the auto-enable set so it defaults to the full fence, while
gfx942/gfx1250 are unchanged.

Extend RCCL_CHEAP_POST_SEND_FENCE_OFF with a new value 2 that
force-enables the cheap fence regardless of arch (overriding auto), so
gfx950 can be re-enabled when desired. Values remain: 0 = arch-tuned
auto, 1 = force off, 2 = force on.

## Issue Tracking

JIRA ID AICOMRCCL-1873)
## Test Plan

Confirm cheap fence can be turned on/off through env var

## Test Result

Passed

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
alexander-sannikov added a commit that referenced this pull request Oct 30, 2026
Backported PRs:
- #10670 fix(rccl): LL protocol explicit 128-bit vector loads/stores
- #10339 fix(rccl): default cheap post-send fence off for gfx950
- #9336 LL: cache-bypass user buffers, plain/non-temporal comm FIFO

Partially backported PRs:
- #9347 [RCCL] [JIRA ID : ROCM-25479] LL128: generate separate kernels for UBR

Aligned with develop:
- RCCL_GFX9_CHEAP_FENCE_OFF default

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants