fix: use a 64-bit operand for st.bulk's size in st_bulk() - #3
Conversation
|
Thanks Keita, fix looks good, and the repro/validation are solid. One side note: the current PTX ISA spec actually allows a 32-bit size operand for st.bulk since PTX ISA 9.0 ("Support for size operand with 32-bit length is introduced in PTX ISA version 9.0", st.bulk docs (https://docs.nvidia.com/cuda/parallel-thread-execution/index.html#data-movement-and-conversion-instructions-st-bulk)). Your repro shows ptxas on CUDA 13.0 still rejects the "r" form in practice, so widening to "l"(uint64_t) is the right call anyway which is valid under every PTX version regardless of what the toolchain emits. Since upstream deepseek-ai/DeepEP carries the same constraint, could you also open a PR there? |
| if (elect_one_sync()) { | ||
| asm volatile("st.bulk.weak.shared::cta [%0], %1, 0;\n" :: | ||
| "r"(static_cast<uint32_t>(__cvta_generic_to_shared(smem_ptr))), | ||
| "r"(kNumBytes) |
There was a problem hiding this comment.
Could you try CUDA13.1, 32-bit size is now supported in CUDA13.1
There was a problem hiding this comment.
Confirmed with CUDA 13.1.80: the original 32-bit "r" form and the widened 64-bit "l" form both compile for sm_100f. nvcc emits PTX .version 9.1, and the resulting object contains an sm_100 cubin.
I also tested the 32-bit form after changing the generated PTX declaration from .version 9.1 to .version 9.0; ptxas 13.1.80 still accepts it. In comparison, ptxas 13.0.88 emits PTX .version 9.0 but rejects the same 32-bit register form with Arguments mismatch for instruction 'mov'.
So you are right that the 32-bit operand is valid under PTX ISA 9.0. The observed compatibility boundary is the ptxas implementation: 13.0.88 rejects this form, while 13.1.80 accepts it. I have kept the 64-bit form because it works with both versions.
…eanups - Public build path (no pre-release artifacts): EFA installer 1.49.0 for OpenMPI/EFA runtime, rdma-core pinned past efadv_create_comp_cntr, libfabric v2.6.0amzn1.0 built from source (GDAKI floor is 2.5; the installer bundles 2.4), aws-ofi-nccl v1.21.1 with GDAKI auto-detected. Build-time gates: efadv_create_comp_cntr in rdma-core, HAVE_EFADV_CREATE_COMP_CNTR in libfabric config.h, and the GDAKI-only ncclGinPlugin_v14 export in the plugin. - Pin the floating refs: aws-ofi-nccl v1.21.1, DeepEP at the validated commit; DEEPEP_REPO/DEEPEP_REF are now build args (also makes the optional gh_token secret reachable). - Drop inert/broken bits: OFI_NCCL_GIN_TYPE / OFI_NCCL_GIN_STRONG_SIGNAL (not consumed by the plugin), EP_SUPPRESS_NCCL_CHECK, --enable-gdaki (not a real configure option), ARG TARGETARCH, blanket apt upgrade and --allow-unauthenticated, the bundled-plugin escape hatch, dead detect_cuda_major/--cuda-home, the SETUP_DEEPEP_GIN_LIB guard, orphaned internal comments. - README: env table now lists the real variables the launchers set, EFA driver >= 3.3.0 and gdrdrv prerequisites with check commands, p6/Blackwell note pending amazon-contributing/DeepEP#3. - MIT-0 headers on both sbatch files; --nccl-root now wins over EP_NCCL_ROOT_DIR; help-text fixes. Co-authored-by: Vladimir Aerov <vaerov@amazon.com>
PTX ISA 9.0 added support for a 32-bit st.bulk size operand, but ptxas 13.0.88 rejects the register form even when nvcc emits PTX .version 9.0. This breaks the affected JIT kernels on sm_100-family GPUs with CUDA 13.0. Pass kNumBytes through a 64-bit "l" constraint instead. The value is unchanged, and the 64-bit form works with both CUDA 13.0 and CUDA 13.1. A CUDA 13.1.80 compile probe emits PTX .version 9.1 and accepts both the 32-bit and 64-bit register forms. Observed on 2 x p6-b300.48xlarge instances (B300, sm_103): tests/elastic/test_ep.py fails at the first dispatch on every rank with CUDA 13.0. With this fix, the 2-node internode run of tests/elastic/test_ep.py --test-first-only passes end-to-end over EFA-GDA, with RUN_EXIT=0 on both nodes and all correctness checks green. Signed-off-by: Keita Watanabe <keitaw09@gmail.com>
fe138e3 to
dd0f872
Compare
|
Thanks for catching this. I found that the equivalent upstream change is already open as deepseek-ai/DeepEP#692. It has the same one-line fix and includes CUDA 13.1 validation, so I will not open a duplicate PR. I have also rebased this branch onto the current |
Problem
The PTX ISA defines
st.bulk's size operand as.u64, butst_bulk()indeep_ep/include/deep_ep/common/ptx.cuhpasseskNumBytesthrough a 32-bit"r"inline-asm constraint. ptxas rejects the generated PTX withArguments mismatch for instruction 'mov'. The call sits in the__CUDA_ARCH__ >= 1000branch, so every JIT kernel that includes this header fails to compile on sm_100-family GPUs (B200/B300); Hopper takes the fallback loop and never compiles this branch — which is why Hopper validation doesn't surface it, and why this branch can never have compiled successfully on any target.Repro
On 2× p6-b300.48xlarge (sm_103),
tests/elastic/test_ep.py --test-first-onlyfails at the first dispatch on every rank:Minimal standalone repro (CUDA 13.0,
--gpu-architecture=sm_100f): thest.bulkasm with"r"(size)fails ptxas with exactly this error; the same asm with"l"((uint64_t)size)compiles clean.Fix
One line: pass the size as
"l"(static_cast<uint64_t>(kNumBytes)). The value is unchanged (widening only); the address operand stays"r", which is correct for a 32-bitshared::ctaaddress; Hopper is unaffected (the branch is preprocessed out there).Validation
With this fix built from this branch, the two-node internode
tests/elastic/test_ep.py --test-first-onlypasses end-to-end over EFA-GDA (NCCL GIN, GDAKI backend) on 2× p6-b300.48xlarge —RUN_EXIT=0on both nodes with all correctness checks green; per-rank dispatch ~115 GB/s scale-out / ~375 GB/s scale-up, combine ~81 GB/s scale-out / ~267 GB/s scale-up (2026-08-21; NCCL 2.31.2, aws-ofi-nccl v1.21.1, libfabric v2.6.0amzn1.0). The passing combine correctness checks also exercise the zero-fill semantics this instruction provides.Note
Separately, single-node intranode runs on the same stack fail earlier, at
ElasticBuffercreation, withNET/IB : Requested properties for GIN GDAKI NIC 6, only 2 GIN GDAKI NICs have been created— that precedes any kernel and is unrelated to this change; happy to file it separately with logs.Upstream deepseek-ai/DeepEP carries the identical constraint, so this applies there too.