Skip to content

[Comgr][hotswap] Decline far s_add_pc_i64 trampolines - #3305

Merged
xintin merged 1 commit into
ROCm:amd-stagingfrom
xintin:amd/dev/xintin/hsv009-rccl-hotswap-fault-bisect
Jul 10, 2026
Merged

[Comgr][hotswap] Decline far s_add_pc_i64 trampolines#3305
xintin merged 1 commit into
ROCm:amd-stagingfrom
xintin:amd/dev/xintin/hsv009-rccl-hotswap-fault-bisect

Conversation

@xintin

@xintin xintin commented Jul 10, 2026

Copy link
Copy Markdown

Problem

On gfx1250 A0, RCCL all_reduce_perf faults 10/10 with hotswap enabled: every rank hits a GPU memory fault in ncclDevKernel_Generic_4 (page-not-present at ~0x10000X000). HSA_HOTSWAP_DISABLE=1 passes 10/10.

Root cause (bisected on hardware + rocgdb): when a B0 to A0 patch site sits beyond s_branch's +/-128 KB reach of the appended trampoline pool (only happens in huge objects like the ~225 MB RCCL fatbin), emitToTrampoline takes the far path, whose backward s_add_pc_i64 branch-back corrupts wave state on gfx1250 A0. The forward positive-offset s_add_pc_i64, the DS-2addr expansion, and the near NOP-sled/short-s_branch path are all correct - only the far backward long branch is at fault. The faulting kernel is unchanged; a trampolined ds_*_2addr site in an RCCL device function (e.g. runTreeUpDown) corrupts data it reads.

Fix

Decline far (Long) trampolines in emitToTrampoline - leave the site's original instruction in place rather than emit the crashing redirect. In-place patches (s_clause to s_nop, etc.) and near short-s_branch trampolines are unaffected. Unpatched ds_*_2addr is only incorrect on A0 for payload-unaligned offsets, which is strictly safer than a hard GPU fault. Small objects (rocBLAS/hipBLASLt/etc.) never hit the far path, so they are unchanged. Follow-up (tracked separately): patch far sites via a scratch-register s_getpc_b64/s_setpc_b64 branch-back, gated on an SGPR-liveness pass so it also covers non-kernel device functions.

This is a gating mechanism; as a follow-up it's the getpc/setpc + SGPR-liveness work.

Testing

  • RCCL all_reduce_perf (float, 4x gfx1250), hotswap enabled: 10/10 PASS (was 0/10), #wrong=0, ~4.3 GB/s.
  • comgr hotswap lit: 59/59; new hotswap-trampoline-ds-long-branch.s regression test encodes the RCCL ds_2addr far-path scenario (load + store, declined).
  • HotswapElfTests 17/17, HotswapMCTests 51/51, rocr hotswap_rewrite 20/20 + hotswap_gfx_query 10/10.

@xintin xintin self-assigned this Jul 10, 2026
@xintin xintin added comgr Related to Code Object Manager hotswap Related to the Comgr Hotswap feature labels Jul 10, 2026
@xintin
xintin changed the base branch from main to amd-staging July 10, 2026 06:44
@xintin
xintin marked this pull request as ready for review July 10, 2026 06:44
@xintin
xintin requested review from chinmaydd and lamb-j as code owners July 10, 2026 06:44
@xintin
xintin force-pushed the amd/dev/xintin/hsv009-rccl-hotswap-fault-bisect branch from 070a2d0 to 3c24f63 Compare July 10, 2026 06:52
@xintin
xintin force-pushed the amd/dev/xintin/hsv009-rccl-hotswap-fault-bisect branch from 3c24f63 to c3e574d Compare July 10, 2026 06:54
@xintin
xintin merged commit 6579a19 into ROCm:amd-staging Jul 10, 2026
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comgr Related to Code Object Manager hotswap Related to the Comgr Hotswap feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants