已合并
Added clipped_swiglu op with clamp_mode #8534
clwsy创建于 23 天前
Added clipped_swiglu op with clamp_mode #8534
已合并
clwsy创建于 23 天前
clwsy
clwsy成员
23 天前

描述

为ClippedSwiglu算子新增clamp_mode属性,用于控制clamp与silu操作的执行顺序

关联的Issue

#4940

测试

进行了ttk,aclnn,图模式以及api测试,均验证通过

文档更新

更新了README、aclnn.md文档

类型标签

AI/Agent生成声明

likedislike
Pull Request已成功合入, 合并人@CANN-robot
(感谢 clwsy 的贡献)
clwsyclwsy成员
23 天前 创建了 pull request,commit 5c2fc60d
atomgit-bot
atomgit-bot
23 天前 评论:

变更摘要

此次变更为 ClippedSwiglu 算子新增了 clamp_mode 属性支持,通过在算子属性、tiling 策略、kernel 计算逻辑、tiling key 调度等各层引入新的参数维度,实现了两种不同的 clamp 计算模式。当 clamp_mode=0 时保持原有行为(对 x1 使用 alpha 缩放的 sigmoid 后 clamp),当 clamp_mode=1 时采用新的计算路径(标准 sigmoid 后再 clamp,不使用 alpha 缩放)。同时新增了 aclnnClippedSwigluV2 的端到端测试用例。

主要改动

  • 算子属性新增 clamp_mode:在 clipped_swiglu_proto.hclipped_swiglu_def.cpp 中为算子注册新增 clamp_mode 属性(Int 类型,默认值 0),并在 clipped_swiglu_def.cpp 中将其版本标记为 CLIPPED_SWIGLU_VERSION_TWO

  • Tiling 层适配 clamp_mode:在 clipped_swiglu_tiling_arch35.cpp 中新增 CLAMPMODE_INDEX 常量和 clampMode_ 成员变量,加入对 clamp_mode 取值的合法性校验(仅允许 0 或 1),并将 clampMode 纳入 SetTilingKey() 的 tiling key 计算和 PrintTilingInfo() 的日志输出。

  • Kernel 计算逻辑分支化:在 clipped_swiglu_kernel.h 中为 ClippedSwigluKernel 模板类新增 clampMode 模板参数,并在 ComputeVfSwiglu 的核心计算中通过 if constexpr 分支:clampMode==0 保持原有 Mins→Muls(negAlpha)→Exp→Adds→Div 路径;clampMode==1 采用 Muls(negScalarOne)→Exp→Adds→Div→Mins(clampLimit) 的新路径,且不再对 x2 添加 bias。

  • Tiling key 调度扩展:在 clipped_swiglu_tiling_key.h 中新增 TPL_NO_CLAMP_MODE / TPL_CLAMP_MODE 宏定义,并将 clampMode 作为新的 tiling 维度加入 ASCENDC_TPL_ARGS_DECLASCENDC_TPL_SEL 模板展开中。

  • 配置文件与测试补充:在 ascend950/clipped_swiglu_binary.json 的三组 tiling 配置中均添加了 clamp_mode 字段;新增 test_aclnn_clipped_swiglu_v2.cpp 测试文件,覆盖 aclnnClippedSwigluV2 接口的端到端调用流程。

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

代码审查

审查总结

文件 审查结果
activation/clipped_swiglu/examples/test_aclnn_clipped_swiglu_v2.cpp 1 个 P2 问题
activation/clipped_swiglu/op_graph/clipped_swiglu_proto.h 无问题
activation/clipped_swiglu/op_host/clipped_swiglu_def.cpp 无问题
activation/clipped_swiglu/op_host/clipped_swiglu_tiling_arch35.cpp 无问题
activation/clipped_swiglu/op_host/config/ascend910_93/clipped_swiglu_binary.json 无问题(仅末尾换行)
activation/clipped_swiglu/op_host/config/ascend910b/clipped_swiglu_binary.json 无问题(仅末尾换行)
activation/clipped_swiglu/op_host/config/ascend950/clipped_swiglu_binary.json 无问题
activation/clipped_swiglu/op_kernel/arch35/clipped_swiglu_kernel.h 3 个问题(P1×1, P2×1, P3×1)

