已合并
【代码侦探Challenge07】 个人开发者 - challenge07_sigmoidcustom #3874
【代码侦探Challenge07】 个人开发者 - challenge07_sigmoidcustom #3874
已合并
JeffDing创建于 9月16日
JeffDing
JeffDing
9月16日

个人开发者

团队信息

成员

算子: challenge07_sigmoidcustom

PyTorch 原生算子:torch.sigmoid,即 y = 1 / (1 + exp(-x))。

likedislike
Pull Request已成功合入, 合并人@CANN-robot
(感谢 JeffDing 的贡献)
JeffDingJeffDing
9月16日 创建了 pull request,commit ef99cb47
atomgit-bot
atomgit-bot
9月16日 评论:

变更摘要

本 PR 为「代码侦探(第七期)」个人开发者提交,实现自定义 Sigmoid 算子(challenge07_sigmoidcustom),对应 PyTorch 原生算子 torch.sigmoid(y = 1 / (1 + exp(-x)))。改动覆盖算子 Host 侧定义、InferShape、Tiling、AscendC Kernel 实现、aclnn 调用示例以及完整的 UT 测试框架(含 Tiling/Kernel UT、数据生成与精度比对脚本),所有文件均为新增,支持 ascend910b 平台上的 float/half 两种数据类型。

主要改动

  • 算子 Host 侧定义与形状推导: sigmoid_def.cpp 注册 Sigmoid 算子(输入 x、输出 y,数据类型 DT_FLOAT/DT_FLOAT16,FORMAT_ND,AICore 配置 ascend910b);sigmoid_infershape.cpp 将输入形状原样传递给输出形状。
  • Tiling 逻辑: sigmoid_tiling.cpp 按元素总数、核数与 UB 容量计算 totalNum、blockFactor、ubFactor,设置 workspace 大小为 0,并按输入数据类型选择 tiling key(FLOAT16/BF16 用 mode 0,其余用 mode 1),对应 sigmoid_tiling_data.h 与 sigmoid_tiling_key.h 中的结构体和模板参数定义。
  • AscendC Kernel 实现: sigmoid.h/sigmoid.cpp 以模板类 NsSigmoid::Sigmoid<T> 实现 Kernel,通过 TPipe/TQue 队列和 DataCopyPad 完成数据搬入搬出,使用 AscendC::Sigmoid 计算,并按 tiling key 分别实例化 half/float 版本。
  • aclnn 调用示例: examples/test_aclnn_sigmoid.cpp 演示两段式接口调用流程(aclnnSigmoidGetWorkspaceSize 获取 workspace 大小、aclnnSigmoid 执行),包含设备内存申请、流同步与资源释放。
  • UT 测试框架与用例: tests/ut 新增基于 GoogleTest 的 op_host Tiling 测试(test_sigmoid_tiling.cpp)与 op_kernel 测试(test_sigmoid.cpp),配套 infershape_context_faker/tiling_context_faker 及对应 case executor;gen_data.py 生成 golden 数据,compare_data.py 以 MERE/MARE 阈值做精度比对,tests/ut/run.sh 统一编排执行;BuildGoogleTest.cmake 从源码构建 GoogleTest 并强制使用旧 ABI(_GLIBCXX_USE_CXX11_ABI=0)以兼容 CANN 库。
likedislike
不准确?
JeffDing
JeffDing
9月16日 评论:

/compile

likedislike
atomgit-bot
atomgit-bot
9月16日 评论:

代码审查

审查结论

发现汇总(按优先级)

  • P0:0 个
  • P1:0 个
  • P2:0 个
  • P3:6 个(均已在上述工具调用中上报)

