Skip to content

Vf costmodel unroll support#975

Open
wang-chonghao wants to merge 1 commit into
hw-native-sys:mainfrom
wang-chonghao:vf-costmodel-submodule-ir
Open

Vf costmodel unroll support#975
wang-chonghao wants to merge 1 commit into
hw-native-sys:mainfrom
wang-chonghao:vf-costmodel-submodule-ir

Conversation

@wang-chonghao

Copy link
Copy Markdown

利用VfSim costmodel预测ptoas elementwise算子unroll展开后的时间,在IR上打上unroll attrs,供vpto后端消费

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@wang-chonghao
wang-chonghao force-pushed the vf-costmodel-submodule-ir branch from e321e92 to a2e8363 Compare July 22, 2026 15:20
@wang-chonghao
wang-chonghao force-pushed the vf-costmodel-submodule-ir branch from 5c84cd6 to e04cd55 Compare July 23, 2026 07:26
@Zhendong404

Copy link
Copy Markdown
Collaborator

Review 结论

目前不建议合并。这个 PR 的功能方向是合理的,但还存在几类需要先解决的问题:默认构建/CI、submodule 依赖治理、安装/export 链路、MLIR rewrite 契约,以及一些静默失效路径。

我本地没有完整构建该 PR;以下结论来自 PR diff、现有 lit/CMake 配置以及 pinned VfSimulator commit 的静态审查。另外当前 gh pr checks 975 显示没有 checks 运行记录。

阻塞问题

1. 新增 lit 测试在默认构建下必失败

涉及文件:

  • test/lit/tile_fusion/vfsim_unroll_gelu_poly_32x128.pto
  • test/lit/tile_fusion/vfsim_unroll_gelu_poly_96x64.pto
  • cmake/VfSimulator.cmake
  • lib/PTO/Transforms/TileFusion/PTOFusionPlan.cpp

新增测试无条件传入 --enable-vfsim-fusion-planner,但 PTO_ENABLE_VFSIM_COSTMODEL 默认是 OFF。默认构建下 planner 会 emit error 并 signalPassFailure(),因此默认 ninja check-pto 会失败。

目前 lit 配置没有对应 feature,测试也没有 REQUIRES

建议:

  • CMake 根据 PTO_ENABLE_VFSIM_COSTMODEL 注入 lit feature;
  • 两个测试添加类似 // REQUIRES: vfsim-costmodel
  • 最好补一个默认构建下的负向测试,确认该 flag 会给出明确错误。

2. 新增 submodule 指向个人 fork 的 feature 分支

涉及文件:

  • .gitmodules
  • 3rdparty/VfSimulator

当前 URL 是 git@github.com:wang-chonghao/VfSimulator.git,作为组织项目的长期构建依赖存在治理和可用性风险;SSH URL 对匿名 HTTPS clone、CI checkout、发布环境也不友好。当前 pinned commit 是 feature branch tip,不是稳定 tag。

建议:

  • 将 VfSimulator 迁入组织仓库;
  • pin 到 tag 或明确 release commit;
  • URL 改为 HTTPS 或相对 URL;
  • 如果 CI 要覆盖该功能,明确初始化 submodule 并保证权限模型可用。

3. PTO_ENABLE_VFSIM_COSTMODEL=ON 时 install/export 链路不完整

PTOTransforms 会安装并导出,但 vfsim 静态库没有对应 install/export 规则;当前 $<BUILD_INTERFACE:vfsim::...> 链接方式在 build tree 内可以工作,但安装后下游通过 find_package(PTOAS) 链接 PTOAS::PTOTransforms 时可能缺 vfsim 符号。

建议三选一:

  • 将 vfsim 静态库一起 install/export;
  • 将 vfsim 做成 OBJECT library 并入 PTOTransforms
  • 明确禁止 VFSIM 构建用于安装导出。

4. VfSimulator 运行时依赖编译机绝对路径,跨机后可能静默失效

VfSimulator native CMake 会把 VFSIM_SOURCE_ROOT 编译进二进制,运行时再从该路径加载 ParamDB/config。安装到其他机器、CI artifact 或 wheel 环境后路径可能不存在;当前失败路径返回 success,planner 可能不写回 unroll 属性且没有有效诊断。

建议:

  • ParamDB 路径改为 CLI option / 环境变量 / 安装后相对定位;
  • 加载失败至少 warning/error,不能静默 success。

核心 pass 正确性风险

5. unroll pattern 绕开 PatternRewriter 修改 IR

涉及文件:

  • lib/PTO/Transforms/TileFusion/PTOUnrollAfterLoopFusion.cpp

当前在 greedy rewrite pattern 内直接调用 mlir::loopUnrollByFactor。该 API 会使用内部 IRRewriter 原地修改 loop、clone body,某些路径还会 erase scf.for;这些操作没有通过外层 PatternRewriter 通知 driver/listener。region->setAttr(...) 也没有通过 modifyOpInPlace

release 构建下可能碰巧工作,但开启 MLIR expensive pattern API checks 的构建可能 fatal error;这也是对 greedy driver 实现细节的隐式依赖。

建议改成普通 walk:先收集 leaf scf.for,再逐个调用 loopUnrollByFactor,不要用 greedy rewrite pattern 承载结构性 unroll。

