feat: fixpipe tpipe design#883
Open
HecreReed wants to merge 17 commits into
Open
Conversation
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Codex Review该评论由 review 机器人自动更新。
SummaryReview failed at stage Findings未生成结构化 findings，因为 review 过程提前失败。 Log Tail |
HecreReed
force-pushed
the
codex/fixpipe-tpipe-design-draft
branch
10 times, most recently
from
57383b6 to
85f7319
Compare
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85f7319d98
ℹ️ 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".
| 推荐把 fixpipe 配置挂到 `initialize_pipe`： | ||
|
|
||
| ```mlir | ||
| pto.aic_initialize_pipe { |
There was a problem hiding this comment.
In the current frontend pipe flow, each data op resolves its id against an initialize_pipe in the same function, so a Vector-side tpop_from_aic will only see the peer function's aiv_initialize_pipe, not this Cube-side init. If the fixpipe attrs are only shown/defined on pto.aic_initialize_pipe, verifier rules that check the consumer result type/layout and any consumer lowering have no local pipe-level contract unless a new cross-function propagation path is designed. Please require the peer aiv_initialize_pipe to carry the same attrs (and verify equality), or document the cross-function lookup explicitly.
Useful? React with 👍 / 👎.
HecreReed
force-pushed
the
codex/fixpipe-tpipe-design-draft
branch
from
85f7319 to
9a54ecb
Compare
Collaborator
Author
|
/run a3 rope_kv_cache |
Collaborator
Author
|
/run a5 rope_kv_cache |
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A5 板测成功
|
HecreReed
force-pushed
the
codex/fixpipe-tpipe-design-draft
branch
from
9a54ecb to
11c96f4
Compare
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A3 板测成功
|
HecreReed
force-pushed
the
codex/fixpipe-tpipe-design-draft
branch
7 times, most recently
from
88a1fe2 to
7662777
Compare
Collaborator
Author
|
/run a3 movfp_fixpipe_reuse_a3 |
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A3 板测失败
失败用例
|
A3 板测失败详情：PR #883movfp_fixpipe_reuse_a3
|
Collaborator
Author
|
/run a3 movfp_fixpipe_reuse_a3 |
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A3 板测失败
失败用例
|
A3 板测失败详情：PR #883movfp_fixpipe_reuse_a3
|
Collaborator
Author
|
/run a3 movfp_fixpipe_reuse_a3 |
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A3 板测成功
|
Collaborator
Author
|
/run a5 movfp_fixpipe_reuse |
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A5 板测成功
|
Collaborator
Author
|
/run a5 test/samples/Movfp/movfp_fixpipe_reuse-pto.pto |
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A5 板测成功
|
zhangstevenunity
left a comment
zhangstevenunity
left a comment
Collaborator
There was a problem hiding this comment.
Re-review: fixpipe tpipe design (head d0f8726f)
Focused on what changed since my last pass at 2418a9ab. The headline item is d07f6b06, which addresses the P2 latent silent-miscompile I flagged: a vector-quant scaling tile could be reused before a later reusing tpush's rematerialized SET_QUANT_VECTOR read it back.
The P2 fix looks sound. The new pto-rematerialize-fixpipe-vector-quant pass runs after PTOViewToMemref and before PlanMemory (ptoas.cpp:2895-2905), rewriting each reusing tpush's implicit binding into an explicit pto.set_quant_vector clone so memref liveness sees it. Verified:
SetQuantVectorOp::getEffectsdeclaresMemoryEffects::Readon thescaling_tileoperand (PTO.cpp:17862), and PlanMemory already treated the single set as a liveness point; replicating it before each reusingtpushgenuinely extends the scaling tile's live interval to the last reuse, so PlanMemory will not reuse its slot in the gap.- The pass is a faithful mirror of the vector arm of
rematerializeFixpipeQuantBindingsin PTOToEmitC.cpp (same per-block map, same id keying, same L2L/L2G2L-only coverage). Running both is idempotent for vector: EmitC erases and re-clones one-per-tpush, so no double emission (the A5 FileCheck still shows exactly oneSET_QUANT_VECTORperTPUSH). - Ordering is safe:
PTOLowerFrontendPipeOps(2863) lowers aic/aiv inits to L2L/L2G2L before the new pass runs, so handling only those two init types is complete. - The new
PLANFileCheck infixpipe_frontend_emitc_vector_remat_a5.ptoruns in build-and-test and confirms the three explicitset_quant_vectorops survive to after PlanMemory.
Non-blocking residuals:
1. Coverage: the A5 reuse path (the actual P2 repro) is still not numerically validated. movfp_fixpipe_reuse-pto is the reuse case (id=0 rebound at the third tpush_to_aiv with no fresh set), but remote-npu-validation is SKIPPED, vpto-sim-validation runs only PyPTO smoke (not these samples), and the manual A5 board run reported OK 0 / FAIL 0 / SKIP 0 -- zero cases executed -- while A3 reported OK 1. So the one arch where scaling tiles are planned memory (A5) never actually ran this kernel end to end. The sample is also minimal (two 1x32 scaling tiles, no buffer pressure between the set and the reusing tpush), so it would not clobber even against the pre-fix compiler. The PLAN check proves the IR transform, not clobber-prevention under pressure. A buffer-pressure A5 case (or fixing the A5 sample so the board actually runs it) would close this. As a bonus, extending scaling-tile liveness is a real memory tradeoff -- worth confirming no tight-UB A5 sample now overflows PlanMemory where it previously (mis)compiled.
2. A3 ui64 verifier relaxation (85258771) contradicts this PR's own design rules 24-25 and the ODS description. Inline.
3. Helper duplication across the two remat paths is a latent drift risk that could resurrect exactly this clobber. Inline.
Net: the P2 fix is correct and no new blocker; posting as COMMENT.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tpipesupport of fixpipe-styleTPUSHtpushNotes
pto-isaTPUSH/TPOP/FixpipeParamssemantics with PTOAS frontend pipe IRTPUSH<Pipe, TileProd, TConfig>during EmitC