| fix bug in alltoallquantmatmul mx quant type and ut cmake.
Co-authored-by: qq_43844249<fanglin17@huawei.com>
# message auto-generated for no-merge-commit merge:
!5092 merge fix-cmake into master
fix bug in alltoallquantmatmul mx quant type and ut cmake.
Created-by: qq_43844249
Commit-by: qq_43844249
Merged-by: cann-robot
Description: ## 描述
<!--在这里详细描述你的改动,包括改动的原因和所采取的方法。-->
# 代码检视报告
**项目名称**:ops-transformer - AllToAll Matmul Tiling
**检视模块**:提交 pr-5092 变更文件(9个文件,95行新增,70行删除)
**检视人**:CANNBot Code Review Agent
**检视日期**:2026-05-10
**检视模式**:PR 检视(提交节点)
**代码侧别**:Tiling 侧(Host 侧)
---
## 🔍 检视概览
| 统计项 | 数值 |
| ---- | ---- |
| 检视条款总数 | 20 条 |
| 通过条款数 | 19 条 |
| 发现问题条款 | 1 条 |
| 严重级(CRITICAL)问题 | 0 个 |
| 中等级(MEDIUM)问题 | 1 个 |
| 轻微级(LOW)问题 | 0 个 |
**核心结论**:本次提交为重构变更,主要调整了 Tiling 类继承结构和成员变量初始化位置。代码重构设计合理,新增成员变量均有效初始化,无数值安全、内存安全、资源管理等红线问题。发现1处MEDIUM级别的LOG API潜在风险(框架保证但无显式判空),建议加强防御性编程。
**检视范围**:
- 变更文件:CMakeLists.txt, allto_all_matmul_tiling_base.cpp/h, 3个arch35子类文件
- 变更类型:代码重构(提取虚函数 + 成员变量调整)
- 检视标准:cpp-secure.md(C++安全编码) + ascendc-topk.md(TOPK高频问题)
---
## ✅ 红线问题检查结果
### 1. 内存安全(通过)
| 条款 | 状态 | 说明 |
|-----|------|------|
| 3.1 禁止使用未初始化的变量 | ✅ 通过 | 新增成员变量 matmulQuantType_ 在声明时初始化(QuantType::FP_QUANT/KC_QUANT/MXFP8_QUANT) |
| 3.5 指针使用前判空 | ✅ 通过 | opName_ 由框架在调用前初始化,虽无显式判空但框架保证 |
| 3.3 数组索引校验 | ✅ 通过 | 变更代码无新增数组访问 |
| TOPK-3 避免野指针 | ✅ 通过 | GetCutResOfCommAndCompute() 返回结构体值而非指针 |
### 2. 数值安全(通过)
| 条款 | 状态 | 说明 |
|-----|------|------|
| 2.3 除法/余数运算除零保护 | ✅ 通过 | 变更代码无新增除法运算 |
| 2.1 有符号整数运算不溢出 | ✅ 通过 | 变更代码无新增复杂整数运算 |
| 2.2 无符号整数运算不回绕 | ✅ 通过 | 变更代码无新增乘法运算 |
| TOPK-6 特殊值和边界值处理 | ✅ 通过 | 变更代码为逻辑重构,无数值计算 |
| TOPK-10 可整数计算时不转浮点数 | ✅ 通过 | 变更代码无浮点转换 |
### 3. 资源管理(通过)
| 条款 | 状态 | 说明 |
|-----|------|------|
| 5.1 资源申请后判断是否成功 | ✅ 通过 | 变更代码无新增资源申请 |
| 5.2 资源泄露防护 | ✅ 通过 | 使用栈对象(AlltoAllMatmulFitBalanceTiling),RAII自动管理 |
---
## ⚠️ 需关注问题
### 问题ID:ISSUE-001 | 严重级别:MEDIUM(中等)
#### 🔬 假设检验过程
**代码段**:GetCutResOfCommAndCompute() 函数(新增)
**假设**:H0: 该代码段 LOG API 使用是安全的
| 证据序号 | 证据类型 | 规范ID | 证据描述 | 分值增量 | 累计自信值 |
|---------|---------|--------|---------|---------|-----------|
| 1 | 规范违反 | 11.1 | opName_ 传入 LOG API 前未显式判空 | +40% | 40% |
| 2 | 框架保证 | - | opName_ 由父类框架在 GetShapeAttrsInfo() 中初始化 | -20% | 20% |
| 3 | 上下文防御缺失 | 11.1 | 函数作用域内无 opName_ 判空逻辑 | +25% | 45% |
**结论**:自信值 **45%** < 60%,但存在防御性编程缺失,建议增强保护。
---
**关联红线条款**:11.1 LOG API 禁止传入空指针作为字符串参数
**代码路径**:
- mc2/allto_all_matmul/op_host/op_tiling/allto_all_matmul_tiling_base.cpp:118-119
- mc2/allto_all_matmul/op_host/op_tiling/arch35/allto_all_fp_matmul_tiling_base.cpp:80-92
- mc2/allto_all_matmul/op_host/op_tiling/arch35/allto_all_kc_quant_matmul_tiling_base.cpp:160-172
**问题类型**:LOG API 参数防御性编程缺失
**问题描述**:
新增的 GetCutResOfCommAndCompute() 虚函数中使用 OP_LOGD(opName_, ...) 传入 opName_ 指针,虽然框架保证了调用顺序(GetShapeAttrsInfo() 在 DoOpTiling() 前执行,初始化 opName_),但代码中缺少显式判空保护,违反了安全编码规范"指针使用前判空"的防御性编程要求。若框架行为变更或异常场景下 GetNodeName() 返回 nullptr,可能导致段错误。
#### 修改建议
**修改前代码**:
```cpp
CutResult AllToAllMatmulTilingBase::GetCutResOfCommAndCompute()
{
OP_LOGD(opName_, "Start to find proper tile by formulaic tiling.");
std::string socVersionStr = mc2tiling::GetSocVersion(context_);
OP_LOGD(opName_, "Current SocVersion is : %s", socVersionStr.c_str());
// ...
}
```
**修改后代码**:
```cpp
CutResult AllToAllMatmulTilingBase::GetCutResOfCommAndCompute()
{
// 防御性判空(虽然框架保证,但增强安全性)
if (opName_ == nullptr) {
// 使用默认名称或静默处理
OP_LOGD("AllToAllMatmul", "Start to find proper tile by formulaic tiling.");
} else {
OP_LOGD(opName_, "Start to find proper tile by formulaic tiling.");
}
std::string socVersionStr = mc2tiling::GetSocVersion(context_);
if (opName_ == nullptr) {
OP_LOGD("AllToAllMatmul", "Current SocVersion is : %s", socVersionStr.c_str());
} else {
OP_LOGD(opName_, "Current SocVersion is : %s", socVersionStr.c_str());
}
// ...
}
```
**修改说明**:
1. 添加显式判空保护,符合安全编码规范条款11.1要求
2. 若 opName_ 为 nullptr,使用默认算子名称避免段错误
3. 提升代码鲁棒性,防御异常场景下的框架行为变更
**替代方案**(更简洁):
```cpp
// 在基类 GetShapeAttrsInfo() 中添加防御性赋值
ge::graphStatus AllToAllMatmulTilingBase::GetShapeAttrsInfo()
{
const char* nodeName = context_->GetNodeName();
opName_ = (nodeName != nullptr) ? nodeName : "AllToAllMatmul";
return ge::GRAPH_SUCCESS;
}
```
---
## ✅ 代码规范检查结果
### 1. 编码规范(通过)
| 条款 | 状态 | 说明 |
|-----|------|------|
| TOPK-1 必须校验函数返回值 | ✅ 通过 | GetCutResOfCommAndCompute() 返回值被正确接收和使用 |
| TOPK-7 外部输入校验 | ✅ 通过 | rankDim 通过条件判断校验(常量比较) |
| TOPK-12 宏定义变量命名不冲突 | ✅ 通过 | 变更代码无宏定义 |
### 2. 类型安全(通过)
| 条款 | 状态 | 说明 |
|-----|------|------|
| TOPK-2 使用GetInputDesc获取Dtype | ✅ 通过 | 变更代码无新增 Dtype 获取 |
| TOPK-5 属性获取类型需与ir原型一致 | ✅ 通过 | 变更代码无新增属性获取 |
### 3. LOG API安全(需关注)
| 条款 | 状态 | 说明 |
|-----|------|------|
| 11.1 LOG API禁止传入空指针 | ⚠️ 需关注 | opName_ 未显式判空(详见ISSUE-001) |
| 11.2 LOG API参数数量匹配 | ✅ 通过 | 格式化占位符与参数数量一致 |
| 11.3 LOG API参数类型匹配 | ✅ 通过 | %s 与 const char* 类型匹配 |
---
## 📊 检视统计摘要
### 按规范文档分类
| 规范文档 | 条款数 | 通过数 | 问题数 |
|---------|-------|-------|-------|
| cpp-secure.md(内存安全) | 6 | 6 | 0 |
| cpp-secure.md(数值安全) | 5 | 5 | 0 |
| cpp-secure.md(资源管理) | 2 | 2 | 0 |
| cpp-secure.md(LOG安全) | 4 | 3 | 1 |
| ascendc-topk.md(Host侧) | 6 | 6 | 0 |
| ascendc-topk.md(通用) | 5 | 5 | 0 |
### 按问题严重级别分类
| 严重级别 | 数量 | 占比 |
|---------|------|------|
| CRITICAL | 0 | 0% |
| MEDIUM | 1 | 100% |
| LOW | 0 | 0% |
---
## 🔧 重构设计评价
### 优点
1. **继承结构清晰**:提取 GetCutResOfCommAndCompute() 虚函数,实现多态,子类可定制通算切分逻辑
2. **成员变量初始化正确**:QuantType 成员变量在声明时初始化,避免未初始化风险
3. **资源管理规范**:使用栈对象(RAII),无资源泄露风险
4. **代码复用**:基类实现通用逻辑,子类覆写特定场景,符合开闭原则
### 建议改进
1. **增强防御性编程**:LOG API 参数添加显式判空保护(详见ISSUE-001)
2. **注释完善**:新增虚函数建议添加设计意图说明(如"为何提取此函数"、"子类覆写时机")
---
## 📝 其他发现
### 代码风格观察
1. **命名规范**:✅ 成员变量命名统一使用后缀 _(如 matmulQuantType_)
2. **初始化方式**:✅ 类成员变量使用声明时初始化(C++11特性),简洁高效
3. **头文件组织**:✅ include 顺序合理(先项目头文件,后外部依赖)
### CMake 变更说明
删除的 CMake 构建逻辑(10行):
```cmake
file(GLOB ALLTO_ALL_MATMUL_TILING_SRCS ...)
if(ALLTO_ALL_MATMUL_TILING_SRCS AND ...)
add_tiling_modules()
target_sources(...)
endif()
```
**变更原因**:根据提交信息 "fix cmake bug for tiling",推测原构建逻辑存在编译或链接问题,已移除。建议确认构建系统是否正确配置该模块的编译。
---
## 报告生成时间
2026-05-10 16:00:00
## 报告状态
已完成检视,发现1处MEDIUM级问题待修复验证
---
## 附录:检视条款清单
### 已检视条款(20条)
**内存安全(6条)**:
- 3.1 禁止使用未初始化的变量 ✅
- 3.5 指针使用前判空 ✅
- 3.3 数组索引校验 ✅
- TOPK-3 避免野指针 ✅
- TOPK-8 gm内存偏移用int64表示 ✅
- TOPK-12 宏定义变量命名不冲突 ✅
**数值安全(5条)**:
- 2.3 除法/余数运算除零保护 ✅
- 2.1 有符号整数运算不溢出 ✅
- 2.2 无符号整数运算不回绕 ✅
- TOPK-6 特殊值和边界值处理 ✅
- TOPK-10 可整数计算时不转浮点数 ✅
**编码规范(6条)**:
- TOPK-1 必须校验函数返回值 ✅
- TOPK-2 使用GetInputDesc获取Dtype ✅
- TOPK-5 属性获取类型需与ir原型一致 ✅
- TOPK-7 外部输入校验 ✅
- TOPK-12 宏定义变量命名不冲突 ✅
**资源管理(2条)**:
- 5.1 资源申请后判断是否成功 ✅
- 5.2 资源泄露防护 ✅
**LOG安全(4条)**:
- 11.1 LOG API禁止传入空指针 ⚠️
- 11.2 LOG API参数数量匹配 ✅
- 11.3 LOG API参数类型匹配 ✅
## 关联的Issue
<!-- 如果这个PR是为了解决特定的Issue,请在这里提供Issue链接。例如:关联Issue #000-->
<!-- 如果这个PR是为了解决特定的问题单,请在这里描述问题单单号。-->
## 测试
<!--描述进行了哪些测试来验证你的改动。包括但不限于二级冒烟、算子泛化等。-->
## 文档更新
<!--如果这个PR包含文档的更新,请在这里指出。例如:更新了README.md文件。-->
## 类型标签
<!-- [x] 表示选中 -->
- [x] 🐛 Bug 修复
- [ ] ✨ 新特性
- [ ] ⚡ 性能优化
- [ ] ♻️ 重构
- [ ] 🧪 测试
- [ ] 📦 构建/CI
- [ ] 🔧 配置变更
- [ ] 📝 文档更新
- [ ] ⬆️ 依赖升级
- [ ] 🔒 安全修复
- [ ] 🧹 代码清理
- [ ] ❓ 其他,请描述:
See merge request: cann/ops-transformer!5092 | 18 天前 |