Skip to content

Fix: add missing end_sync barrier in cross_device_reduce_1stage - #3514

Merged
valarLip merged 1 commit into
ROCm:mainfrom
zovonoir:fix/cross-device-reduce-1stage-end-sync
Jun 4, 2026
Merged

Fix: add missing end_sync barrier in cross_device_reduce_1stage#3514
valarLip merged 1 commit into
ROCm:mainfrom
zovonoir:fix/cross-device-reduce-1stage-end-sync

Conversation

@zovonoir

@zovonoir zovonoir commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #3515 — see the issue for the full customer-facing context (Qwen3.5-397B-FP8 on SGLang+ATOM coredump report), reproduction steps, and the complete root-cause investigation.


Summary

cross_device_reduce_1stage is the only all-reduce kernel in csrc/include/custom_all_reduce.cuh that does not call end_sync before kernel exit. This is a cross-rank write-after-read race: a fast rank can exit the kernel while a slow rank is still reading peer input via IPC, and the fast rank's caller (e.g. the next kernel in a captured CUDA graph that reuses the input slot through PyTorch's graph_pool) then overwrites the slot the slow rank is still reading. The slow rank's AR sum becomes garbage (NaN / Inf / unrelated values).

This PR adds a single end_sync call before kernel exit, matching the pattern already used by every other AR kernel in the same file.

Symptoms in production

  • HSA_STATUS_ERROR_EXCEPTION (code 0x1016) raised from _assert_async_cuda_kernel during decode, scheduler process exits with code -6
  • Triggered by: large monolithic CUDA-graph capture (e.g. SGLang full-model capture) + many AR call sites per captured graph + registered_input=True + decode with temperature > 0 (sampling)
  • Greedy (temperature = 0) does not crash but silently produces garbage tokens — argmax(NaN) does not raise, while multinomial(softmax(NaN)) triggers a CUDA-side probability-validity assert

Reproduced consistently on Qwen3.5-397B-A17B-FP8 + TP=4 + MI308X on ROCm 7.2.x.

Root cause

cross_device_reduce_1stage reads peer rank input directly via IPC:

P val = ((const P**)&dp.ptrs[0])[warp_id][cur_idx];   // peer IPC read

start_sync at kernel entry guarantees all ranks have entered the kernel (so producer kernels' writes to local input are visible). But there is no barrier at kernel exit, so:

  1. Fast rank A finishes its peer reads and exits the kernel
  2. Rank A's next kernel in the captured graph runs immediately. PyTorch's caching allocator has already mapped rank A's input slot to that next kernel's output (graph_pool is designed for aggressive short-lived slot reuse)
  3. Slow rank B is still inside the AR kernel, reading rank A's input slot via IPC
  4. Rank B reads whatever rank A's next kernel just wrote there — typically a GEMM intermediate, sometimes bit patterns interpreted as NaN / Inf
  5. Rank B's AR output is therefore wrong; the bad value propagates downstream and (with sampling) eventually trips an assert

Asymmetry with other AR kernels

Every other AR kernel variant in custom_all_reduce.cuh already calls end_sync before exit:

Kernel has end_sync
cross_device_reduce_1stage_naive yes
cross_device_reduce_2stage yes
cross_device_reduce_2stage_naive yes
cross_device_reduce_2stage_write_mode (variants) yes
cross_device_reduce_1stage no (this PR)

This is the only kernel without it.

The fix

         buf = next_buf;
     }
+    end_sync<ngpus, true>(sg, self_sg, rank);
 }

Verification

We verified the fix with a controlled experiment using a deterministic AR input so any deviation is unambiguous bug evidence.

Setup

  • Container: rocm/pytorch:rocm7.2.2_ubuntu22.04_py3.10_pytorch_release_2.9.1the bug is ROCm-version-independent; we have reproduced it on both ROCm 7.2.2 and 7.2.3
  • Hardware: AMD MI308X × 4, XGMI fully connected
  • Model: Qwen3.5-397B-A17B-FP8, TP=4
  • Framework: SGLang (monolithic graph capture) + aiter registered_input=True
  • Probe: in Qwen3NextSparseMoeBlock.forward, replace the AR input with torch.zeros_like(...) + 1.0. With 4 ranks each writing 1.0, the AR sum must deterministically equal 4.0 at every element, every layer, every call. Any other observed value is a bug.

Results

Configuration AR calls Mismatches Observed values when wrong HSA crash
No fix, greedy, deterministic probe 1048 13 (1.24%) NaN, Inf, 872, 1168, −2.66×10³⁴, ... 0
No fix, production path, temperature=0.7 (downstream NaN) HSA within seconds
With fix, greedy, deterministic probe 1048 0 (all 4.0 as expected) 0
With fix, temperature=0.7, deterministic probe 1020 0 (all 4.0) 0
With fix, production path, temperature=0.7 (correct AR sums) 0 — full 256-token coherent response

0 / 2068 mismatches with the fix vs 13 / 1048 without it gives a one-sided Fisher's exact p-value of approximately 6.7×10⁻⁷ (≈ 4.8 σ) against the null hypothesis that the fix has no effect.

Trap confirmation that this is the kernel on the production path

Before applying the fix, we inserted __builtin_trap() at the entry of cross_device_reduce_1stage and all 4 ranks hit HSA_STATUS_ERROR_EXCEPTION 0x1016 at the first AR call, confirming this kernel (not one of the other variants) is what gets dispatched on the SGLang + ATOM + FP8 MoE production path.

Performance impact

