GDA support for alltoall via rocshmem integration #2099

nusislam · 2025-12-10T22:12:10Z

Details

Experimental enablement of GDA-based alltoall in RCCL via rocshmem integration. Tested on MI300 with Thor2 NICs.

Work item: AICOMRCCL-332

What were the changes?
One sentence describing the work done.

Why were the changes made?
Explain the motivation behind the work. Provide any publicly-available historical context.

How was the outcome achieved?
Technical details behind the work. Explain any publicly-available hardware peculiarities.

Additional Documentation:
What else should the reviewer know?

Approval Checklist

Do not approve until these items are satisfied.

Verify the CHANGELOG has been updated, if
- there are any NCCL API version changes,
- any changes impact library users, and/or
- any changes impact any other ROCm library.

…l commit

atulkulk · 2026-01-05T16:41:49Z

src/device/generate.py

+all_pipeline = ["0", "1"]
+pipelined_types = ["bf16"]


Is all_pipeline just a placeholder for now? I do not see it being used. OR is it a dup of line 14?
pipelined_types is also defined again below on line 162. Duplicate?

atulkulk · 2026-01-05T16:45:27Z

src/device/alltoall_gda.h

@@ -0,0 +1,39 @@
+/*************************************************************************


What is the diff between alltoall_gda and alltoall_gda1 impl?

I had the same question.

alltoall_gda1 is a kernel for large messages where we use multiple CUs for the data copies.

alex-breslow-amd · 2026-01-05T16:57:19Z

src/init.cc


+#ifdef ENABLE_ROCSHMEM
+#include <rocshmem/rocshmem.hpp>
+#define NUM_SYM_BUF 8


This seems potentially dangerous. Can we get this from ROCSHMEM?

This is a rccl defined number to control the number of symmetric buffers used in rccl.

alex-breslow-amd · 2026-01-05T16:58:21Z

src/init.cc

+#ifdef ENABLE_ROCSHMEM
+  /* --- sanity-check print statement for development purposes --- */
+  if (rcclParamRocshmemEnabled()) { // @TODO - This doesn't seem to disable when I set ROCSHMEM_ENABLE=0 on command line
+    printf("Initializing rocSHMEM inside of RCCL\n");


INFO debug print instead?

alex-breslow-amd · 2026-01-05T16:59:43Z

src/init.cc

+    	comm->sourceRshmem[i] = (void *)rocshmem::rocshmem_malloc((size_t)(1*1024*1024));
+    	comm->destRshmem[i] = (void *)rocshmem::rocshmem_malloc((size_t)(1*1024*1024));


This constant looks like it should be pulled out and given a name. What are the implications of this sizing?

This is a good point. This size should be >= threshold. I will change it.

what happens if threshold is set to greater than 1MB? do you clamp it?

alex-breslow-amd · 2026-01-05T17:02:12Z

.gitmodules

 	url = https://github.com/nlohmann/json.git
 	ignore = dirty
 	shallow = true
+[submodule "ext-src/rocSHMEM"]


To be honest, I don't hate this.

I guess the question I had is about how we are planning on rolling out rocSHMEM changes. Are we planning on using a pinned version of rocSHMEM or running part of RCCL CI tests as part of rocSHMEM's release? I think we need something like that for safety if we're planning on enabling this code path by default as part of our release.

We plan to use a pinned version of rocSHMEM.

alex-breslow-amd · 2026-01-05T17:10:40Z

Can you comment on how this affects compilation time for gfx942 and gfx950?

mustafabar · 2026-01-05T17:10:35Z

src/device/generate.py

  func_pattern = func_pattern[0]
 else:
-  func_pattern = "AllGather|AllReduce|AllToAllPivot|Broadcast|Reduce|ReduceScatter|SendRecv"
+  func_pattern = "AllGather|AllReduce|AllToAllPivot|AllToAllGda|AllToAllGda1|Broadcast|Reduce|ReduceScatter|SendRecv"


are these new functions built if rocshmem is disabled? I see the guards in the source code, but will this take care of avoiding the build?

mustafabar · 2026-01-05T17:42:05Z

cmake/Findrocshmem_static.cmake

+find_package_handle_standard_args(rocshmem_static DEFAULT_MSG ROCSHMEM_INCLUDE_DIR ROCSHMEM_LIBRARY)
+## mark_as_advanced(MSCCLPP_INCLUDE_DIRS MSCCLPP_NCCL_STATIC_LIB) add this for Rocshmem?
+
+## --- TODO --- Remove this, just use for testing purposes -- ###


It is a HARD fatal error. Do you want to still keep it?

mustafabar · 2026-01-05T17:46:07Z

src/enqueue.cc

  bool specialized;
 };

+static int symId = 0;


Looks like a race. It is shared across all communicators, and can err in multi-threaded mode.

mustafabar · 2026-01-05T17:48:57Z

src/init.cc

+     	rocshmem::rocshmem_free(comm->sourceRshmem[i]);
+     	rocshmem::rocshmem_free(comm->destRshmem[i]);	  
+     }
+     rocshmem::rocshmem_finalize();


Is rocshmem::rocshmem_finalize(); safe when multiple comms call finalize?

mustafabar · 2026-01-05T17:51:50Z

src/init.cc