6. planner 之后无条件保留 PreFusionAnalysis

PreFusionAnalysis 缓存了包含 Operation * 的 DFG。PR 假设外部 planner 只原地写属性,但这个约束当前主要靠文档,代码没有校验。外部 submodule 后续如果增删或替换 op,RegionGen 可能使用悬垂指针。

建议 planner 调用后保守地不 preserve,或在 debug 构建中校验 IR 结构未变化;同时把“planner 只能写属性,不能修改 IR 结构”明确为硬契约。

7. clearPlanningAttrs 未清理新增 unroll 属性

FusionPlan 重复运行时,上一轮残留的 pto.fusion.row_unroll_factor / pto.fusion.col_unroll_factor 可能继续进入 RegionGen 并被 unroll pass 消费。

建议在 clearPlanningAttrs 中同步删除这两个属性。

8. getCommonSpanI64Attr 对部分成员缺属性的情况静默放行

当前只检查持有属性的成员是否一致,缺属性成员直接跳过。planner 如果只给组内部分 tileop 写回因子,RegionGen 会把片面结果提升到整个 pto.fusion_region,没有诊断。

建议“部分有、部分无”时直接报错,或者明确记录这是有意语义;提升前也建议校验 factor >= 1

9. row/col 因子可能错配到错误维度的 leaf loop

当前 “col preferred, else row” 的逻辑在某些场景下可能用 row factor 展开仍然存在的 col leaf loop。IR 语义未必错,但 cost model 的维度意图会被静默错配。建议只有确认 leaf loop 属于 row 层时才使用 row factor,否则保守跳过。

CLI / 行为一致性

10. --enable-vfsim-fusion-planner 在不满足条件时静默忽略

在 A2/A3、未开启 op fusion、或特定 fusion level 下,用户传该 flag 不会生效,也没有错误。相比之下,--enable-unroll-after-loop-fusion 做了硬校验,两个相关 flag 行为不一致。

建议不满足条件时报错,至少 warning,并在 CLI help 中说明依赖 A5 + op fusion。

11. --enable-unroll-after-loop-fusion 校验仍不完整

当前校验只看 arch == "a5"opFusionEnabled,但 pass 只插入 VPTO backend 路径。某些 emit 模式下 flag 通过校验却不会执行。不开 planner 时该 pass 也可能是静默 no-op。

建议 help 中明确该 pass 消费 pto.fusion.*_unroll_factor,通常需要配合产生这些属性的 planner 使用。

仓库卫生 / 全局副作用

12. results/vfsim_region_gen_ir/...mlir 不应提交

这看起来是调试 dump:仓库此前没有顶层 results/ 约定,内容也已过期(使用了当前实现不存在的 pto.fusion.unroll 属性,且 IR 结尾形态不完整)。建议从 PR 删除。

13. _bootstrap.py 全局设置 RTLD_GLOBAL 副作用过大

ptodsl/ptodsl/_bootstrap.py 在 import 时执行 sys.setdlopenflags(os.RTLD_NOW | os.RTLD_GLOBAL),会影响进程内之后所有 native module 的 dlopen,存在符号互插和冲突风险;且该改动与是否启用 VFSIM 无关。

建议从本 PR 删除;如确有需要,拆独立 PR,并只在具体加载点临时设置、加载后恢复。

测试 / 文档

14. 测试硬编码 cost model 结果,后续升级 submodule 会很脆

测试固定期待 row_unroll_factor = 8col_unroll_factor = 2 等具体 planner 决策值。以后 cost model 合理变化也会导致测试失败。建议至少在测试头注明这些期望值绑定当前 VfSimulator commit;更好的方式是把“planner 输出值”和“unroll pass 消费属性”拆成两层测试。

15. 设计文档与代码已有不一致

docs/designs/vf_costmodel_external_planner_protocol.md 中部分 ptoas.cpp 行号已过期;没有介绍新增的 --enable-unroll-after-loop-fusionpto-unroll-after-loop-fusion;也没有说明 unroll pass 消费后会把 factor 重置为 1

建议文档避免写易过期行号,改为文件 + 符号名,并补充 unroll pass 的输入、消费语义和属性复位行为。

其他较小问题

  • cmake/VfSimulator.cmakeFORCE 覆盖用户显式传入的 cache 变量,不够友好。
  • PTO_ENABLE_VFSIM_NATIVE define 当前 C++ 代码中无人使用。
  • $<BUILD_INTERFACE:vfsim::ir_planner> 这类生成表达式包装没有必要,直接链接 target 更清楚。
  • 新 pass 使用 mlir::loopUnrollByFactor,建议显式链接 MLIRSCFUtils,避免静态 MLIR 构建断链。

建议合并前至少完成

  1. 给新增 lit 测试增加 feature 门控;
  2. 删除 results/ 调试 dump;
  3. 删除或收窄 _bootstrap.pyRTLD_GLOBAL
  4. 重写 unroll pass,避免在 greedy pattern 内直接调用结构性 unroll API;
  5. 补齐 VFSIM=ON 时 install/export 链路;
  6. 将 VfSimulator 迁到组织仓库并使用稳定 tag / HTTPS URL;
  7. 明确 planner 属性写回契约,并处理陈旧 unroll 属性清理。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants