已合并
【PR】: fix: 修复 Reduce 子图复制的边重连 #1544
czways创建于 7月29日
【PR】: fix: 修复 Reduce 子图复制的边重连 #1544
已合并
Pull Request已成功合入, 合并人@CANN-robot
(感谢 czways 的贡献)7月29日 添加了label:enhancement
atomgit-bot
7月29日 评论:
7月29日 评论:
变更摘要
此 PR 主要修复了当图中存在同名节点时,节点克隆与图分区过程中的边重连失败问题。核心改动是将基于节点名称字符串(std::string)的节点映射改为基于节点原始指针(const Node*)的 NodeCloneMap 映射,从而确保在节点名称重复时仍能正确建立源节点到克隆节点的对应关系。同时新增了 RelinkClonedDataEdges 和 RelinkClonedControlEdges 辅助函数,重构了 RelinkGraphEdges 的重载逻辑,并在 reduce_schedule_case_generator 中将节点匹配从名称比较改为指针比较,新增了按输出索引拷贝张量属性的 DoCopyAscNodeOutputTensorAttr 函数。
主要改动
- 引入
NodeCloneMap替换基于名称的映射: 在graph_utils.h中新增using NodeCloneMap = std::unordered_map<const Node*, NodePtr>类型别名,将Autofuse图拷贝 (ascendc_ir.cc) 和ScheduleGroupGraphPartitioner中的AddConnectedNodes、MergeGraphs方法全部从std::unordered_map<std::string, NodePtr>/std::map<std::string, NodeView>切换为NodeCloneMap,消除了同名节点导致的映射覆盖问题。 - 新增按指针索引的
RelinkGraphEdges重载: 在graph_utils.cc中新增接受NodeCloneMap参数的RelinkGraphEdges方法,并在RelinkClonedDataEdges和RelinkClonedControlEdges中遍历原始节点的所有输出锚点,通过node_clone_map.find(peer_node)按指针查找对端克隆节点来重建数据边与控制边。 PartitionByConnectivity输出节点收集优化: 将schedule_group_partitioner.cpp中的输出节点收集从std::map<std::string, NodeView>改为std::vector<AscNodePtr>,当node_order为空时直接移动并排序,避免因名称冲突丢失输出节点。- 节点身份比较从名称改为指针: 在
reduce_schedule_case_generator.cpp的PartitionByNode、PartitionLoad、PartitionScalar中,将对端节点匹配条件由GetOwnerNodeBarePtr()->GetName() == dst_node->GetName()改为GetOwnerNodeBarePtr() == dst_node.get(),消除同名节点的误匹配风险。 - 新增按输出索引拷贝张量属性函数: 在
reduce_schedule_case_generator.cpp中新增DoCopyAscNodeOutputTensorAttr函数,支持按指定的输出索引将源节点的AscTensorAttr(含调度属性与 IR 属性)拷贝到目标节点的对应输出描述上,替换了原有的全量DoCopyAscNodeTensorAttr调用。


不准确?
atomgit-bot
7月29日 评论:
7月29日 评论:
代码审查
审查总结
逐文件审查了全部 6 个变更文件,结果如下:
| 文件 | 审查结论 |
|---|---|
autofuse/graph_metadef/graph/ascendc_ir/core/ascendc_ir.cc |
无问题 — NodeCloneMap(指针键)替换 name-keyed map,配合 AscNode : public Node 单继承,.get() 地址一致,正确 |
autofuse/graph_metadef/graph/utils/graph_utils.cc |
1 个 P3 建议(新旧 RelinkGraphEdges 对缺失 peer 行为不一致) |
autofuse/inc/graph_metadef/graph/utils/graph_utils.h |
无问题 — NodeCloneMap typedef 和新重载声明正确 |
autofuse/optimize/task_generator/reduce_schedule_case_generator.cpp |
1 个 P3 建议(DoCopyAscNodeOutputTensorAttr 目标侧硬编码 0U) |
autofuse/optimize/task_generator/schedule_group_partitioner.cpp |
无问题 — 核心修复正确:name-based map → pointer-based map,name-based std::map → std::vector,消除重名丢节点 bug |
autofuse/tests/ut/optimize/task_generator/test_schedule_group_partitioner.cpp |
无问题 — 两个新测试正确覆盖了重名 workspace 的 Copy 和 Partition 场景 |
发现汇总:P0–P2: 0 个 | P3: 2 个
整体风险评估:低风险。此 PR 的核心改动是将多处基于名称(string key)的节点映射替换为基于指针(raw pointer key)的映射,以修复"重名节点导致连接关系丢失"的 bug。这一策略正确且所有调用点的生命周期均安全。两个 P3 发现均为 API 设计层面的改进建议,不影响当前功能的正确性。
| 类型 | 数量 |
|---|---|
| 🔴 阻塞 | 0 |
| 🟡 建议 | 1 |
💬 仅评论


