Skip to content

feat(pipe): support dynamic valid shape on frontend tpop#479

Open
HecreReed wants to merge 2 commits intohw-native-sys:mainfrom
HecreReed:codex/tpop-dynshape
Open

feat(pipe): support dynamic valid shape on frontend tpop#479
HecreReed wants to merge 2 commits intohw-native-sys:mainfrom
HecreReed:codex/tpop-dynshape

Conversation

@HecreReed
Copy link
Copy Markdown
Collaborator

Summary

  • add optional (valid_row, valid_col) operands to pto.tpop_from_aic and pto.tpop_from_aiv
  • lower frontend dynamic-shape pops through pto.set_validshape before pto.tpop
  • keep the existing static tpop emitc path unchanged, but emit declare-only tiles for dynamic declare_tile-backed pops so generated C++ becomes Tile<..., -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 by FileCheck)

@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@HecreReed HecreReed marked this pull request as ready for review April 14, 2026 03:41
@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +9130 to +9132
if (op->hasAttr(kForceDynamicValidShapeAttrName)) {
declSpec.useConstructor = false;
declSpec.constructorArgs.clear();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

@reedhecre
Copy link
Copy Markdown

reedhecre commented Apr 14, 2026

Codex Review

该评论由 review 机器人自动更新。

  • PR: feat(pipe): support dynamic valid shape on frontend tpop #479 feat(pipe): support dynamic valid shape on frontend tpop
  • Author: HecreReed
  • Base/Head: main / codex/tpop-dynshape
  • Head SHA: 3c3e12938b45
  • Trigger: PR 有新提交
  • Generated At: 2026-04-14T04:26:25Z
  • Previous Head SHA: 882219315a79
  • Status: completed

Summary

PR #479 在 dynamic declare_tile 的 EmitC lowering 上引入了一个行为回归:只要后续存在任意 set_validshape 用户,就会取消默认的 full-shape valid 初始化,可能让部分路径上的 tile valid shape 变成未初始化/错误值。

Findings

  1. P2 declare_tile 的默认 valid shape 会被任意 set_validshape 用户全局地取消 lib/PTO/Transforms/PTOToEmitC.cpp:9126

在这次修改前,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 模式上。

@HecreReed
Copy link
Copy Markdown
Collaborator Author

/run a3

@reedhecre
Copy link
Copy Markdown

A3 板测失败

日志尾部


===== STAGE check-python-deps @ 2026-04-14 14:00:13 =====
/usr/bin/python3 - <<'PY'
import numpy, pybind11
print('numpy', numpy.__version__)
print('pybind11', pybind11.__version__)
PY
numpy 2.2.6
pybind11 2.13.6
===== END STAGE check-python-deps rc=0 @ 2026-04-14 14:00:13 =====

===== STAGE check-mlir-python @ 2026-04-14 14:00:13 =====
/usr/bin/python3 - <<'PY'
from mlir.ir import Context
ctx = Context()
print('mlir_python_ok')
PY
mlir_python_ok
===== END STAGE check-mlir-python rc=0 @ 2026-04-14 14:00:14 =====

===== STAGE fetch-source @ 2026-04-14 14:00:14 =====
set -euo pipefail
rm -rf /home/zhongxuan/ptoas-board-monitor/runtime/runs/20260414_140004_manual_pr479/repo
mkdir -p /home/zhongxuan/ptoas-board-monitor/runtime/runs/20260414_140004_manual_pr479/repo
cd /home/zhongxuan/ptoas-board-monitor/runtime/runs/20260414_140004_manual_pr479/repo
git init -q .
git remote add origin https://github.com/hw-native-sys/PTOAS.git
rc=0
for attempt in 1 2 3; do
  if git -c http.version=HTTP/1.1 fetch --depth 1 --no-tags origin +refs/pull/479/merge:refs/remotes/origin/pr/479/merge; then
    rc=0
  else
    rc=$?
    break
  fi
  if [[ "$rc" -eq 0 ]]; then
    git checkout -q -f refs/remotes/origin/pr/479/merge
    exit 0
  fi
  sleep $((attempt * 2))
done
if git -c http.version=HTTP/1.1 ls-remote --exit-code origin refs/pull/479/head >/dev/null 2>&1; then
  echo 'merge conflict against origin/main; board validation skipped' >&2
  exit 86
fi
exit "$rc"
fatal: unable to access 'https://github.com/hw-native-sys/PTOAS.git/': Failed to connect to github.com port 443 after 129994 ms: Couldn't connect to server
===== END STAGE fetch-source rc=128 @ 2026-04-14 14:04:35 =====

@HecreReed
Copy link
Copy Markdown
Collaborator Author

/run a3

@reedhecre
Copy link
Copy Markdown

A3 板测完成(有跳过)

  • 触发方式:manual
  • 源码提交:d69ecaf497c8
  • 结果汇总:OK 184 / FAIL 0 / SKIP 1
  • 日志:/home/zhongxuan/ptoas-board-monitor/runtime/logs/20260414_142404_manual_pr479.log
  • 结果 TSV:/home/zhongxuan/ptoas-board-monitor/runtime/logs/20260414_142404_manual_pr479.tsv
  • 手动指令:/run a3
  • 触发人:HecreReed
  • 触发评论:feat(pipe): support dynamic valid shape on frontend tpop #479 (comment)

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