已合并
[shmem] fix with aclshmem #29046
[shmem] fix with aclshmem #29046
已合并
王超创建于 1月5日
王超成员
1月5日

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind bug
/kind task
/kind feature

What does this PR do / why do we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewers:

likedislike
Pull Request已成功合入, 合并人@ascend-robot
(感谢 王超 的贡献)
王超成员
1月5日 创建了 pull request,commit 8ba2cbfd
ascend-robot
ascend-robot成员
1月5日 评论:

Thank your for your pull-request.

The full list of commands accepted by me can be found at here.

You can get sig-info at here

likedislike
ascend-robot
ascend-robot成员
1月5日 评论:

以下是根据您提交的修改文件推荐的Reviewer和Committer序列,需各模块评审通过后方可合入

Module List Reviewers Committers
repo-Ascend/pytorch dilililiwhy, guoguanghao, wangchao285, culechan, li_jing_hw liwei386, chenhao_1209, shaoyf, medivh-x, anyrenwei
likedislike
ascend-robotascend-robot成员
1月5日 添加了label:ascend-cla/yes
王超成员
1月5日 评论:

compile

likedislike
AtlasAccount
AtlasAccount成员
1月5日 评论:

问题/功能描述

本次PR对共享内存(SHMEM)库进行了全面的接口重构和兼容性增强。核心目标是重构并扩展类型定义,以支持新增的ACLSHMEM接口和更多数据操作引擎(如SDMA、ROCE),同时确保系统在底层库存在新旧两套API(shmemaclshmemx)时能够正确初始化并运行。此外,统一了核心内存操作函数的命名规范,使其与底层库保持一致。

修改方案描述

修改方案主要包括三个方面:1) 类型定义重构与扩展:精简了通用类型头文件,更新了枚举常量前缀,并新增了用于ACLSHMEM初始化的结构体和枚举。2) 双API兼容性支持:在接口层和初始化逻辑中引入运行时检测,优先尝试加载新API并优雅回退至旧API,确保向后兼容。3) 函数命名统一:将Shmem_mallocShmem_free等核心内存操作函数重命名为Aclshmem_前缀,以符合新的命名规范。系统清理逻辑也同步更新了接口调用。

likedislike
AtlasAccount
AtlasAccount成员1月5日进行代码检视1
third_party/shmem/include/shmem_host_def.h
@@ -115,0 +200,4 @@
200+ int n_pes;
201+ char ip_port[ACLSHMEM_MAX_IP_PORT_LEN];
202+ uint64_t local_mem_size;
203+ aclshmem_init_optional_attr_t option_attr = {(1 << 16) + sizeof(aclshmem_init_optional_attr_t), ACLSHMEM_DATA_OP_MTE, DEFAULT_TIMEOUT, DEFAULT_TIMEOUT, DEFAULT_TIMEOUT};
AtlasAccount
AtlasAccount1月5日评论:

代码结构与可维护性: 在结构体aclshmemx_init_attr_t的定义中,成员option_attr使用了C++风格的默认成员初始化(= {...})。然而,该头文件被extern "C"包裹,表明其主要意图是提供C语言接口。在C语言中,结构体成员不允许进行默认初始化。这会导致在纯C代码中包含此头文件时编译错误。

问题类型: 代码结构与可维护性
文件路径: third_party/shmem/include/shmem_host_def.h
行号: 203
问题代码:

    aclshmem_init_optional_attr_t option_attr = {(1 << 16) + sizeof(aclshmem_init_optional_attr_t), ACLSHMEM_DATA_OP_MTE, DEFAULT_TIMEOUT, DEFAULT_TIMEOUT, DEFAULT_TIMEOUT};

修改建议:

移除结构体内的默认初始化。改为提供一个独立的初始化宏(例如`ACLSHMEMX_INIT_ATTR_INITIALIZER`)或文档说明用户必须在声明变量后手动初始化该成员。如果确实需要C++特性,应考虑将C和C++接口分离。

此评论由代码审查工具自动生成