不准确?
7月29日 添加了label:cann-cla/yes
此处折叠了61条消息 查看更多
7月30日 删除了label:ci-pipeline-running
7月30日 添加了label:ci-pipeline-passed
zhanj
7月30日 评论:
7月30日 评论:
/approve


7月30日 添加了label:approved
7月30日 合入了pull request
Pull Request
描述
修复 Autofuse 在 Reduce 相关场景复制或合并子图时的边重连问题:以源节点到克隆节点的 NodeCloneMap 替换按节点名的映射,分别重连数据边和控制边;同时按节点 ID 稳定输出节点顺序,消除调度组结果对容器遍历顺序的依赖。
变更类型
关联的Issue
如何测试
核对清单
其他信息
核心问题
图中存在重名节点时,按节点名称做查找/映射/匹配会导致边连接错误或节点丢失。
重名节点的来源:
PartitionByNode中,workspace_pre和workspace_post通过GetNewNodeName(src_node, dst_node, "Workspace", out_anchor->GetIdx())生成名称(reduce_schedule_case_generator.cpp:442-444)。当不同 reduce 节点被分区时,可能产生同名的 workspace 节点。这些重名节点进入图后,后续的克隆、分区操作按名称查找就会出错。修改点 1:新增
NodeCloneMap类型别名文件:
autofuse/inc/graph_metadef/graph/utils/graph_utils.h:59// 新增 using NodeCloneMap = std::unordered_map<const Node *, NodePtr>;为什么改:旧方案用
std::unordered_map<std::string, NodePtr>按节点名做映射。unordered_map的 key 唯一,重名节点后插入的会覆盖先插入的,导致映射中丢失节点。改用裸指针const Node *作 key,每个节点的内存地址天然唯一,不受名称冲突影响。修改点 2:新增基于指针的
RelinkGraphEdges重载文件:
autofuse/graph_metadef/graph/utils/graph_utils.cc:2978-3048+graph_utils.h:942-947新增 3 个函数:
RelinkClonedDataEdges— 按指针映射重连数据边RelinkClonedControlEdges— 按指针映射重连控制边GraphUtils::RelinkGraphEdges(const NodePtr &node, const NodeCloneMap &node_clone_map)— 公开入口为什么改:旧的
RelinkGraphEdges(graph_utils.cc:2984)通过node->GetName() + suffix在 map 中查找克隆后的新节点。重名场景下查找会命中错误的节点,导致边连到错误目标。新实现用node_clone_map.find(node.get())直接按原始节点指针查找对应的克隆节点,精确无误。同时显式处理了控制边(
RelinkClonedControlEdges),旧实现在同一函数中也处理控制边,新实现拆分为独立函数,逻辑更清晰。修改点 3:
DoCopyAscNodeAndRelink改用指针映射文件:
autofuse/graph_metadef/graph/ascendc_ir/core/ascendc_ir.cc:640-653// 旧 std::unordered_map<std::string, NodePtr> all_new_nodes; all_new_nodes[dst_new_node->GetName()] = ...; GraphUtils::RelinkGraphEdges(src_node, "", all_new_nodes); // 新 NodeCloneMap node_clone_map; node_clone_map[src_node.get()] = ...; GraphUtils::RelinkGraphEdges(src_node, node_clone_map);为什么改:图克隆(
CopyFrom)时,如果源图中有重名节点(如 Reduce 分区产生的同名 workspace),按名映射会让后插入的同名节点覆盖先前的,克隆后边的重连会连到错误节点。改用src_node.get()作 key,保证每个源节点都能映射到正确的克隆节点。修改点 4:
PartitionByConnectivity输出节点收集从 map 改为 vector+sort文件:
autofuse/optimize/task_generator/schedule_group_partitioner.cpp:68-82// 旧 std::map<std::string, ::ascir::NodeView> name_to_output_nodes; // ... name_to_output_nodes.emplace(node->GetName(), node); for (const auto &name_and_node : name_to_output_nodes) { node_order.emplace_back(name_and_node.second); } // 新 std::vector<AscNodePtr> output_nodes; // ... output_nodes.emplace_back(node); node_order = std::move(output_nodes); std::sort(node_order.begin(), node_order.end(), CompareByNodeId);为什么改:两个原因:
避免重名输出节点被去重丢失:
std::map<std::string, ...>的 key 唯一,重名输出节点会被去重。丢失输出节点后,该节点所在的连通分量不会被遍历到,最终visited.size() != num_nodes检查失败(schedule_group_partitioner.cpp:102-109),图分区直接报错。改用vector保留所有输出节点。保证确定性排序:直接用
vector会导致遍历顺序不确定(取决于图中节点排列),违反编码红线规则 5(图改写结果必须确定)。因此用CompareByNodeId(按GetOpDesc()->GetId()排序)保证确定性的遍历顺序。旧实现靠std::map的 key 排序(按名称字典序),现在名称不可靠,改用 OpDesc ID 排序。修改点 5:
AddConnectedNodes和MergeGraphs改用指针映射文件:
autofuse/optimize/task_generator/schedule_group_partitioner.cpp:155,268,271,312,316// 旧 std::unordered_map<std::string, af::NodePtr> all_new_nodes; all_new_nodes[dst_new_node->GetName()] = dst_new_node; GraphUtils::RelinkGraphEdges(src_node, "", all_new_nodes); // 新 af::NodeCloneMap node_clone_map; node_clone_map[asc_node.get()] = dst_new_node; GraphUtils::RelinkGraphEdges(src_node, node_clone_map);为什么改:与修改点 3 相同的原因。子图构建和图合并过程中,如果连通分量内有重名节点,按名映射会导致边重连错误。
修改点 6:新增
DoCopyAscNodeOutputTensorAttr函数文件:
autofuse/optimize/task_generator/reduce_schedule_case_generator.cpp:53-77// 旧调用(PartitionByNode 中) DoCopyAscNodeTensorAttr(src_node, load_node); // 复制 src_node 的所有输出 DoCopyAscNodeTensorAttr(src_node, store_node); // 新调用 DoCopyAscNodeOutputTensorAttr(src_node, load_node, out_anchor->GetIdx()); // 只复制指定输出 DoCopyAscNodeOutputTensorAttr(src_node, store_node, out_anchor->GetIdx());为什么改:旧函数
DoCopyAscNodeTensorAttr(第 34-51 行)遍历src_node->outputs()的所有输出,逐个复制到dst_node的对应索引:for (size_t i = 0U; i < src_node->outputs().size(); i++) { auto tensor_attr_group = op_desc->MutableOutputDesc(i)->GetOrCreateAttrsGroup<...>(); *tensor_attr_group = src_node->outputs[i].attr; }但
load_node和store_node是单输出节点(只有 1 个 output)。当src_node(reduce 节点)有多个输出时,i会超过 load_node/store_node 的输出数量,MutableOutputDesc(i)越界访问。新函数
DoCopyAscNodeOutputTensorAttr只将src_node的第src_output_index个输出的属性复制到dst_node的第 0 个输出,避免越界:const auto output_desc = op_desc->MutableOutputDesc(0U); // dst_node 的第 0 个输出 *tensor_attr_group = src_node->outputs[src_output_index].attr; // src_node 的指定输出修改点 7:3 处节点匹配从名称比较改为指针比较
文件:
autofuse/optimize/task_generator/reduce_schedule_case_generator.cpp,3 个函数各 1 处// 旧(PartitionByNode:460, PartitionLoad:507, PartitionScalar:531) if (peer_in_anchor->GetOwnerNodeBarePtr()->GetName() == dst_node->GetName()) // 新 if (peer_in_anchor->GetOwnerNodeBarePtr() == dst_node.get())为什么改:这 3 处逻辑是遍历
src_node的下游节点,判断哪个 peer 是目标节点dst_node,以执行RemoveEdge(src->dst)+AddEdge(load->dst)。旧实现通过名称比较判断,当图中存在与dst_node同名的其他节点时,会误判为匹配,导致:改用指针直接比较
== dst_node.get(),精确匹配目标节点对象本身,不受重名干扰。修改点 8:测试适配——从按索引断言改为按集合断言
文件:
autofuse/tests/st/optimize/optimize_st.cpp+autofuse/tests/ut/optimize/test_optimizer.cpp8a.
ConcatFirstDim测试适配// 旧:按索引断言 schedule_groups[0] 和 [1] 的 axis 名称 EXPECT_EQ(axis_names_0, expected_0); EXPECT_EQ(axis_names_1, expected_1); // 新:用 set<set<string>> 收集所有 group 的结果,不依赖顺序 std::set<std::set<std::string>> axis_name_groups; // ... 收集所有 group EXPECT_EQ(axis_name_groups, expected_axis_name_groups);为什么改:修改点 4 将输出节点排序从按名称字典序改为按 OpDesc ID。这导致
schedule_groups的顺序可能变化(不再保证schedule_groups[0]对应某一组)。旧的硬编码索引断言不再可靠,改用集合比较,不依赖顺序。8b.
EliminateSizeVar测试适配// 旧:按索引逐个断言 schedule_groups[0..6] 的 SizeVar 数量 EXPECT_EQ(schedule_groups[0]...GetAllSizeVar().size(), 4UL); EXPECT_EQ(schedule_groups[1]...GetAllSizeVar().size(), 5UL); // ... 7 个 // 新:用 set<size_t> 收集所有数量,与预期集合比较 std::set<size_t> size_var_counts; // ... 收集 const std::set<size_t> expected_size_var_counts{4UL, 5UL, 6UL, 7UL, 8UL, 9UL, 10UL}; EXPECT_EQ(size_var_counts, expected_size_var_counts);为什么改:同上,group 顺序不再确定。
8c.
NodeCacheMarkerConcat测试适配// 旧:按固定索引路径断言 EXPECT_EQ(schedule_results[1].schedule_groups[1].impl_graphs.size(), 3UL); const auto &impl6_brc0 = impl_graphs[2].FindNode("brc0"); // 新:遍历所有 schedule_result、group、impl_graph 查找目标节点 bool found_cached_brc0 = false; // ... 遍历查找 EXPECT_TRUE(found_cached_brc0);为什么改:同上,索引顺序不再确定,改为遍历查找。
修改点 9:新增重名节点 UT
文件:
autofuse/tests/ut/optimize/task_generator/test_schedule_group_partitioner.cpp9a.
DuplicateWorkspaceNames_CopyAndPartitionKeepEdges构造含两个 workspace 节点的图,手动将两个 workspace 重命名为同名
"workspace",然后:copied_graph.CopyFrom(graph)— 验证克隆后 workspace 边正确(一个 workspace 有 Data 输入,另一个有 Load 输出)PartitionByConnectivity(graph)— 验证分区后边仍正确9b.
DuplicateWorkspaceNames_PartitionDoesNotDropComponents构造两个独立连通分量(data0→workspace0, data1→workspace1),将两个 workspace 重命名为同名,验证分区后仍得到 2 个子图(不被去重为 1 个)。
为什么新增:直接复现重名节点场景,回归验证修复效果。符合编码红线通用规则 7(不做无关重构)和测试规范(Bug 修复必须有复现用例)。
修改点总结
NodeCloneMapRelinkGraphEdges重载DoCopyAscNodeAndRelink改用指针AddConnectedNodes/MergeGraphs改用指针DoCopyAscNodeOutputTensorAttr