Skip to content

Fix Mega MoE task info slot release ordering - #441

Merged
zheanxu merged 1 commit into
mainfrom
fix-mega-moe-fence
Sep 14, 2026
Merged

zheanxu merged 1 commit into
mainfrom
fix-mega-moe-fence

Conversation

@zheanxu

@zheanxu zheanxu commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@zheanxu
zheanxu merged commit 78b6900 into main Sep 14, 2026
2 of 3 checks passed

CUTLASS_DEVICE void release_task_info() const {
// Complete metadata reads before the scheduler can overwrite this slot.
ptx::fence_acq_rel_cta();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔵 suggestion: 非阻塞建议:非 leader CTA 的消费者与位于 CTA 0 的调度 warp 是跨 CTA 同步,严格按 PTX 内存模型应使用 .cluster scope(fence.acq_rel.cluster 或 mbarrier.arrive.release.cluster.shared::cluster)。实际硬件上 fence.acq_rel.cta 已足以让本线程未完成的 shared 加载在 arrive 前完成,因此行为安全且开销更低;建议在注释中补充说明这一取舍(为何选 .cta 而非 .cluster),或在 ld_st.cuh 中同时提供 fence_acq_rel_cluster() 变体以备后续需要。

🤖 v5

@ds-review-bot

Copy link
Copy Markdown
Collaborator

🤖 ds-review-bot Code Review

v6

⚠️ 未完成评审(upstream_error:模型上游服务不可用)

v5

该提交为 Mega MoE 调度器的 task info 槽位释放路径补上了内存序保证。原实现中,消费者线程在 get_next_task() 里用普通 ld.shared 拷贝 task_infos[stage],随后在 release_task_info() 中通过 CUTLASS 的 ClusterBarrier::arrive(cta_id) 远程 arrive 到 CTA 0 的 empty barrier;该内联汇编(third-party/cutlass/include/cutlass/arch/barrier.h:485-492)没有 "memory" clobber,且 task_info 的字段(valid_m / pool_block_idx / n_cluster_idx / block_phase)都在 arrive 之后才被使用(如 sm100_bf16_mega_moe.cuh:863-872),编译器可以把字段加载下沉到 arrive 之后。一旦调度 warp 观察到 empty 并用 st_async_cluster 覆盖该槽,消费者就会读到下一个任务的元数据,导致错误的 m_idx / n_idx / block_phase(静默错误结果或越界)。新增的 ptx::fence_acq_rel_cta()(fence.acq_rel.cta + "memory" clobber)同时在编译器层阻止重排、在硬件层保证先前的 shared 加载在 arrive 前完成,能够封住这个窗口。验证结论:变更最小且正确;两个 impl(bf16 / fp8_fp4)都通过同一个 release_task_info() 受益;sched_stage_idx ^ 1 语义未变;新封装放在 /// Memory ordering 分节下与现有 ld_acq / red_xor_rel 风格一致。唯一可商榷之处是 fence scope(见评论),属非阻塞建议。总体建议合并。

v4

The MR fixes a real producer/consumer race on the double-buffered task_infos schedule slots. In MegaMoEScheduler::release_task_info(), the epilogue threads signal task_info_empty_barriers so the scheduler can reuse the slot. Previously the plain (non-atomic) reads of task_infos[sched_stage_idx] performed in get_next_task() were not explicitly ordered before the barrier arrival, so the compiler/hardware could let the scheduler overwrite the slot while metadata reads were still in flight. Adding ptx::fence_acq_rel_cta() immediately before the arrive makes the release ordering explicit, and the "memory" clobber also blocks compiler reordering. The fence scope matches the access pattern: the slot lives in the local CTA's shared memory and the barrier is the CTA/cluster mbarrier used to gate reuse. The change is minimal, well-placed in the shared scheduler header, and therefore applies to both sm100_bf16_mega_moe and sm100_fp8_fp4_mega_moe. The new helper is added next to the other memory-ordering/PTX wrappers in ld_st.cuh and is correctly annotated CUTLASS_DEVICE. No correctness issues found.

Files reviewed: 2
Issues found: 🔵 1 suggestion
Inline comments posted: 1

⚠️ Parse warning: [v6] upstream_error:模型上游服务不可用

murphymatt added a commit to fw-ai-external/DeepGEMM that referenced this pull request Sep 15, 2026
…ring (#16)

* Fix a race condition where in-flight tensormaps are updated in-place (deepseek-ai#343)

* Public release 26/07 (deepseek-ai#377)

* Public Release 26/09 (deepseek-ai#432)

* Public release 26/09

* Update News

* Fix Mega MoE task info slot release ordering (deepseek-ai#441)

---------

Co-authored-by: Ivan Komarov <Ivan.Komarov@dfyz.info>
Co-authored-by: Pengle Zhang <53843527+guyan364@users.noreply.github.com>
Co-authored-by: Zhean Xu <94977922+zheanxu@users.noreply.github.com>
vipulSharma18 added a commit to fw-ai-external/DeepGEMM that referenced this pull request Oct 13, 2026
…upstream deepseek-ai#441)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vipulSharma18 added a commit to fw-ai-external/DeepGEMM that referenced this pull request Oct 14, 2026
…upstream deepseek-ai#441)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vipulSharma18 added a commit to fw-ai-external/DeepGEMM that referenced this pull request Oct 14, 2026
…upstream deepseek-ai#441)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vipulSharma18 added a commit to fw-ai-external/DeepGEMM that referenced this pull request Oct 14, 2026
…odule (#18)

* Add sgl-project's NVFP4 MegaMoE as an additive `deep_gemm.mega_nvfp4` module

Port the SM100 NVFP4 x NVFP4 MegaMoE decode kernel and the fused
`mega_moe_pre_dispatch` kernel from sgl-project/DeepGEMM@ec226bc (#27, 593e376,
deepseek-ai#80, deepseek-ai#81) so Fireworks can drop the separate `sgl_deep_gemm` wheel.

Purely additive: the only change to an existing file is the registration hook in
csrc/python_api.cpp. No existing header changes, so no existing kernel's JIT
cache key changes.

- Device code: sgl's headers copied into deep_gemm/include/deep_gemm/mega_nvfp4/
  (kernel, pre-dispatch, layout, scheduler, barrier, math/utils/tma_copy, ptx
  ld_st/tcgen05/tma/utils, sym_buffer, and the `MmaKind` block of types.cuh),
  moved into namespace `deep_gemm::mega_nvfp4` with includes redirected and
  nothing else changed (line numbers kept, so even assert-path SASS matches).
- Host code: csrc/apis/mega_nvfp4.hpp plus heuristics/nvfp4_mega_moe.hpp and
  impls/sm100_nvfp4_mega_moe{,_pre_dispatch}.hpp, in `deep_gemm::mega_nvfp4`
  with `nvfp4`-marked names. Like the separate sgl library, these kernels keep
  their own SM count, compile for sgl's `sm_100f` target and launch without
  PDL, so `deep_gemm.set_num_sms`/`set_pdl` and these kernels never affect each
  other. Not ported: TRT-LLM weight layout, FP8 combine, BF16 and SM90 paths.
- Python: deep_gemm/mega_nvfp4 exposes the sgl_deep_gemm subset Fireworks
  uses (get_symm_buffer_for_mega_moe, transform_weights_for_mega_moe,
  mega_moe_pre_dispatch, nvfp4_mega_moe, set_num_sms, get_block_m_for_mega_moe,
  transform_ue4m3_sf_into_required_layout).
- tests/test_mega_moe_nvfp4.py ports sgl's pre-dispatch and NVFP4 alphas tests
  and adds a bytewise check of the NVFP4 pre-dispatch quantization.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(mega_nvfp4): fence task-info metadata reads before slot release (upstream deepseek-ai#441)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
zcnrex added a commit to sgl-project/DeepGEMM that referenced this pull request Oct 16, 2026
…-ai#471) into dev

Two-parent merge of upstream 1fc9c66 (deepseek-ai#441, deepseek-ai#462, deepseek-ai#471) into sgl dev
b296ce3, keeping the SGL wheel, the TORCH_LIBRARY binding and the
downstream kernels.

- Mega MoE: upstream locality-domain scheduling, 4D weight TMA, L1/L2 store
  blocks and the TMEM prefetch epilogue, with NVFP4/MXFP4, TRT-LLM weights,
  alphas, SiTU and FP8 combine carried over. TRT weights stay unlocalized.
- GEMM epilogue classes threaded through every dispatch; SM90/SM120 assert
  that no custom epilogue is set. SM120 kernels drop the head-split epilogue
  parameter, as upstream did for SM90.
- Attention takes upstream's API as-is: no clean_logits/logits_dtype, no
  legacy fp8_mqa_logits/fp8_paged_mqa_logits, no fp8_gemm_nt_skip_head_mid.
  The SM120 MQA kernels that only served the old API are removed.
- Mega gate is sqrtsoftplus-only; bf16_mega_gate defaults to it and
  get_bf16_mega_gate_config is gone.
- TORCH_LIBRARY binding: epilogue classes (Identity, Alpha,
  FP8CastWithClamp, BF16StochasticRounding) cross as
  (kind, scalar, sfd, count) into every GEMM/einsum op; shutdown and the
  locality-domain ops are registered; MLOPart reaches Python through ctypes
  callbacks installed by deep_gemm/locality_domain.py, which re-raises the
  callback's exception from the allocating call.
- sgl_deep_gemm tests pick up upstream's test changes and the new
  test_locality_domain.py.
- Dev's per-expert post-norm (#104) sits on upstream's restructured epilogue
  and combine. Its store block of 48 at block_m 192 becomes the L1 store
  block, upstream's FP8xFP4 L1/L2 blocks at 240 are kept, and
  DG_MEGA_MOE_TILES sets both L1 and L2.
- deep_jit stays at dev's 3732a3b, a descendant of upstream's 2efdab4.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
weireweire added a commit to weireweire/DeepGEMM that referenced this pull request Nov 10, 2026
…ence

Root cause: deepseek-ai#441 put `fence.acq_rel.cta` (MEMBAR.ALL.CTA) before every
task-info slot release in the epilogue. Besides the slot reads, the fence
waits for all of the thread's outstanding memory operations, including the
previous block's remote combine stores, which costs ~6 us per fp8xfp4
Mega MoE call at a 32-rank decode shape.

Fix: copy the slot after the full-barrier wait with two
`ld.acquire.cta.shared::cta.v4.u32` loads and drop the fence. The acquire
orders every later operation of the thread, including the release arrive,
after the slot reads, at the same scope as deepseek-ai#441. ptxas emits no MEMBAR or
CCTL for it, and nothing waits on unrelated stores.

Validation (32 ranks over 8 GB300 nodes, fp8xfp4, 416 experts, top-6,
108 tokens/rank, BLOCK_M 64, CUDA graph, interleaved rounds): slowest-rank
median 89.6-89.8 -> 83.5-83.8 us/call, equal to dropping the fence;
outputs bitwise identical to the fenced kernel and across 200 graph
replays x 10 calls on every rank.
zcnrex pushed a commit to sgl-project/DeepGEMM that referenced this pull request Nov 10, 2026
…ad of a fence (#145)

* Order Mega MoE task-info slot reads with acquire loads instead of a fence

Root cause: deepseek-ai#441 put `fence.acq_rel.cta` (MEMBAR.ALL.CTA) before every
task-info slot release in the epilogue. Besides the slot reads, the fence
waits for all of the thread's outstanding memory operations, including the
previous block's remote combine stores, which costs ~6 us per fp8xfp4
Mega MoE call at a 32-rank decode shape.

Fix: copy the slot after the full-barrier wait with two
`ld.acquire.cta.shared::cta.v4.u32` loads and drop the fence. The acquire
orders every later operation of the thread, including the release arrive,
after the slot reads, at the same scope as deepseek-ai#441. ptxas emits no MEMBAR or
CCTL for it, and nothing waits on unrelated stores.

Validation (32 ranks over 8 GB300 nodes, fp8xfp4, 416 experts, top-6,
108 tokens/rank, BLOCK_M 64, CUDA graph, interleaved rounds): slowest-rank
median 89.6-89.8 -> 83.5-83.8 us/call, equal to dropping the fence;
outputs bitwise identical to the fenced kernel and across 200 graph
replays x 10 calls on every rank.

* Make the task-info slot load an overload of ld_acquire_cta_shared

---------

Co-authored-by: weireweire <20922698+weireweire@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.

2 participants