已合并
add assert #8422
link164735创建于 8月7日
add assert #8422
已合并
link164735创建于 8月7日
link164735
link164735成员
8月7日

描述

此 PR 在三个 GEMM kernel 模板类中添加了编译期 static_assert,对 AType、BType、CType、BiasType 类型组合以及 LayoutA、LayoutB、LayoutC、LayoutBias 布局组合进行白名单校验,防止用户传入不支持的类型或布局组合而导致不可预期的运行时行为。

关联的Issue

#4755

测试

本地验证通过,二级冒烟通过

文档更新

类型标签

AI/Agent生成声明

likedislike
Pull Request已成功合入, 合并人@CANN-robot
(感谢 link164735 的贡献)
link164735link164735成员
8月7日 创建了 pull request,commit 40e9866d
atomgit-bot
atomgit-bot
8月7日 评论:

变更摘要

此 PR 主要将多个矩阵乘法 Kernel 中的默认数据类型宏从固定 half 改为跟随输入数据类型 DTYPE_X1,同时更新了第三方依赖 ops-tensor 的仓库地址和提交版本。PR 标题为"add assert",但实际 diff 中未体现 assert 相关改动。

主要改动

  • 统一 DTYPE_YDTYPE_BIAS 的默认类型为 DTYPE_X1:在 mat_mul_input_k_eq_zero_clear_output.h 中,将 DTYPE_YDTYPE_BIAS 的默认值从 half 改为 DTYPE_X1,使输出类型和偏置类型不再硬编码为半精度,而是与输入数据类型保持一致。
  • mat_mul_v3.cppDTYPE_BIAS 默认值调整:将 DTYPE_BIAS 的条件默认值从 half 改为 DTYPE_X1,与其他 Kernel 文件保持一致的偏置类型推导逻辑。
  • transpose_quant_batch_mat_mul.cppDTYPE_BIAS 默认值调整:同样将偏置类型的回退默认值从 half 改为 DTYPE_X1,使量化批量矩阵乘法的偏置类型推导与输入数据类型对齐。
  • 更新 ops-tensor 第三方依赖:在 cmake/third_party/ops-tensor.cmake 中,将 OPTENSOR_TAG_ID 指向新的提交 a80b1176,并将 Git 仓库地址从 cann/ops-tensor 改为作者个人分支 huangkejie1647/ops-tensor
likedislike
不准确?
atomgit-bot
atomgit-bot
8月7日 评论:

代码审查

现在让我再检查一下 cmake 文件中 URL 变更(第 85、88 行)是否需要单独报告——它们与 TAG_ID 变更是同一根因的组成部分。已完成最终扫描,下面做总结。


审查总结

按优先级统计

优先级 数量
P0 1
P1 0
P2 0
P3 0

各文件审查结果

文件 结论
cmake/third_party/ops-tensor.cmake 1 个 P0 问题:ops-tensor 依赖指向作者个人 fork,TAG_ID 和 Git URL 均需恢复为官方仓库
matmul/mat_mul_v3/op_kernel/arch35/mat_mul_input_k_eq_zero_clear_output.h 无问题。DTYPE_YDTYPE_BIAS 默认值从硬编码 half 改为 DTYPE_X1,使回退默认值与输入类型一致,是合理的泛化改进。
matmul/mat_mul_v3/op_kernel/arch35/mat_mul_v3.cpp 无问题。DTYPE_BIAS 回退默认值变更与头文件保持一致。
matmul/transpose_quant_batch_mat_mul/op_kernel/arch35/transpose_quant_batch_mat_mul.cpp 无问题。DTYPE_BIAS 回退默认值变更与其他文件保持一致。

整体风险评估

高风险,不应合并。 cmake 文件中将 ops-tensor 依赖指向个人 fork 的变更是明确的构建阻断和供应链安全问题。该变更疑似本地开发产物被误提交——PR 标题为"add assert"但与实际 diff 内容完全不符。C++ 侧的三个类型默认值变更本身是正确的改进,但建议在修复 cmake 问题后一并审查。

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

⛔ 需要修改

likedislike
不准确?
CANN-robotCANN-robot成员
8月7日 添加了label:cann-cla/yes
CANN-robot
CANN-robot成员
8月7日 评论:

Thanks for your pull-request.
The full list of commands accepted by me can be found at here
You can get sig-info at here


PR Approval Progress

Congratulations! All modules have met the lgtm and approve requirements.

Module Approval Details

module lgtm status approve status
cmake 商晓波, 林鹏翔 (2/2) 商晓波 (1/1)
matmul 商晓波, 林鹏翔 (2/2) 商晓波 (1/1)

💡 Tip:

  • Committer can comment /approve or /lgtm
  • Commenting /approve implies both code review (lgtm) and intent to merge (approve)

CLA Signature Pass

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

likedislike
此处折叠了99条消息 查看更多
sxb154714
sxb154714成员
26 天前 评论:

/lgtm
/approve

likedislike
CANN-robotCANN-robot成员
26 天前 添加了label:lgtmapproved
CANN-robotCANN-robot成员
26 天前 关闭了关联的issue
CANN-robotCANN-robot成员
26 天前 合入了pull request
CANN-robot
CANN-robot成员
26 天前 评论:

Pull Request 已合并或已关闭。

If you want to solve this problem, you can click here to do it in the FAQs.

likedislike