feat(pipe): support dynamic valid shape on frontend tpop#479
feat(pipe): support dynamic valid shape on frontend tpop#479HecreReed wants to merge 2 commits intohw-native-sys:mainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 882219315a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
lib/PTO/Transforms/PTOToEmitC.cpp
Outdated
| if (op->hasAttr(kForceDynamicValidShapeAttrName)) { | ||
| declSpec.useConstructor = false; | ||
| declSpec.constructorArgs.clear(); |
There was a problem hiding this comment.
Preserve valid-shape initialization for dynamic declare tiles
This branch clears constructor emission for every bind_tile with __pto.force_dynamic_validshape coming from declare_tile_memref, but PTOLowerFrontendPipeOpsPass only inserts pto.set_validshape when both pop operands are present (if (pop.getValidRow() && pop.getValidCol())). A dynamic frontend pop without explicit (valid_row, valid_col) therefore now lowers to an uninitialized Tile<..., -1, -1> declaration before TPOP, whereas the previous code initialized it via constructor fallback to full tile shape. That changes runtime behavior for dynamic-result pops that omit explicit valid-shape operands.
Useful? React with 👍 / 👎.
Codex Review该评论由 review 机器人自动更新。
SummaryPR #479 在 dynamic declare_tile 的 EmitC lowering 上引入了一个行为回归:只要后续存在任意 set_validshape 用户,就会取消默认的 full-shape valid 初始化,可能让部分路径上的 tile valid shape 变成未初始化/错误值。 Findings
在这次修改前,dynamic valid 的 declare_tile 会先 lower 成带 full-shape 构造参数的 Tile<..., -1, -1>(rows, cols),因此即使后面某些路径没有执行 set_validshape,tile 仍然有可用的默认 valid shape。现在这里仅通过“是否存在任意 SetValidShapeOp 用户”来决定把构造改成裸声明 Tile<..., -1, -1> v;,但这个判断没有检查顺序或支配关系。结果是:如果 set_validshape 是条件执行的,或者出现在另一个 use/TAssign/TPop 之后,原本依赖默认 full-shape valid 的路径会被静默改成未初始化/错误的 valid shape。这个优化需要至少验证 set_validshape 在所有 use 之前且必经,或者只限定在本 PR 新增的前端 tpop lowering 模式上。 |
|
/run a3 |
A3 板测失败
日志尾部 |
|
/run a3 |
A3 板测完成(有跳过)
|
Summary
(valid_row, valid_col)operands topto.tpop_from_aicandpto.tpop_from_aivpto.set_validshapebeforepto.tpopdeclare_tile-backed pops so generated C++ becomesTile<..., -1, -1> tile; tile.SetValidShape(...); TPOP(...)Test
cmake --build /private/tmp/pr-tpop-dynshape/build --target ptoas --parallel 8/private/tmp/pr-tpop-dynshape/build/tools/ptoas/ptoas --pto-arch=a5 test/basic/tpush_tpop_dynamic_validshape_a5.pto | FileCheck .../private/tmp/pr-tpop-dynshape/build/tools/ptoas/ptoas --pto-arch=a5 test/basic/tpush_tpop_dynamic_validshape_invalid.pto(verify error checked byFileCheck)