已开启
fix(AllToAll): 修复srcGroupSize和cclBufferCountPerRank计算错误及maxDataSizePerLoop多除dataTypeSize问题 #2763
yaott123创建于 12 天前
fix(AllToAll): 修复srcGroupSize和cclBufferCountPerRank计算错误及maxDataSizePerLoop多除dataTypeSize问题 #2763
已开启
yaott123创建于 12 天前
yaott123
yaott123
12 天前

描述

关联的Issue

测试

文档更新

类型标签

likedislike
合并受阻
yaott123yaott123
12 天前 创建了 pull request,commit 8e0f5086
atomgit-bot
atomgit-bot
12 天前 评论:

变更摘要

本 PR 主要修复分层 AllToAll(Hier)实现中的计算错误:srcGroupSizecclBufferCountPerRank 计算错误,以及 maxDataSizePerLoop 多除了一次 dataTypeSize 的问题;同时新增一整套分层 AllToAll 执行框架,包括新的 InsV2AlltoAllHierSequenceExecutor 执行器、TopoMatchNLevel 多层级拓扑匹配、AlltoAllStageTemplateRegistry 阶段模板注册表与 InsTempAlltoAllHierMeshStage 网格阶段模板,并让选择器切换到新算法。

主要改动

  • 修复 maxDataSizePerLoop 多除 dataTypeSize: 在 InsV2AlltoAllHierSequenceExecutor::OrchestrateLoop 中,maxDataSizePerLoop 改为直接以 cclBuffSize / totalRankSize_ 并按 HCCL_MIN_SLICE_ALIGN 对齐计算(不再除以 dataTypeSize),随后由 maxDataCountPerLoop = maxDataSizePerLoop / dataTypeSize_ 得到每轮数据条数,保证循环切分数据量正确。

  • 新增分层 AllToAll 序列执行器: 新增 InsV2AlltoAllHierSequenceExecutorins_v2_all_to_all_hier_sequence_executor.cc/.h),将 AllToAll 编排为按 stage 顺序执行的过程,通过 SetStageParams 计算各阶段参数(如 inputSliceStride = maxDataCountPerLoop * otherDimRankSize * dataTypeSize_outputSliceStride 与首/尾/中间阶段缓冲类型),并注册为 AicpuAllToAllSoleMeshHier 执行器。

  • 修复 srcGroupSizecclBufferCountPerRank 计算: 在 InsTempAlltoAllHierMeshStage::KernelRun 中,srcGroupSize 改为正确的 otherDimRankSizecclBufferCountPerRank_ 改为按 inputSliceStride / dataTypeSize_ / otherDimRankSize 计算,供首/尾阶段本地拷贝及 RunSendRecv 收发使用。

  • 新增多层级拓扑匹配与阶段模板注册机制: 新增 TopoMatchNLeveltopo_match_nlevel.cc/.h)从 rank 图逐层构建 rank 分组并填充 AlgHierarchyInfoForAllLevel;新增 AlltoAllStageTemplateRegistryREGISTER_A2A_STAGE_TEMPLATE 宏用于注册/创建阶段模板,InsTempAlltoAllHierStageBase 定义首/尾/中间 stage 角色判定。

  • 选择器切换新算法并补充单测: AlltoAllAutoSelector 将 level0 为 MESH_1D/CLOS/MESH_1D_CLOS 的场景由 AicpuAllToAllSoleMesh 切换为 AicpuAllToAllSoleMeshHier;新增 test/ut/alltoall_hier/alltoall_hier_test.cc 覆盖阶段角色判定与阶段模板注册/创建的单元测试。

likedislike
atomgit-bot
atomgit-bot
12 天前 评论:

代码审查

我已报告所有确定的问题。下面给出收尾总结。

收尾总结