逐文件审查结果(14 个变更文件)

  1. CMakeLists.txt(根):发现 1 个问题——第 29 行提示 bash build.sh --soc=<soc>,但 build.sh 未实现该选项(已在 build.sh 中联合上报)。
  2. build.sh:发现 1 个问题——--soc 选项缺失,与 CMakeLists 文档不一致;set -e 下错误路径均能响亮失败,rm -rf 目标为固定路径,无 root 删除风险;-u 引用的 tests/ut/run.sh 存在于工作区(非本 PR 变更)。
  3. examples/CMakeLists.txt:无问题。依赖查找与 FATAL_ERROR 守卫、链接与 rpath 配置合理。
  4. examples/run.sh:无问题。路径均加引号,set -e 覆盖 cmake/make 失败。
  5. examples/test_aclnn_sigmoid.cpp:发现 1 个问题——错误路径泄漏设备内存/stream/tensor(P3)。返回码检查完整;stride/形状计算在本示例形状下正确。
  6. op_host/CMakeLists.txt:无问题。GLOB 属 CANN 模板惯例,环境变量缺失会在 find_package 阶段响亮失败。
  7. op_host/sigmoid_def.cpp:无问题。dtype/format 列表配对一致,AddConfig 与 build.sh 默认 soc 匹配。
  8. op_host/sigmoid_infershape.cpp:发现 1 个问题——输入 shape 为空时静默成功且不设置输出 shape,另有恒真死分支(P3)。
  9. op_host/sigmoid_tiling.cpp:发现 2 个问题——BF16 分支不可达且若启用会按 FP16 位模式错误计算(P3);TYPE_SIZE/MIN_SPLIT_THRESHOLD/g_vec_1_shape/EnsureNotScalar 未引用死代码(P3)。tiling 数值计算(blockFactor/ubFactor/block_dim)经推演正确,无 32 位乘法链溢出。
  10. op_kernel/CMakeLists.txt:无问题。
  11. op_kernel/sigmoid.cpp:无问题。tiling key 0/1 与模板实例化(half/float)映射一致。
  12. op_kernel/sigmoid.h:发现 1 个问题——单 buffer 串行流水导致 GM 延迟无法隐藏(P3 性能)。GM 偏移、块划分(blockFactor×blockIdx 覆盖互不重叠且总和等于 totalNum)、尾块 currentNum、uint32 收窄(每 tile ≤~60KB 无截断风险)等经推演均正确。
  13. op_kernel/sigmoid_tiling_data.h:无问题。3 个 int64 字段在 host/kernel 两侧 sizeof 一致。
  14. op_kernel/sigmoid_tiling_key.h:无问题。宏命名/取值与 CANN 模板惯例一致,与 kernel 侧枚举值对齐。

整体风险评估

本 PR 为全新的 sigmoid 自定义算子(host tiling + device kernel + 示例 + 构建脚本),核心 tiling 数值与 kernel 数据流经逐行推演未发现 P0/P1/P2 级正确性、安全或数据一致性缺陷:块划分互斥且全覆盖、无越界/无 32 位溢出/无跨核同步缺失、输入输出 dtype 路径自洽。6 个 P3 问题均属边界健壮性、死代码与性能优化范畴(BF16 死分支、InferShape 空输入兜底、--soc 文档不一致、示例错误路径泄漏、单 buffer 串行流水、未引用声明),整体变更风险较低,可在合并前按 P3 项逐一清理。


所有发现已上报完毕。以下为本次评审总结。

评审总结

发现统计

  • P0: 0 项
  • P1: 0 项
  • P2: 2 项(infershape 空指针解引用;TLS 校验关闭的供应链风险)
  • P3: 5 项(测试断言/提前返回逻辑、workspace 校验漏检、SoC 查表静默空值、构建命令可移植性)
  • 合计 7 项

各文件评审结论

  1. tests/ut/CMakeLists.txt — no issues(路径守卫 EXISTS ../../op_api 正确,子目录均在位)
  2. tests/ut/cmake/BuildGoogleTest.cmake — 2 项:TLS_VERIFY OFF 且无哈希校验的下载供应链风险(P2);硬编码 $(MAKE) 在 Ninja 生成器下失败(P3)
  3. tests/ut/common/any_value.h — no issues(shared_ptr 内存管理正确,枚举值无冲突)
  4. tests/ut/common/infershape_case_executor.cpp — 2 项:GetOpImpl(...)->infer_shape 无空指针校验(P2);提前返回条件用 expectResult 而非实际返回值,且 GetOutputShape 未判空(P3)
  5. tests/ut/common/infershape_case_executor.h — no issues
  6. tests/ut/common/infershape_context_faker.cpp — no issues
  7. tests/ut/common/infershape_context_faker.h — no issues(两个构造函数重载无歧义)
  8. tests/ut/common/tiling_case_executor.cpp — 3 项:workspace 校验被 workspaceCount>0 跳过导致回归漏检(P3);socToUpper/socToArch 用 operator[] 查表静默产生空串(P3);tiling 提前返回条件用错变量、失败后解引用可能为空的 RawTilingData(P3)
  9. tests/ut/common/tiling_case_executor.h — no issues
  10. tests/ut/common/tiling_context_faker.cpp — no issues(DeterministicInfo 对 null 参数未判空,但本 PR 内无调用路径,未作为独立缺陷上报)

