Skip to content

Add switch-channel NVLS cross-rank barrier#843

Open
Empyreus wants to merge 5 commits into
mainfrom
rjsouza/nvls-barrier
Open

Add switch-channel NVLS cross-rank barrier#843
Empyreus wants to merge 5 commits into
mainfrom
rjsouza/nvls-barrier

Conversation

@Empyreus

Copy link
Copy Markdown
Contributor

Adds a device-side cross-rank barrier built on NVLS (multimem) atomics

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Empyreus added 5 commits July 17, 2026 17:37
(cherry picked from commit 51a8235)
(cherry picked from commit 38494de)
(cherry picked from commit 5f93881)
(cherry picked from commit 921b280)
(cherry picked from commit 1d55a28)

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 adds a device-side, cross-rank barrier for NVLS (multimem) SwitchChannels and wires it through the executor and Python DSL so kernels can synchronize all ranks in an NVLS group without an O(n²) semaphore mesh.

Changes:

  • Add NVLS-backed barrier state to NvlsConnection/SwitchChannel and expose it via SwitchChannelDeviceHandle::barrier().
  • Introduce a new executor op (MULTI_BARRIER / "gbarrier") with device-side implementation that grid-syncs then issues a single multimem arrival per rank.
  • Add Python DSL support (SwitchChannel.barrier(...)) plus a single-node allgather example that uses the new barrier.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/core/switch_channel.cc Creates an auxiliary NVLS multicast for barrier state and stamps barrier pointers onto bound SwitchChannels.
src/core/include/execution_kernel.hpp Implements MULTI_BARRIER device handler (grid-converge → one multimem barrier → release).
src/core/include/execution_common.hpp Adds OperationType::MULTI_BARRIER.
src/core/executor/execution_plan.cc Parses "gbarrier" and maps JSON switch_channel_id into Operation.nvlsInputIndex.
include/mscclpp/switch_channel.hpp Stores barrier resources on NvlsConnection and propagates them into SwitchChannel handles.
include/mscclpp/switch_channel_device.hpp Adds SwitchChannelDeviceHandle::barrier() API and barrier metadata fields.
python/mscclpp/language/internal/types.py Adds DSL instruction enum value for "gbarrier".
python/mscclpp/language/internal/operations.py Adds GroupBarrier operation with instancing semantics collapsed onto one per-rank syncer.
python/mscclpp/language/internal/buffer_access.py Treats group_barrier as an access-sequence breaker like other barriers/nops.
python/mscclpp/language/channel.py Adds SwitchChannel.barrier() plus rank-view convenience wrapper and docstrings.
python/mscclpp/language/tests/single_node/allgather_nvls_zero_copy_barrier.py Example program exercising the new switch-native barrier.

Comment thread include/mscclpp/switch_channel_device.hpp
Comment on lines +312 to +315
// One-time sync so every rank has bound its barrier flag into the multicast before any rank issues
// a multimem operation on it at kernel time. Synchronize only allRanks (not the whole bootstrap,
// which may include ranks outside this NVLS group).
bootstrap->groupBarrier(allRanks);
Comment on lines +21 to +26
// Barrier state inherited from the owning NvlsConnection (see NvlsConnection::bindAllocatedMemory).
// All are null / zero if the connection was created without barrier support.
uint32_t* barrierLocalFlag_ = nullptr;
uint32_t* barrierMcFlag_ = nullptr;
uint32_t* barrierGen_ = nullptr;
int barrierNRanks_ = 0;

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.

Do you think it's better to have a switchChannelSemaphore to store these fields? SwitchChannel only hold the semaphore obj, same as memoryChannel

Comment on lines +75 to +98
MSCCLPP_DEVICE_INLINE void barrier(cuda::memory_order memoryOrder = cuda::memory_order::relaxed,
[[maybe_unused]] int64_t maxSpinCount = 100000000) {
// Guard against calling barrier() on a channel whose connection has no barrier support. This is
// a debug-only diagnostic; in release builds a null pointer here dereferences and crashes, which
// is intentionally preferred over a silent no-op barrier (that would hide a cross-rank race).
MSCCLPP_ASSERT_DEVICE(barrierGen != nullptr, "SwitchChannel::barrier() called without barrier support");
// Advance this rank's private target. All ranks advance identically, so targets stay in lock-step.
const uint32_t target = (*barrierGen += static_cast<uint32_t>(nRanks));
// Signal arrival: one multimem add increments every rank's copy of the counter through the switch.
if (memoryOrder == cuda::memory_order::relaxed) {
// Relaxed arrival: pure execution barrier, no data-visibility ordering.
asm volatile("multimem.red.relaxed.sys.add.u32 [%0], %1;" ::"l"(mcBarrierFlag), "r"(1U) : "memory");
} else {
// Release arrival publishes this rank's prior writes before the arrival is observed by peers.
asm volatile("multimem.red.release.sys.add.u32 [%0], %1;" ::"l"(mcBarrierFlag), "r"(1U) : "memory");
}

cuda::memory_order waitOrder =
(memoryOrder == cuda::memory_order::relaxed) ? cuda::memory_order::relaxed : cuda::memory_order::acquire;
// Wait until every rank has arrived. The signed (wrap-safe) compare means "counter is behind target".
POLL_MAYBE_JAILBREAK(
(static_cast<int32_t>(atomicLoad<uint32_t, scopeSystem>(localBarrierFlag, waitOrder) - target) < 0),
maxSpinCount);
}

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.

How about split this API into signal and wait, same as the other channel.

if (
operation.name == Instruction.nop
or operation.name == Instruction.barrier
or operation.name == Instruction.group_barrier

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.

Also here change to switchChannel.signal() swithcChannel.wait(), not barrier API

@Binyang2014
Binyang2014 requested a review from chhwang July 20, 2026 15:55
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.

3 participants