各变更文件审查结论

  1. src/ops/all_to_all_v/executor/CMakeLists.txt — 将新 executor 源加入 NOT HCCL_CANN_COMPAT_850 分支,与同目录其他 v2 executor 的构建条件一致;但其门控与 selector 的无条件改动构成跨文件不一致(见 finding #2 的根因),本文件本身无独立问题。
  2. src/ops/all_to_all_v/executor/ins_v2_all_to_all_hier_sequence_executor.cc — 发现 2 个问题:① stage 模板 channel 索引错位(执行序 k vs stageIndex,P1);② maxDataCountPerLoop 除数未校验(P3)。
  3. src/ops/all_to_all_v/executor/ins_v2_all_to_all_hier_sequence_executor.h — 发现 1 个问题:未使用的常量 FULLMESH_THRESHOLD(P3,可选清理)。
  4. src/ops/all_to_all_v/selector/alltoall_auto_selector.cc — 发现 1 个问题:无条件选中 AicpuAllToAllSoleMeshHier,在 HCCL_CANN_COMPAT_850 构建下该算法未注册 → 运行期 HCCL_E_PARA(P2)。
  5. src/ops/all_to_all_v/template/aicpu/CMakeLists.txt — no issues;新增 registry 与 mesh stage 源文件均在 NOT HCCL_CANN_COMPAT_850 分支内,一致。
  6. src/ops/all_to_all_v/template/aicpu/hier/alltoall_stage_template_registry.cc — no issues;加锁注册/创建,未找到 creator 时返回 nullptr 并记录错误,路径完整。
  7. src/ops/all_to_all_v/template/aicpu/hier/alltoall_stage_template_registry.h — no issues;单例与删除拷贝构造、REGISTER_A2A_STAGE_TEMPLATE 宏定义均正确。

发现统计与总体风险判断

  • P1 × 1(stage 模板 channel 索引错位,多 stage 拓扑下会用镜像 stage 的通道与错误远端建链,存在数据错发/挂死风险)
  • P2 × 1(HCCL_CANN_COMPAT_850 兼容构建回归,选中未注册算法导致报错)
  • P3 × 2(潜在的除零守卫缺口;未使用的死常量)

总体风险判断:本次 PR 引入了新层级序列 executor 并改变多级拓扑下的算法选择路径。核心风险集中在通道资源与 stage 模板的索引对齐(P1)以及兼容构建下算法名未注册(P2)两处——前者会影响多级 alltoall 的实际通信正确性,后者会在特定编译配置下直接报错。建议合入前优先修复 P1 索引错位,并确认 compat 构建的 selector/executor 一致性;P3 项为可选加固/清理。


Review complete. 我已完成对全部 9 个变更文件的审查。

审查结果汇总(按优先级)

  • P0:0 个
  • P1:0 个
  • P2:2 个
  • P3:1 个

各文件审查确认

文件 结论
src/ops/all_to_all_v/template/aicpu/hier/ins_temp_all_to_all_hier_mesh_stage.cc 1 个 P2:LocalCopyForMyGroup(写槽位 targetIdx*R+myAlgRank)与 LocalCopyForMyRank(读槽位 myAlgRank*R+srcIdx)对同一 ccl buffer 采用不一致的槽位索引布局,且每 rank 区域容量按 otherDimRankSize 计算;当 templateRankSize_ > otherDimRankSize(首阶段写槽位达 R²-1,末阶段读槽位在 R≥2 时即超界)会越界读写。
src/ops/all_to_all_v/template/aicpu/hier/ins_temp_all_to_all_hier_mesh_stage.h no issues
src/ops/all_to_all_v/template/aicpu/hier/ins_temp_all_to_all_hier_stage_base.h no issues(SetStageRole 首/末阶段编号反转与测试一致,为有意设计)
src/ops/op_common/topo/CMakeLists.txt no issues(条件编译追加源文件,与现有列表一致)
src/ops/op_common/topo/topo_match_nlevel.cc 1 个 P2:TopoForLayerGenericHcclRankGraphGetLinks(L129)未检查返回值且 links 未初始化,与同目录其余实现(均用 CHK_RET)不一致;失败时静默按"无链路"处理,导致各 rank 分组不一致。
src/ops/op_common/topo/topo_match_nlevel.h no issues
test/ut/CMakeLists.txt no issues
test/ut/alltoall_hier/CMakeLists.txt no issues(file(GLOB) 已用 if 守卫,链接/测试命令沿用现有模式)
test/ut/alltoall_hier/alltoall_hier_test.cc 1 个 P3:测试仅覆盖 SetStageRole 与注册表创建路径,未覆盖本 PR 核心的数据搬运/偏移计算逻辑(KernelRun/LocalCopy/RunSendRecv),此类偏移计算恰是易错点。

总体风险判断

本 PR 新增了分层 AllToAll 的 Mesh Stage 核心实现与 N 级拓扑匹配,涉及复杂的偏移计算和分组构建逻辑。发现的核心风险集中在两点:一是 ccl buffer 槽位索引布局在两处本地拷贝间不一致、在特定 R/G 组合下存在越界读写风险;二是拓扑匹配中一处系统调用返回值未检查,可能导致各 rank 分组不一致。两者均属于条件触发型问题,且新代码目前没有针对数据搬运路径的测试覆盖,建议在合入前统一 ccl buffer 索引口径并补充偏移计算的单元测试。整体为中等偏高风险,需修复上述 P2 项后再合入。

类型 数量
🔴 阻塞 0
🟡 建议 1

💬 仅评论

likedislike
CANN-robotCANN-robot成员
12 天前 添加了label:stat/needs-squash
CANN-robotCANN-robot成员
12 天前 添加了label:cann-cla/yes
此处折叠了771条消息 查看更多
yaott123
yaott123
1 分钟前 评论:

compile

likedislike
yaott123yaott123
1 分钟前 update merge request[project id: 8277906, iid: 2763, commit_id: bfb87774fc4f311b43ecffb66894c7117746a979] virtual merging success
CANN-robot
CANN-robot成员
1 分钟前 评论:
🚀 CI 流水线已启动
📋 执行详情: 点击查看流水线
likedislike
CANN-robotCANN-robot成员
1 分钟前 删除了label:ci-pipeline-failed
CANN-robotCANN-robot成员
1 分钟前 添加了label:ci-pipeline-running