[wip] Symm support #1922

Chao1Han · 2025-08-11T02:34:55Z

No description provided.

Copilot

Pull Request Overview

This pull request adds symmetric memory support for XPU devices. The implementation provides Level Zero-based memory allocation, inter-process communication (IPC), and symmetric memory management functionality.

Adds Level Zero exception handling and dynamic library loading for XPU
Implements XPU-specific symmetric memory allocator and memory management classes
Provides IPC-based memory exchange mechanisms for multi-process communication

Reviewed Changes

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

Show a summary per file

File	Description
ze_exception.hpp	Level Zero exception handling and dynamic library loading utilities
XPUSymmetricMemoryTypes.hpp	Type definitions for XPU symmetric memory implementation
XPUSymmetricMemoryUtils.hpp	Utility classes for symmetric memory operations and IPC communication
XPUSymmetricMemoryUtils.cpp	Implementation of IPC channels and memory mapping utilities
XPUSymmetricMemory.hpp	Main symmetric memory class declarations for XPU
XPUSymmetricMemory.cpp	Implementation of XPU symmetric memory allocator and management
ProcessGroupXCCL.cpp	Temporary workaround for barrier synchronization
IPCExchange.hpp	Inter-process communication utilities for memory handle exchange

Copilot · 2025-08-11T02:37:03Z

src/xccl/ze_exception.hpp

+  if (ze_handle != nullptr) {
+    return true;
+  }
+  const char* lib_names[] = {"/usr/lib/x86_64-linux-gnu/libze_loader.so"};


The hardcoded library path is platform-specific and may not work across different Linux distributions or architectures. Consider adding multiple common library paths or using a more flexible discovery mechanism.

Suggested change

const char* lib_names[] = {"/usr/lib/x86_64-linux-gnu/libze_loader.so"};

const char* lib_names[] = {

"/usr/lib/x86_64-linux-gnu/libze_loader.so",

"/usr/lib64/libze_loader.so",

"/usr/lib/libze_loader.so",

"/usr/local/lib/libze_loader.so",

"libze_loader.so"

};

Copilot · 2025-08-11T02:37:04Z

src/xccl/ze_exception.hpp

