Skip to content

hybrid unordered kernels: guard token_map_at_dispatch bit packing at JIT time - #12

Open
Xuan-1998 wants to merge 1 commit into
amazon-contributing:mainfrom
Xuan-1998:combine-recv-addr-overflow-guard
Open

hybrid unordered kernels: guard token_map_at_dispatch bit packing at JIT time#12
Xuan-1998 wants to merge 1 commit into
amazon-contributing:mainfrom
Xuan-1998:combine-recv-addr-overflow-guard

Conversation

@Xuan-1998

Copy link
Copy Markdown

The bug

pack_combine_recv_addr in deep_ep/include/deep_ep/impls/combine_utils.cuh:38-40 packs (rank, slot, channel) into an int32 by shifting into 5/14/12 bit fields with no runtime mask:

int pack_combine_recv_addr(const int& rank, const int& slot, const int& channel) {
    return (rank << kCombineRecvMapRankShift) | (slot << kCombineRecvMapSlotShift) | channel;
}

Callers pass unchecked ints:

  • hybrid_dispatch_unordered.cuh:704 writes the packed word into token_map_at_dispatch[token][k].
  • hybrid_combine_unordered.cuh:719 reads it back and asserts only channel == channel_idx (the low 12 bits).

If slot exceeds its 14-bit field (16383), its high bits bleed into the rank field at bit 26. channel_idx stays intact, so the runtime EP_DEVICE_ASSERT(channel == channel_idx) at hybrid_combine_unordered.cuh:722 still passes. The kernel then routes the partial to a wrong scale-out rank and either misdirects the reduction or hangs in the arrival-count wait at :752-767 because the corrupted rank never sends.

Slot is combine_slot, bounded by kNumMaxTokensPerChannel * kNumTopk. Both rank (bounded by kNumScaleoutRanks, 5-bit field) and channel (bounded by kNumChannels, 12-bit field) have the same silent-overflow shape at 32 / 4096.

The fix

Instantiations of the two hybrid unordered kernels have all three bounds as compile-time constants. Add EP_STATIC_ASSERTs at the top of each kernel body, so an out-of-range template combination fails to instantiate at NVRTC JIT compile time (per csrc/kernels/elastic/dispatch.hpp:108) with a message that names the offending parameter — instead of a silent misrouted put at runtime.

Diff

  • deep_ep/include/deep_ep/impls/combine_utils.cuh: a 6-line comment above pack_combine_recv_addr documenting the caller contract and pointing at the enforcing sites.
  • deep_ep/include/deep_ep/impls/hybrid_dispatch_unordered.cuh: three EP_STATIC_ASSERTs at the top of hybrid_unordered_dispatch_impl (line 180 area).
  • deep_ep/include/deep_ep/impls/hybrid_combine_unordered.cuh: same three EP_STATIC_ASSERTs at the top of hybrid_unordered_combine_impl (line 59 area).

No runtime path change. Total 35 lines added.

Verification

  • Parse-only check with nvcc -std=c++17 -arch=sm_90 -c against the real nccl.h: passes.
  • The kernels are NVRTC-instantiated at runtime, so the new asserts fire during JIT compile with a message pointing at the offending parameter (before any launch).
  • Reviewed each cited line against source at HEAD (54fffef) on p5en48xlarge.

AI disclosure

Found and drafted with Claude Code. Every claim above was verified against source line by line before I opened the PR.

`pack_combine_recv_addr` in `combine_utils.cuh` packs rank/slot/channel
into 5/14/12 bits by shifting and ORing without any mask. Callers pass
unchecked ints:

  - `hybrid_dispatch_unordered.cuh` writes the packed word into
    `token_map_at_dispatch[token][k]`
  - `hybrid_combine_unordered.cuh` reads it back and asserts only that
    the channel bits match the channel it is running on

If `slot` exceeds 16383 (its 14-bit field) it bleeds into the rank
field at bit 26; the channel bits stay intact so the runtime
`EP_DEVICE_ASSERT(channel == channel_idx)` still passes. The result is
a misrouted combine partial with no visible failure, or a hang in the
arrival-count wait when the corrupted rank never sends.

Slot is `combine_slot`, bounded by `kNumMaxTokensPerChannel * kNumTopk`.
For a large `kNumMaxTokensPerRank` with few channels and topk >= 8
this can exceed 16383. rank and channel have the same silent-overflow
shape at 32 / 4096. All three bounds are compile-time constants on
the two kernel templates, so `EP_STATIC_ASSERT` at the top of each
kernel body catches the bad instantiation at NVRTC JIT time with a
clear message that names the offending template parameter, instead
of at runtime with a misrouted put.
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.

1 participant