总体风险评估

本 PR 全部为新增的 UT 测试基础设施代码,不涉及生产算子逻辑。主要风险集中在两点:一是测试执行器在算子未注册/实际失败路径上存在空指针解引用,可能导致 UT 进程段错误、掩盖真实失败(P2);二是 gtest 源码下载关闭 TLS 校验构成构建期供应链风险(P2)。其余为测试断言覆盖缺口与可移植性问题(P3)。整体为中等偏低风险,建议在合入前修复 P2 两项并顺手补齐 P3 断言逻辑。


审查结论

发现汇总

共上报 5 个问题:P2 × 2,P3 × 3。

  • P2(2 个):run.sh op_api 测试块判断路径与 cd 目标不一致,set -e 下将提前中断脚本且引用的二进制不存在;test_sigmoid.cpp 的内核 UT 全程无任何断言,内核为空实现时 gtest 仍"假绿",且从不校验 GmAlloc 返回值、可能读取未初始化 GM 内存。
  • P3(3 个):run.sh 硬编码个人 CANN 路径作为默认 ASCEND_HOME_PATH 且 op_host CMake 缺乏存在性校验/回退(与 op_kernel 的处理不一致);test_sigmoid.cpp 中 FloatToHalf/FloatToBFloat16 两个未引用且语义错误的死函数;gen_data.py 中未使用的 np_type 变量。

各变更文件审查状态