+    	comm->sourceRshmem[i] = (void *)rocshmem::rocshmem_malloc((size_t)(1*1024*1024));
+    	comm->destRshmem[i] = (void *)rocshmem::rocshmem_malloc((size_t)(1*1024*1024));


what happens if threshold is set to greater than 1MB? do you clamp it?

thomas-huber · 2026-01-06T20:57:34Z

Can you comment on how this affects compilation time for gfx942 and gfx950?

Hey @alex-breslow-amd!

On gfx942, a traditional build (./install.sh -l) takes 603.045s and the "with rocshmem" build takes 605.864s (./install.sh -l --rocshmem).

alex-breslow-amd · 2026-01-06T21:11:09Z

Can you comment on how this affects compilation time for gfx942 and gfx950?

Hey @alex-breslow-amd!

On gfx942, a traditional build (./install.sh -l) takes 603.045s and the "with rocshmem" build takes 605.864s (./install.sh -l --rocshmem).

Great, that is good to hear @thomas-huber and @nusislam!

alex-breslow-amd · 2026-01-06T21:12:19Z

Sorry for closing by accident.

nusislam marked this pull request as draft December 10, 2025 22:12

thomas-huber and others added 23 commits December 15, 2025 17:58

ROCSHMEM linking/building to match MSCCL++ style

b1d5a71

add rocSHMEM as a submodule

f9c4381

Move rocSHMEM submodule to ext-src/rocSHMEM

e002c64

Adding submodule support proper, as well as a patch for rocshmem

f157533

Cleaning up INCLUDE_DIR vs INCLUDE_DIRS mixup

b0461c4

updating patch file

c9b8c96

Pointing rocshmem submodule to edgars fixup patch

7b0b7e4

Adding IBVERBS link to the submodule build

2ca3149

More IBVERBS patching

8f6752a

pin rocshmem submodule to b534423

c93da10

Adding IPC support in rocSHMEM build

9547182

updating rocshmem submodule to resolve CQ errors

3480a5e

Updating submodule to include recent a2a optimizations

1d9a871

invoke rocshmem alltoall from rccl

23b32c6

Updating submodule to CQ error number hang

88ed78e

Updating submodule to include a2a improvements and bug fixes

d0141e6

Updating submodule to point to Yiltan's fork and doorbell ring remova…

f15a032

…l commit

Updating hash to correspond with submodule change

af9e5fd

Updating to no-ctx wg call and updating submodule

53726db

copy-in/copy-out using multiples CUs

9fa99af

Updating rocSHMEM submodule to include doorbell improvs

9c02c51

updating gitmodule to point to upstream

f23d91a

code cleanup and adjust threashold

26126ef

nusislam force-pushed the rccl-rocshmem-alltoall-new branch from c8d1d16 to 26126ef Compare December 17, 2025 18:26

nusislam and others added 3 commits December 17, 2025 12:28

Merge branch 'develop' into rccl-rocshmem-alltoall-new

6180498

Merge branch 'develop' into rccl-rocshmem-alltoall-new

ad58577

guard rocshmem a2a invocation

dacc06a

nusislam marked this pull request as ready for review December 19, 2025 22:37

Merge branch 'develop' into rccl-rocshmem-alltoall-new

f580596

Only build with rocshmem when specified

78729a4

thomas-huber self-requested a review December 31, 2025 18:17

Merge branch 'develop' into rccl-rocshmem-alltoall-new

3c0b4c7

nusislam requested a review from wenkaidu January 2, 2026 15:59

Merge branch 'develop' into rccl-rocshmem-alltoall-new

c57cc09

atulkulk reviewed Jan 5, 2026

View reviewed changes

alex-breslow-amd reviewed Jan 5, 2026

View reviewed changes

mustafabar reviewed Jan 5, 2026

View reviewed changes

code cleanup

69e8dcd

nusislam force-pushed the rccl-rocshmem-alltoall-new branch from 693c20e to 4e57ace Compare January 5, 2026 21:37

alex-breslow-amd closed this Jan 6, 2026

alex-breslow-amd reopened this Jan 6, 2026

address review comments

b343b6c

nusislam force-pushed the rccl-rocshmem-alltoall-new branch from 4e57ace to b343b6c Compare January 6, 2026 21:43

Merge branch 'develop' into rccl-rocshmem-alltoall-new

3f75084

		@@ -0,0 +1,39 @@
		/*************************************************************************

		comm->sourceRshmem[i] = (void )rocshmem::rocshmem_malloc((size_t)(11024*1024));
		comm->destRshmem[i] = (void )rocshmem::rocshmem_malloc((size_t)(11024*1024));

GDA support for alltoall via rocshmem integration #2099

Are you sure you want to change the base?

GDA support for alltoall via rocshmem integration #2099

Conversation

nusislam commented Dec 10, 2025 • edited by gaoikawa Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Approval Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-breslow-amd commented Jan 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mustafabar Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomas-huber commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alex-breslow-amd commented Jan 6, 2026

Uh oh!

alex-breslow-amd commented Jan 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nusislam commented Dec 10, 2025 •

edited by gaoikawa

Loading

mustafabar Jan 5, 2026 •

edited

Loading

thomas-huber commented Jan 6, 2026 •

edited

Loading