+    ze_result_t result = (x);                                       \
+    if (result != ZE_RESULT_SUCCESS) {                              \
+      auto e = zeException(result);                                 \
+      std::cout << "Throw " << e.what() << std::endl;               \


Using std::cout for error logging in a library is not recommended. Consider using a proper logging framework or removing debug output from production code.

Copilot · 2025-08-11T02:37:04Z

src/xccl/ze_exception.hpp

+#define zeCheck(x)                                  \
+  if (x != ZE_RESULT_SUCCESS) {                     \
+    auto e = zeException(x);                        \
+    std::cout << "Throw " << e.what() << std::endl; \


Using std::cout for error logging in a library is not recommended. Consider using a proper logging framework or removing debug output from production code.

Copilot · 2025-08-11T02:37:04Z

src/xccl/XPUSymmetricMemoryUtils.hpp

+  size_t seq_id_ = 0;
+};
+
+// Teturns a pointer of virtual address that is mapped to the physical memory


Typo in comment: 'Teturns' should be 'Returns'.

Suggested change

// Teturns a pointer of virtual address that is mapped to the physical memory

// Returns a pointer of virtual address that is mapped to the physical memory

Copilot · 2025-08-11T02:37:04Z

src/xccl/XPUSymmetricMemoryUtils.cpp

+  sycl::context sycl_ctx = current_queue.get_context();
+  ze_context_handle_t ze_context =
+      sycl::get_native<sycl::backend::ext_oneapi_level_zero>(sycl_ctx);
+  std::cout << "zl_debug in map_block to get virtual address " << std::endl;


Debug output should be removed from production code. Consider using a proper logging framework or removing these debug statements.

Suggested change

std::cout << "zl_debug in map_block to get virtual address " << std::endl;

Copilot · 2025-08-11T02:37:06Z

src/xccl/XPUSymmetricMemory.cpp

+  c10::Device local_device(c10::DeviceType::XPU, local_device_idx);
+  c10::DeviceGuard guard(local_device);
+
+  // todo: zl_debug


TODO comment should be addressed or removed before merging to production.

Suggested change

// todo: zl_debug

Copilot · 2025-08-11T02:37:06Z

src/xccl/XPUSymmetricMemory.cpp

+#include <sys/socket.h>
+#include <unistd.h>
+
+// todo: fixed with kernel barrier


TODO comment should be addressed or removed before merging to production.

Suggested change

// todo: fixed with kernel barrier

Copilot · 2025-08-11T02:37:06Z

src/xccl/ProcessGroupXCCL.cpp

@@ -1911,7 +1911,7 @@ c10::intrusive_ptr<Work> ProcessGroupXCCL::barrier(const BarrierOptions& opts) {
  }

  auto currentStream = at::xpu::getCurrentXPUStream(barDevIdx);
-  currentStream.synchronize();
+  //  currentStream.synchronize(); // zl_debug workaround for symm barrier


Commented out synchronization appears to be a temporary workaround. This should be properly addressed before production deployment as it may affect correctness.

Suggested change

// currentStream.synchronize(); // zl_debug workaround for symm barrier

currentStream.synchronize(); // Ensure stream synchronization for barrier

Copilot · 2025-08-11T02:37:06Z

src/xccl/IPCExchange.hpp

+  ~allreducer() {
+    if (initialized) {
+      std::cerr << "Warning: allreducer destroyed without calling release()"
+                << std::endl;


Using std::cerr for warnings in a library may not be appropriate. Consider using a proper logging framework or throwing an exception if cleanup is critical.

Suggested change

<< std::endl;

throw std::runtime_error("allreducer destroyed without calling release()");

Copilot · 2025-08-11T02:37:07Z

src/xccl/IPCExchange.hpp

+        }
+      }
+    } catch (const std::exception& e) {
+      std::cerr << "Warning: Level Zero cleanup failed: " << e.what()


Using std::cerr for warnings in a library may not be appropriate. Consider using a proper logging framework.

zhangxiaoli73 added 30 commits July 15, 2025 16:21

add XPU symm

57d67ea

correct include

5756399

remove XPUGuard

bcf9cc4

debug

c9d3e38

debug

13e0f21

debug

762ccfd

debug

0661f94

debug

4cf53e0

debug

9abc672

debug

2713e8b

debug

99ff4ce

debug

f638c3d

debug

33fdeb1

add debug logs

413c718

check device type

b4d4dd4

debug async ops

8e9dc93

debug

ba1e1d4

refine to void*

4a71bfa

debug

01a8c69

debug copy

bbae412

debug sharded handle

da8b020

debug

055b038

debug

b0fc7e8

enable torch-ccl exchange

b118c92

remove unneeded

3d9acd2

fix a bug and move to local IPC exchange

29a74cd

add symm copy_buffer API

f9ae0d5

barrier with MPI

1973dfe

support arc

2b9edef

refine barrier

e269c12

zhangxiaoli73 and others added 13 commits July 15, 2025 16:23

workaroud barrier

9c36685

refine ipc exchange

369fe16

refine ipc exchange

226dab1

reabse and then workaround barrier

b6fb65a

impl base class virtual function

65643c3

format

3fbe388

Merge remote-tracking branch 'origin/main' into symm-support-bak

4ce540f

rm hardcode ze

eeb81ae

clean code

b756e41

update

ac8015d

avoid symbol conflict

b6199ad

refine IPCExchang

d7130fe

rm header

4065ad1

Copilot AI review requested due to automatic review settings August 11, 2025 02:34

Copilot AI reviewed Aug 11, 2025

View reviewed changes

update

5345b3c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[wip] Symm support #1922

[wip] Symm support #1922

Uh oh!

Chao1Han commented Aug 11, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 11, 2025

Uh oh!

Copilot AI Aug 11, 2025

Uh oh!

Copilot AI Aug 11, 2025

Uh oh!

Copilot AI Aug 11, 2025

Uh oh!

Copilot AI Aug 11, 2025

Uh oh!

Copilot AI Aug 11, 2025

Uh oh!

Copilot AI Aug 11, 2025

Uh oh!

Copilot AI Aug 11, 2025

Uh oh!

Copilot AI Aug 11, 2025

Uh oh!

Copilot AI Aug 11, 2025

Uh oh!

Uh oh!

-  const char* lib_names[] = {"/usr/lib/x86_64-linux-gnu/libze_loader.so"};
+  const char* lib_names[] = {
+    "/usr/lib/x86_64-linux-gnu/libze_loader.so",
+    "/usr/lib64/libze_loader.so",
+    "/usr/lib/libze_loader.so",
+    "/usr/local/lib/libze_loader.so",
+    "libze_loader.so"
+  };

	// Teturns a pointer of virtual address that is mapped to the physical memory
	// Returns a pointer of virtual address that is mapped to the physical memory

	// currentStream.synchronize(); // zl_debug workaround for symm barrier
	currentStream.synchronize(); // Ensure stream synchronization for barrier

	<< std::endl;
	throw std::runtime_error("allreducer destroyed without calling release()");

[wip] Symm support #1922

Are you sure you want to change the base?

[wip] Symm support #1922

Uh oh!

Conversation

Chao1Han commented Aug 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!