Skip to content

[CRL] Add intra-node P2P transport and migrate to window-based API#494

Merged
MC952-arch merged 8 commits into
flagos-ai:mainfrom
MC952-arch:rma-p2p
Jun 22, 2026
Merged

[CRL] Add intra-node P2P transport and migrate to window-based API#494
MC952-arch merged 8 commits into
flagos-ai:mainfrom
MC952-arch:rma-p2p

Conversation

@MC952-arch

@MC952-arch MC952-arch commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

PR Category

CRL

PR Types

New Features

PR Description

This PR introduces a window-based, stream-integrated one-sided RMA API (intended to be NCCL-aligned) and adds an intra-node D2D “bypass” path in the hetero RMA proxy, while updating the host_api perf benchmarks and CI to exercise the new API.

Copilot AI left a comment

Copy link
Copy Markdown

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 introduces a window-based, stream-integrated one-sided RMA API (intended to be NCCL-aligned) and adds an intra-node D2D “bypass” path in the hetero RMA proxy, while updating the host_api perf benchmarks and CI to exercise the new API.

Changes:

  • Replaces the public flagcxPutSignal/flagcxSignal/flagcxWaitSignal API surface with new window/stream-based signatures and updates the host_api perf tests accordingly.
  • Extends the hetero RMA proxy with GPU-visible completion sequence counters for stream-based flush/wait, plus initial scaffolding for intra-node IPC/D2D bypass.
  • Adds a new RMA test script and a GitHub Actions workflow to build and run the perf-based RMA tests.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 13 comments.

Show a summary per file
File Description
test/script/rma_test.sh New script to run RMA perf binaries under MPI.
test/perf/host_api/test_put.cpp Migrates put benchmark to window-based RMA API.
test/perf/host_api/test_one_side_register.cpp Migrates multi-comm one-sided registration test to window-based API.
test/perf/host_api/test_get.cpp Migrates get benchmark signaling/waiting to the new API.
flagcx/include/flagcx.h Updates public one-sided API declarations to window/stream-based forms.
flagcx/flagcx.cc Implements the new high-level one-sided APIs and WaitSignal logic.
flagcx/core/include/flagcx_hetero.h Adds hetero proxy state for stream-based synchronization and IPC bypass.
flagcx/core/flagcx_hetero.cc Implements stream-based flush and intra-node D2D bypass helpers.
.github/workflows/unittest-rma.yml Adds CI job to build and run the RMA perf tests.
Comments suppressed due to low confidence (1)

test/perf/host_api/test_one_side_register.cpp:69

  • Receiver waits with desc = {1, senderRank} but the current flagcxWaitSignal implementation flushes local outgoing opSeqs for that peer, so it won't reliably wait for the incoming PutSignal completion in this test.
    flagcxWaitSignalDesc_t desc = {1, senderRank};
    res = flagcxWaitSignal(1, &desc, comm, waitStream);
    if (res != flagcxSuccess) {
      fprintf(stderr, "[rank %d] flagcxWaitSignal failed (err=%d)\n", proc,
              int(res));
      return false;
    }
    devHandle->streamSynchronize(waitStream);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread flagcx/include/flagcx.h
Comment thread flagcx/flagcx.cc Outdated
Comment thread flagcx/flagcx.cc Outdated
Comment thread flagcx/flagcx.cc Outdated
Comment thread flagcx/flagcx.cc
Comment thread test/perf/host_api/test_get.cpp
Comment thread flagcx/core/flagcx_hetero.cc
Comment thread flagcx/include/flagcx.h
Comment thread test/script/rma_test.sh Outdated
Comment thread flagcx/flagcx.cc Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 15 comments.

Comment thread test/perf/host_api/test_put.cpp
Comment thread test/perf/host_api/test_put.cpp
Comment thread test/perf/host_api/test_get.cpp Outdated
Comment thread test/perf/host_api/test_get.cpp Outdated
Comment thread test/perf/host_api/test_get.cpp Outdated
Comment thread flagcx/flagcx.cc
Comment thread flagcx/flagcx.cc Outdated
Comment thread flagcx/core/flagcx_hetero.cc
Comment thread flagcx/core/flagcx_hetero.cc Outdated
Comment thread flagcx/core/sym_heap.cc

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.

Comment thread flagcx/core/flagcx_hetero.cc Outdated
Comment thread flagcx/core/flagcx_hetero.cc Outdated
Comment thread flagcx/flagcx.cc Outdated
Comment thread flagcx/core/flagcx_hetero.cc
Comment thread flagcx/flagcx.cc
Comment thread flagcx/include/flagcx.h Outdated
Comment thread flagcx/include/flagcx.h
Comment thread flagcx/flagcx.cc Outdated
MC952-arch added a commit to MC952-arch/FlagCX that referenced this pull request Jun 9, 2026
MC952-arch added a commit to MC952-arch/FlagCX that referenced this pull request Jun 9, 2026
@MC952-arch MC952-arch closed this Jun 9, 2026
@MC952-arch MC952-arch reopened this Jun 9, 2026
@MC952-arch MC952-arch force-pushed the rma-p2p branch 2 times, most recently from 1eea64e to ae2f566 Compare June 10, 2026 07:14
MC952-arch added a commit to MC952-arch/FlagCX that referenced this pull request Jun 10, 2026
@MC952-arch MC952-arch requested a review from Copilot June 10, 2026 07:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.

Comment thread flagcx/flagcx.cc Outdated
Comment thread flagcx/flagcx.cc Outdated
Comment thread flagcx/flagcx.cc Outdated
Comment thread flagcx/core/include/onesided.h Outdated
Comment thread flagcx/core/flagcx_hetero.cc Outdated
Comment thread flagcx/include/flagcx.h
Comment thread test/script/rma_test.sh Outdated
Comment thread flagcx/core/include/flagcx_hetero.h

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.

Comment thread flagcx/core/include/onesided.h Outdated
Comment thread flagcx/core/flagcx_hetero.cc
Comment thread flagcx/core/flagcx_hetero.cc Outdated
Comment thread flagcx/core/flagcx_hetero.cc Outdated
Comment thread flagcx/core/flagcx_hetero.cc
Comment thread flagcx/flagcx.cc
Comment thread flagcx/include/flagcx.h
@MC952-arch MC952-arch requested a review from Copilot June 11, 2026 03:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 7 comments.

Comment thread flagcx/flagcx.cc
Comment thread flagcx/flagcx.cc
Comment thread flagcx/flagcx.cc
Comment thread flagcx/service/bootstrap.cc Outdated
Comment thread test/unittest/rma/rma_test.cpp
Comment thread test/unittest/rma/rma_test.cpp
Comment thread flagcx/include/flagcx.h
@MC952-arch MC952-arch merged commit 0b18e89 into flagos-ai:main Jun 22, 2026
12 of 13 checks passed
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