Repository navigation
Conversation
nvshmemi_add_reduce_mcast_threadroup() reduces 16-, 8- and 4-byte aligned words and returns without touching anything else, so for half and bfloat16 a slice that starts on an odd element is not written at all. nvshmemi_reduce_threadgroup() only checks that the total length is a multiple of four bytes before choosing NVLS, so this happens when: - the two-shot algorithm gives each PE an odd number of elements (nreduce = 1048574 on 2 PEs leaves half the output unwritten); - a caller passes 2-byte aligned dest/source pointers (buf + 1 leaves the whole output unwritten). Require 4-byte aligned dest/source for the NVLS reduce and reducescatter paths and an even per-PE slice for 16-bit two-shot. rdxn_on_stream_kernel() reaches both cases by itself: it splits nreduce evenly over its CTAs (16 above 256 KB) without regard to alignment, so each CTA can get an odd per-PE slice (nreduce = 131104 on 2 PEs leaves 65568 elements unwritten) and the last CTA can start on an odd element (nreduce = 1048573 leaves its 65548 elements unwritten). With the checks above those CTAs would fall back to the non-NVLS path. Split on_stream reductions into 16-byte multiples instead, so that every CTA but the last starts aligned and, with 2 or 4 PEs, keeps NVLS. Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0z5a
force-pushed
the
fix-nvls-rdxn-alignment
branch
from
October 25, 2026 03:46
41c1046 to
80fa764
Compare
Contributor
Author
|
A correction to the description. The on-stream reduction uses 16 CTAs by default, not 32. With 16, the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Incremental review: three small hunks against
devel(2dca770):src/include/non_abi/device/coll/reduce.cuh,src/include/non_abi/device/coll/reducescatter.cuhandsrc/host/stream/coll/rdxn/reduce_common.cuh.Problem
nvshmemi_add_reduce_mcast_threadroup()handles 16-, 8- and 4-byte aligned words and then returns. Forhalfand__nv_bfloat16, a range that starts on an odd element is therefore not written at all.nvshmemi_reduce_threadgroup()only checks(nreduce * sizeof(TYPE)) % 4 == 0before choosing an NVLS algorithm, so this is reachable with ordinary arguments:nreduce / npeselements, which can be odd. With 2 PEs andnreduce = 1048574, half of the output is not written.dest/sourcepointers, e.g.buf + 1. With an evennreduce, none of the output is written.The affected outputs keep whatever was in the destination before. Nothing reports an error.
The on-stream API reaches both cases by itself.
rdxn_on_stream_kernel()splitsnreduceevenly over its CTAs (16 above 256 KB) without regard to alignment. Withnreduce = 131104on 2 PEs, every CTA gets 8194 elements and runs two-shot with an odd 4097-element slice, leaving 4098 outputs per CTA unwritten (65568 in total). Withnreduce = 1048573, the first 15 CTAs get an odd byte length and correctly take the non-NVLS path, but the last CTA starts on an odd element and writes nothing (65548 outputs).Fix
destandsourceare 4-byte aligned.Calls that no longer qualify for NVLS take the existing non-NVLS algorithms.
Validation
One node, 2 PEs on two B300 GPUs (NVSwitch, CUDA multicast supported), CUDA 13.3, MPI bootstrap.
develwas built forsm_103with and without this change. A small test program fills the destination with0xffffand counts elements that are still0xffffafterwards. It also compares against theNVSHMEM_DISABLE_NVLS=1result. bf16 is shown below; fp16 is identical.nreduce*_sum_reduce_on_streamnvshmemx_*_sum_reduce_block4 PEs (four B300 GPUs, same builds and test program). bf16 is shown below; fp16 is identical.
nreduceAfter the change, nothing is left unwritten in any case above, bf16 and fp16, on both APIs. With 2 PEs every output is one of the two representable neighbours of the exact sum. With 4 PEs that holds wherever NVLS runs; the CTAs that take the non-NVLS path give bitwise the same results as
NVSHMEM_DISABLE_NVLS=1, which accumulates in the 16-bit type step by step, so about 9% of those are not neighbours of the exact sum. Within NVLS, the remaining differences against theNVSHMEM_DISABLE_NVLS=1result are the usual NVLS rounding of 16-bit sums, which is not round-to-nearest.Some cases get slower because work that used to leave outputs unwritten now takes the non-NVLS algorithm: the block-scope calls with an odd per-PE slice, and on 4 PEs the on-stream 524292 case, where the last CTA (32772 elements, 8193 per PE) falls back. Splitting off the remainder and handling it separately would keep almost all of it on NVLS, but I kept this change minimal.
Not tested: 8 or more PEs, multi-node NVLS, and min/max reductions, which use the same helpers. With 8 or more PEs a 16-byte CTA slice can still give an odd per-PE slice; those CTAs then take the non-NVLS path, which is correct but slower. Aligning the split to 16 bytes times the team size would avoid that. A unit test in
test/could fill the destination and check odd lengths and offsets. I can add one if you tell me where you would like it.The commit carries a DCO sign-off.