按优先级统计: P1: 1, P2: 2, P3: 1,共 4 个问题。

整体风险评估: 此次变更引入了 clamp_mode 属性以支持两种不同的 SwiGLU 裁剪模式。Tiling 侧(属性注册、校验、TilingKey 生成)实现正确,JSON 配置变更合理。但 kernel 侧存在两个需要关注的问题:尾块掩码从 maskT 错误改为 mask(可能导致数值异常),以及 clampMode == 1 分支静默忽略了 alpha/bias 参数(需确认是否为设计意图)。建议在合入前修复尾块掩码问题,并确认 clampMode == 1 的参数语义。


审查完成

经过对两个变更文件的详细审查,现报告如下:


文件 1: activation/clipped_swiglu/op_kernel/arch35/clipped_swiglu_tiling_key.h

变更内容

  • 新增 TPL_NO_CLAMP_MODE 0 / TPL_CLAMP_MODE 1 宏定义
  • ASCENDC_TPL_ARGS_DECL 末尾追加 clampMode 参数声明
  • ASCENDC_TPL_ARGS_SEL 末尾追加 clampMode 选择器

审查结论:无问题。新增字段按规范追加在原有字段末尾(符合规则 15:TilingKey 新增字段只能追加,不得修改历史顺序)。ASCENDC_TPL_UINT_SEL 的参数个数与 isInterleaved 一致(4 参数),isGroup 额外携带的 TPL_FLOOR 是其预存模式,clampMode 无需跟从。

文件 2: activation/clipped_swiglu/op_kernel/clipped_swiglu_apt.cpp

变更内容

  • 模板签名从 <uint64_t isInterleaved, uint64_t isGroup> 扩展为 <uint64_t isInterleaved, uint64_t isGroup, uint64_t clampMode>
  • 四处 ClippedSwigluKernel 实例化均通过 clampMode 作为第四模板参数

审查结论:无问题。clampMode 类型与 Kernel 模板声明 template <typename T, bool, bool, uint64_t clampMode> 一致;主机端 tiling(clipped_swiglu_tiling_arch35.cpp)已同步通过 GET_TPL_TILING_KEY(isInterleavedKey, isGroupKey, clampMode) 传递该值;Kernel 内部通过 if constexpr (clampMode == 0) 在编译期正确分支;主机端校验了 clampMode 取值仅为 0 或 1。


总结

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

整体风险判断:此变更安全、一致,未引入任何可检测的缺陷。两个变更文件均已逐一审查,均无问题。

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

⛔ 需要修改

likedislike
CANN-robotCANN-robot成员
23 天前 添加了label:cann-cla/yes
CANN-robot
CANN-robot成员
23 天前 评论:

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.
You can self-configure the PR merge rules for this repository. For more details, please refer to Here.
For more, you also can visit HICANN.


PR Approval Progress

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

Module Approval Details

module lgtm status approve status
*/*/README.md 陈娇, 唐玮玮 (2/2) 陈娇 (1/1)
*/*/docs/acl*.md 唐玮玮, 陈娇 (2/2) 陈娇 (1/1)
*/*/op_graph/*_proto.h 王永光, 唐玮玮 (2/2) 王永光 (1/1)
*/*/op_host/*_def.cpp 王永光, 唐玮玮 (2/2) 王永光 (1/1)
activation 苏跃明, 唐玮玮 (2/2) 苏跃明 (1/1)
docs 陈娇, 唐玮玮 (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

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

likedislike
此处折叠了279条消息 查看更多
tangweiwei2成员
12 天前 评论:

/lgtm
/approve

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

Pull Request 已合并或已关闭。

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

likedislike