likedislike
AtlasAccount
AtlasAccount成员1月5日进行代码检视1
third_party/shmem/include/shmem_host_def.h
@@ -75,0 +114,4 @@
114+*/
115+/// \def ACLSHMEM_UNIQUEID_INITIALIZER
116+/// \brief Initializer macro for the ACLSHMEM unique ID structure
117+#define ACLSHMEM_UNIQUEID_INITIALIZER \
AtlasAccount
AtlasAccount1月5日评论:

代码结构与可维护性: 宏ACLSHMEM_UNIQUEID_INITIALIZER的定义与结构体aclshmemx_uniqueid_t的成员不匹配。结构体有四个成员:version, my_pe, n_pes, internal。而宏只初始化了第一个成员version(通过ACLSHMEM_UNIQUEID_VERSION),并将一个零初始化列表赋给了第二个位置(这实际上会尝试初始化my_pe)。这会导致my_pe, n_pesinternal数组未被正确初始化(my_pe可能被初始化为0,但语义不清,且n_pesinternal是未定义行为)。这与第83行的SHMEM_UNIQUEID_INITIALIZER宏(其结构体只有versioninternal两个成员)的初始化方式不一致,容易引起误用。

问题类型: 代码结构与可维护性
文件路径: third_party/shmem/include/shmem_host_def.h
行号: 117
问题代码:

#define ACLSHMEM_UNIQUEID_INITIALIZER                   \
    {                                                   \
        ACLSHMEM_UNIQUEID_VERSION,                      \
        {                                               \
            0                                           \
        }                                               \
    }

修改建议:

