| matmulAllReduce 8p性能优化公式化tiling
Co-authored-by: ouyf<ouyangfei9@huawei.com>
# message auto-generated for no-merge-commit merge:
!4869 merge master into master
matmulAllReduce 8p性能优化公式化tiling
Created-by: ouyf
Commit-by: ouyf
Merged-by: cann-robot
Description: ## 描述
<!--在这里详细描述你的改动,包括改动的原因和所采取的方法。-->
## 关联的Issue
<!-- 如果这个PR是为了解决特定的Issue,请在这里提供Issue链接。例如:关联Issue #000-->
<!-- 如果这个PR是为了解决特定的问题单,请在这里描述问题单单号。-->
## 测试
<!--描述进行了哪些测试来验证你的改动。包括但不限于二级冒烟、算子泛化等。-->
## 文档更新
<!--如果这个PR包含文档的更新,请在这里指出。例如:更新了README.md文件。-->
## 类型标签
<!-- [x] 表示选中 -->
- [ ] 🐛 Bug 修复
- [ ] ✨ 新特性
- [ ] ⚡ 性能优化
- [ ] ♻️ 重构
- [ ] 🧪 测试
- [ ] 📦 构建/CI
- [ ] 🔧 配置变更
- [ ] 📝 文档更新
- [ ] ⬆️ 依赖升级
- [ ] 🔒 安全修复
- [ ] 🧹 代码清理
- [ ] ❓ 其他,请描述:
================================================================================
PR #4869 Review (Round 2): matmulAllReduce公式化tiling
Repository: cann/ops-transformer
Commit: 26e6fcc1 (updated)
Date: 2026-04-29
================================================================================
## 变更概要 (更新后)
本PR涉及7个文件的修改,共+47/-28行:
1. hccl_performance_arch35.cpp: 新增8P AllReduce(two shot)拟合参数
2. matmul_all_reduce_tiling_950.cpp/.h: 扩展A2A通路支持8P
3. quant_matmul_all_reduce_tiling_950.cpp/.h: 扩展A2A通路支持8P
4. weight_quant_matmul_all_reduce_tiling_950.cpp/.h: 扩展A2A通路支持8P
核心意图:将MatmulAllReduce系列算子的A2A通路从仅支持4P扩展到同时支持4P和8P。
## 上一轮检视问题修复状态
| 问题 | 严重级别 | 状态 | 说明 |
|------|---------|------|------|
| TopoType硬编码导致8P参数无法命中 | P0 | ✓ 已修复 | 三个文件均添加了else if (Is8P)分支,使用TopoType::EIGHT_P |
| 函数命名与语义不符 | P1 | ✓ 已修复 | GetWorkspaceSizeInStandardCard4P -> GetWorkspaceSizeForA2ARSAG |
| 条件判断缺少空格 | P2 | ✓ 已修复 | matmul_all_reduce中已修正为if (mc2tiling::...) |
================================================================================
## [P2 - 代码风格] else if 后缺少空格
### 问题描述
三个文件的GetTilingResult()中新增的else if分支,)和{之间缺少空格,
与同文件其他else if分支风格不一致。
### 影响位置
- matmul_all_reduce_tiling_950.cpp:411
} else if (mc2tiling::Is8P(args_.rankDim, npuArch_)){
- quant_matmul_all_reduce_tiling_950.cpp:1009
} else if (mc2tiling::Is8P(args_.rankDim, npuArch_)){
- weight_quant_matmul_all_reduce_tiling_950.cpp:696
} else if (mc2tiling::Is8P(args_.rankDim, npuArch_)){
### 对比参考
同仓库 matmul_reduce_scatter_v2_tiling.cpp:212 的正确写法:
} else if (mc2tiling::Is8P(args_.rankDim, npuArch_)) { // {前有空格
### 修复建议
统一改为 } else if (mc2tiling::Is8P(args_.rankDim, npuArch_)) {
================================================================================
## [P2 - 代码风格] weight_quant文件中if后缺少空格
### 问题描述
weight_quant_matmul_all_reduce_tiling_950.cpp:274 (修改后)
if(mc2tiling::IsUseA2APath(args_.rankDim, npuArch_)){
if和(之间缺少空格,且){之间也缺少空格。
### 对比
同文件第466行的SetMc2Hcomm()中:
if (isUseA2APath) { // 格式正确
### 修复建议
改为 if (mc2tiling::IsUseA2APath(args_.rankDim, npuArch_)) {
================================================================================
## [P3 - 建议] GetTilingResult()中未使用的变量
### 问题描述
三个文件的GetTilingResult()方法中声明了以下变量但未使用:
```cpp
const gert::StorageShape* commQuantScaleShape1 = mmrCtxInfo_.comm_quant_scale_1_shape;
const gert::StorageShape* commQuantScaleShape2 = mmrCtxInfo_.comm_quant_scale_2_shape;
```
这些变量在GetTilingResult()中声明后未被引用,可能是历史遗留代码。
虽然不影响功能,但会触发编译器unused variable警告。
### 影响位置
- matmul_all_reduce_tiling_950.cpp:404-405
- quant_matmul_all_reduce_tiling_950.cpp:1003-1004
- weight_quant_matmul_all_reduce_tiling_950.cpp:690-691
### 修复建议
建议在本PR中一并清理,或单独提交清理。非阻塞性问题。
================================================================================
## [正面评价]
1. P0问题修复正确:三个文件均正确添加了Is8P分支并使用TopoType::EIGHT_P,
现在"4_5_1_8"拟合参数可以正确命中。
2. 函数重命名合理:GetWorkspaceSizeForA2ARSAG准确反映了函数支持A2A+RS/AG通路,
不再局限于4P场景。
3. 与matmul_reduce_scatter_v2的实现模式保持一致,降低了后续维护成本。
4. 头文件(.h)中的声明同步更新,保持了接口一致性。
5. 错误日志信息同步更新(如"get workspace size By GetWorkspaceSizeForA2ARSAG failed"),
便于问题排查。
================================================================================
## 检视总结
| 严重级别 | 数量 | 说明 |
|---------|------|-------------------------------|
| P0 | 0 | 上一轮P0问题已全部修复 |
| P2 | 2 | 代码风格问题(空格不一致) |
| P3 | 1 | 未使用变量建议清理 |
结论: 上一轮P0缺陷已修复,本PR可以合入。
剩余问题均为代码风格和小建议,不阻塞合入,建议在合入前或后续PR中修正。
================================================================================
See merge request: cann/ops-transformer!4869 | 1 个月前 |