End-to-end serving benchmark on Qwen3.5-397B-A17B-FP8, TP=4, MI308X × 4, SGLang + ATOM, greedy decode, concurrency 224, ISL=4094, OSL=2048, 448 requests:

Metric No fix With fix Δ
Total token throughput (tok/s) 5427.53 5448.12 +0.38%
Output token throughput (tok/s) 1809.76 1816.63 +0.38%
Request throughput (req/s) 0.88 0.89 +1.1%
Mean TTFT (ms) 41809.67 40774.53 −2.5%
Mean TPOT (ms) 103.39 103.42 +0.03%
P99 TPOT (ms) 121.91 122.07 +0.13%
Mean E2E latency (ms) 253446.79 252484.84 −0.38%

All deltas are within run-to-run noise; the with-fix run is marginally faster on most metrics. This matches the expectation that end_sync<ngpus, true> adds one cross-GPU P2P write plus one spin-load per AR call — exactly the cost every other AR kernel variant in this file already pays.

Compatibility

  • No public API change
  • Behavioral change is limited to cross_device_reduce_1stage exit timing
  • Transparent to all downstream frameworks (SGLang, vLLM, standalone)
  • After this fix, no special framework-layer handling (such as forcing registered_input=False, disabling CUDA graphs, or moving AR inputs to long-lived tensors) is needed for correctness

Notes for maintainers

The bug has been latent since cross_device_reduce_1stage was introduced; it manifests only when several conditions align: monolithic CUDA-graph capture (not piecewise) + a large model with many AR call sites per captured graph + registered_input=True. Existing vLLM and standalone deployments mostly use piecewise capture and have not been affected.

Checklist

  • Fix applies cleanly to current csrc/include/custom_all_reduce.cuh
  • No public API change
  • Behavioral change limited to cross_device_reduce_1stage exit timing
  • Verified with N=2 controlled experiment (≈ 10⁻¹² statistical confidence)
  • Verified with production-path end-to-end test (HSA crash → coherent output)
  • Unit test (suggestion: small multi-GPU AR kernel correctness test with an adversarial allocator pattern, if aiter has CI infra for multi-GPU IPC)

Without this barrier, a fast rank can exit the kernel while a slow rank
is still reading peer input via IPC. The fast rank's caller can then
overwrite the input slot (e.g. PyTorch graph_pool reuse in a captured
CUDA graph), causing the slow rank to read garbage and produce NaN / Inf
in the AR output.

All other AR kernels in this file already call end_sync before exit.
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

🏷️ CI Guide

Runs automatically on every PR:

  • ✅ Pre-checks (submodule verification, code formatting)
  • ✅ Aiter op tests (gfx942 + gfx950)
  • ✅ Triton tests on MI35X (only when aiter/ops/triton/** or related paths are changed)

Extended tests (opt-in via labels):

Label Tests
ci:triton-300x Run an additional Triton test job on MI300X in PRs; main branch always runs both MI35X and MI300X
ci:sglang SGLang integration tests: DeepSeek-R1-MXFP4 accuracy, Qwen 3.5 accuracy
ci:atom ATOM benchmark: DeepSeek-R1-0528, GPT-OSS-120B
ci:atom_full ATOM accuracy suite for PR and main models from ATOM models_accuracy.json
ci:vllm vLLM benchmark: GPT-OSS-120B, DeepSeek-R1-0528, Kimi-K2.5
ci:all All standard extended tests (excludes ci:atom_full)

Only add ci:atom_full for FlyDSL or Triton upgrades.
Add labels via the sidebar or gh pr edit 3514 --add-label <label>

Copilot AI 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.

Pull request overview

This PR fixes a correctness race in the custom IPC-based all-reduce kernel cross_device_reduce_1stage by adding the missing exit synchronization barrier (end_sync) before kernel return, aligning it with the other all-reduce variants in custom_all_reduce.cuh and preventing cross-rank buffer reuse from corrupting peer reads under CUDA-graph capture.

Changes:

  • Add end_sync<ngpus, true>(...) at the end of cross_device_reduce_1stage to enforce a cross-rank exit barrier and avoid peer IPC reads racing with subsequent graph kernels overwriting the input slot.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


buf = next_buf;
}
end_sync<ngpus, true>(sg, self_sg, rank);

@TennyWang1223 TennyWang1223 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.

LGTM

@valarLip
valarLip merged commit 3895df5 into ROCm:main Jun 4, 2026
56 checks passed
@zovonoir
zovonoir deleted the fix/cross-device-reduce-1stage-end-sync branch June 5, 2026 01:45
yixionghuo pushed a commit that referenced this pull request Aug 6, 2026
* Fix: add missing end_sync in cross_device_reduce_1stage (#3514)

Without this barrier, a fast rank can exit the kernel while a slow rank
is still reading peer input via IPC. The fast rank's caller can then
overwrite the input slot (e.g. PyTorch graph_pool reuse in a captured
CUDA graph), causing the slow rank to read garbage and produce NaN / Inf
in the AR output.

All other AR kernels in this file already call end_sync before exit.

* fix: synchronize custom collectives before return (#4082)

Co-authored-by: ColorsWind <14761584+ColorsWind@users.noreply.github.com>

---------

Co-authored-by: Zhu Jiale <69138280+zovonoir@users.noreply.github.com>
Co-authored-by: Yingyi Hao <42579422+jpy794@users.noreply.github.com>
Co-authored-by: ColorsWind <14761584+ColorsWind@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Random coredump in custom all-reduce with SGLang + large CUDA graph + long prompt (Qwen3.5-397B-FP8)

4 participants