修改宏定义以正确初始化所有成员。例如:
```c
#define ACLSHMEM_UNIQUEID_INITIALIZER                   \
    {                                                   \
        ACLSHMEM_UNIQUEID_VERSION,                      \
        0, /* my_pe */                                  \
        0, /* n_pes */                                  \
        {0} /* internal */                              \
    }

并确保在文档中说明此初始化器会将my_pen_pes设置为0。


---
*此评论由代码审查工具自动生成*
likedislike
AtlasAccount
AtlasAccount成员1月5日进行代码检视1
third_party/shmem/include/shmem_host_def.h
@@ -75,0 +114,4 @@
114+*/
115+/// \def ACLSHMEM_UNIQUEID_INITIALIZER
116+/// \brief Initializer macro for the ACLSHMEM unique ID structure
117+#define ACLSHMEM_UNIQUEID_INITIALIZER \
AtlasAccount
AtlasAccount1月5日评论:

初始化宏设计缺陷: ACLSHMEM_UNIQUEID_INITIALIZER宏只初始化了version字段,但aclshmemx_uniqueid_t结构体还有my_pen_pes字段未初始化。这会导致使用该宏初始化时,my_pen_pes字段处于未定义状态,可能包含随机值,引发运行时错误。

问题类型: 初始化宏设计缺陷
文件路径: third_party/shmem/include/shmem_host_def.h
行号: 117
问题代码:

#define ACLSHMEM_UNIQUEID_INITIALIZER                   \
    {                                                   \
        ACLSHMEM_UNIQUEID_VERSION,                      \
        {                                               \
            0                                           \
        }                                               \
    }

修改建议:

修改初始化宏以初始化所有字段:
#define ACLSHMEM_UNIQUEID_INITIALIZER                   \
    {                                                   \
        ACLSHMEM_UNIQUEID_VERSION,                      \
        0,                                              \
        0,                                              \
        {0}                                             \
    }

此评论由代码审查工具自动生成

likedislike
王超成员
1月5日 评论:

compile

likedislike
王超成员
1月5日 update merge request[project id: 7404318, iid: 29046, commit_id: f7e2dc906f3c48218093d7f3a5f021565ac2c243] virtual merging success
王超成员
1月5日 强制推送  1 个提交:81abf26e-[shmem] fix with aclshmemxxx
王超成员
1月5日 update merge request[project id: 7404318, iid: 29046, commit_id: e497e152ae2bae745616cb47c5cea62fcd5d7a67] virtual merging success
ascend-robot
ascend-robot成员
1月5日 评论:

以下是根据您提交的修改文件推荐的Reviewer和Committer序列,需各模块评审通过后方可合入

Module List Reviewers Committers
repo-Ascend/pytorch shaoyf, zhanJ, chuboning, yangkaixin, zhaozhijie shaoyf, wangchao285, wangqiang160, zqwenn, liwei386
likedislike
ascend-robot
ascend-robot成员
1月5日 评论:

CLA Signature Pass

@wangchao430, thanks for your pull request. All authors of the commits have signed the CLA. 👍

likedislike
AtlasAccountAtlasAccount成员
1月5日 添加了label:ci-pipeline-failed
王超成员
1月5日 评论:

compile

likedislike
ascend-robotascend-robot成员
1月5日 删除了label:ci-pipeline-failed
ascend-robotascend-robot成员
1月5日 添加了label:ci-pipeline-running
AtlasAccount
AtlasAccount成员
1月5日 评论:

/torch_npu/csrc/distributed/symm_mem/NPUSHMEMInterface.cpp

likedislike
王超成员
1月5日 评论:

compile

likedislike
ascend-robotascend-robot成员
1月5日 删除了label:ci-pipeline-running
ascend-robot
ascend-robot成员
1月5日 评论:
流水线 PR-pipeline_pytorch#1605已终止运行
阶段 任务名 状态 详情
编译构建 Build_X86 >>>
Build_LibTorch >>>
Build_ARM >>>
Build_ARM_inductor 🛑 >>>
Build_X86_py311 🛑 >>>
Build_ARM_py311 🛑 >>>
dist_test_or_not >>>
恶意代码检查 Antipoison >>>
编码安全与规范检查 CodeCheck >>>
check_error >>>
开源片段检查 SCA >>>
开发者测试 UT_DIST_X86 🟨 >>>
UT_X86_Part_01 >>>
UT_X86_Part_02 >>>
UT_ARM_A2_Part_01 🛑 >>>
UT_ARM_A2_Part_02 🛑 >>>
UT_inductor_arm 🛑 >>>
流水线 PR-pipeline_pytorch 🟨 >>>
likedislike
ascend-robotascend-robot成员
1月5日 添加了label:ci-pipeline-running
AtlasAccount
AtlasAccount成员
1月5日 评论:

/torch_npu/csrc/distributed/symm_mem/NPUSHMEMInterface.cpp

likedislike
ascend-robotascend-robot成员
1月5日 删除了label:ci-pipeline-running
ascend-robotascend-robot成员
1月5日 添加了label:ci-pipeline-failed
ascend-robotascend-robot成员
1月6日 删除了label:ci-pipeline-failed
ascend-robotascend-robot成员
1月6日 添加了label:ci-pipeline-running
ascend-robotascend-robot成员
1月6日 删除了label:ci-pipeline-running
ascend-robotascend-robot成员
1月6日 添加了label:ci-pipeline-passed
ascend-robot
ascend-robot成员
1月6日 评论:
流水线 PR-pipeline_pytorch#1686(重试第1次)已完成
阶段 任务名 状态 详情
编译构建 Build_X86 >>>
Build_LibTorch >>>
Build_ARM >>>
Build_ARM_inductor 🛑 >>>
Build_X86_py311 🛑 >>>
Build_ARM_py311 🛑 >>>
dist_test_or_not >>>
恶意代码检查 Antipoison >>>
编码安全与规范检查 CodeCheck >>>
check_error >>>
开源片段检查 SCA >>>
开发者测试 UT_DIST_X86 >>>
UT_X86_Part_01 >>>
UT_X86_Part_02 >>>
UT_ARM_A2_Part_01 🛑 >>>
UT_ARM_A2_Part_02 🛑 >>>
UT_inductor_arm 🛑 >>>
流水线 PR-pipeline_pytorch >>>
likedislike
shaoyf成员
1月6日 评论:

/approve

likedislike
ascend-robotascend-robot成员
1月6日 添加了label:lgtm
zhangqiongwen成员
1月6日 评论:

/approve

likedislike
ascend-robotascend-robot成员
1月6日 添加了label:approved
ascend-robot
ascend-robot成员
1月6日 评论:

Review Guide

This Pull-Request Passes Review.
Committers who writed a comment of /approve are: shaoyf, zqwenn.
Reviewers who writed a comment of /lgtm are: shaoyf, zqwenn.

likedislike
ascend-robotascend-robot成员
1月6日 合入了pull request