文件 状态
tests/ut/common/tiling_context_faker.h 已审查,no issues(#endif 注释拼写与 guard 名不一致属纯注释噪音,不报告)
tests/ut/op_host/CMakeLists.txt 已审查,问题并入 run.sh 硬编码路径项(对 $ENV{ASCEND_HOME_PATH} 无存在性校验/回退)
tests/ut/op_host/test_op_host_main.cpp 已审查,no issues
tests/ut/op_host/test_sigmoid_tiling.cpp 已审查,no issues
tests/ut/op_kernel/CMakeLists.txt 已审查,硬编码 CANN 回退路径问题并入 run.sh 项
tests/ut/op_kernel/sigmoid_data/compare_data.py 已审查,no issues(dtype 推断、MERE/MARE 阈值逻辑与 float32 流程一致)
tests/ut/op_kernel/sigmoid_data/gen_data.py 已审查,1 项(未使用变量 np_type)
tests/ut/op_kernel/sigmoid_tiling.h 已审查,no issues
tests/ut/op_kernel/test_sigmoid.cpp 已审查,2 项(无断言/假绿、死代码函数)
tests/ut/run.sh 已审查,2 项(op_api 块路径不一致、硬编码 CANN 路径)

总体风险判断

本 PR 为算子 UT 测试配套代码(新增 1094 行,全部为测试与脚本),不涉及生产算子/host 代码变更,因此无 P0/P1 级风险。风险集中在测试有效性与脚本可靠性:kernel UT 无进程内断言会使内核回归无法被 gtest 直接捕获(依赖外部脚本),run.sh 的 op_api 块存在潜在中断点。均为可修复的测试基础设施问题,整体风险中低。

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

💬 仅评论

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

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
repo-cann/cann-outreach ✅ shaoyf, 傅涛 (2/2) ✅ 傅涛, shaoyf (2/1)

💡 Tip:

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

CLA Signature Pass

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

likedislike
JeffDingJeffDing
9月16日 关联了issue:[Feature]: [代码侦探(第七期)] 个人开发者 - challenge07_sigmoidcustom
atomgit-bot
atomgit-bot9月16日进行代码检视2
2026/CANN-Code-Detective/Challenge07-SigmoidCustom/PR_code/Developer_JeffDing/code/challenge07_sigmoidcustom/tests/ut/common/infershape_case_executor.cpp
@@ -0,0 +68,4 @@
68+ auto contextHolder = contextFaker.SetOpType(infershapeContextPara.opName_.c_str()).Build(); \
69+ /* 2. get infershape func */ \
70+ auto spaceRegistry = gert::DefaultOpImplSpaceRegistryV2::GetInstance().GetSpaceRegistry(); \
71+ auto infershapeFunc = spaceRegistry->GetOpImpl(infershapeContextPara.opName_.c_str())->infer_shape; \
atomgit-bot
atomgit-bot9月16日评论:

🟡 Medium Priority

变更行:infershape_case_executor.cpp 第 70-71 行 auto spaceRegistry = gert::DefaultOpImplSpaceRegistryV2::GetInstance().GetSpaceRegistry(); auto infershapeFunc = spaceRegistry->GetOpImpl(...)->infer_shape;。
受影响行为:spaceRegistry 本身可能为 nullptr(GetSpaceRegistry() 未检查),GetOpImpl(opName) 在算子未注册(测试用例拼错算子名、或该算子尚未注册到 registry)时也会返回 nullptr,随后 ->infer_shape 直接解引用空指针。
失败模式:与同批新增的 tiling_case_executor.cpp 第 141-147 行(对 spaceRegistry 和 GetOpImpl 结果均做了 nullptr 检查并 throw std::invalid_argument)形成明显不一致;任何未注册算子名的 infershape 测试都会使整个 UT 进程段错误崩溃,而不是单个用例干净失败,CI 中会掩盖其他用例结果。

建议:在取 infer_shape 前先检查 spaceRegistry 与 GetOpImpl 返回值是否为 nullptr,为空时抛出异常或记录 FAIL 后返回,避免空指针解引用。

likedislike
不准确?
JeffDing
JeffDing
9月22日 评论:

UT代码在CANNJunge平台无法修改

atomgit-bot
atomgit-bot9月16日进行代码检视2
2026/CANN-Code-Detective/Challenge07-SigmoidCustom/PR_code/Developer_JeffDing/code/challenge07_sigmoidcustom/tests/ut/cmake/BuildGoogleTest.cmake
@@ -0,0 +31,4 @@
31+ExternalProject_Add(
32+ third_party_gtest
33+ URL ${GTEST_URL}
34+ TLS_VERIFY OFF
atomgit-bot
atomgit-bot9月16日评论:

🟡 Medium Priority

变更行:BuildGoogleTest.cmake 第 34 行 TLS_VERIFY OFF。
受影响行为:ExternalProject_Add 从第三方 gitcode 镜像 URL 下载 googletest-1.14.0.tar.gz 源码并在本机构建、链接进 UT 可执行文件;TLS_VERIFY OFF 关闭了 HTTPS 证书校验,且没有配置 EXPECTED_HASH 校验下载产物完整性。
失败模式:中间人攻击或被篡改的镜像可在下载阶段替换 tarball,恶意代码随后被编译并链接到测试二进制中,在开发者/CI 机器上实现构建期任意代码执行(供应链投毒)。该下载发生在每次 configure 阶段,风险面持续存在。

建议:移除 TLS_VERIFY OFF,并增加 EXPECTED_HASH 校验下载源码包完整性。

likedislike
不准确?
JeffDing
JeffDing
9月22日 评论:

UT代码在CANNJunge平台无法修改

shaoyf成员
12 天前 评论:

/approve
/lgtm

likedislike
fulltower成员
12 天前 评论:

/approve
/lgtm

likedislike
CANN-robotCANN-robot成员
12 天前 添加了label:lgtmapproved
CANN-robot
CANN-robot成员
12 天前 评论:

The MR can not be merged, because of CodeReview discussion not resolved

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

likedislike
fulltower成员
12 天前 评论:

/check-pr

likedislike
CANN-robot
CANN-robot成员
12 天前 评论:

The MR can not be merged, because of CodeReview discussion not resolved

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

likedislike
Ffulltower成员
12 天前 解决了最后一个问题
fulltower成员
12 天前 评论:

/check-pr

likedislike
CANN-robotCANN-robot成员
12 天前 关闭了关联的issue
CANN-robotCANN-robot成员
12 天前 合入了pull request
JeffDingJeffDing
1 天前 修改标题为 “【代码侦探Challenge7】 个人开发者 - challenge07_sigmoidcustom”,原标题为“[代码侦探(第七期)] 个人开发者 - challenge07_sigmoidcustom”
JeffDingJeffDing
1 天前 修改标题为 “【代码侦探Challenge07】 个人开发者 - challenge07_sigmoidcustom”,原标题为“【代码侦探Challenge7】 个人开发者 - challenge07_sigmoidcustom”