fix(a2a3): revert #2523 level3 TMP; pin ptoas v0.57 - #2563
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughA2/A3 level3 ChangesA2/A3 TCI scratch behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change keeps A2/A3 tile.ci emission in the intended two-argument form to avoid the device pipeline mismatch, with no actionable merge-blocking risk remaining after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 9b1af4925c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (IsOp(call, "tile.ci") && call->args_.size() == 2) { | ||
| if (!materialize_a2a3_scratch_) return std::nullopt; | ||
| auto result_type = As<TileType>(call->GetType()); | ||
| INTERNAL_CHECK_SPAN(result_type, span) << "tile.ci result must be TileType before InitMemRef"; | ||
| // PTOAS v0.60 level3 TCI tmp width: 192 FP32 cols for 32-bit dst, 448 for 16-bit dst. | ||
| const int64_t cols = | ||
| result_type->dtype_.GetBit() == 32 ? kA2A3CiScratchColsInt32 : kA2A3CiScratchColsInt16; | ||
| return PtoScratchSpec{MakeStaticShape({1, cols}, span), DataType::FP32, "ci"}; | ||
| // Do not materialize A2/A3 level3 TCI scratch. The 3-arg (dst, start, f32 | ||
| // tmp) path is implemented with vector ops on PIPE_V but Op::TCI is still | ||
| // labeled PIPE_S, so ptoas emits wait(V→S)/set(S→V) and device outputs can | ||
| // go non-finite (pypto#2558 / pypto-lib#1072). Keep the 2-arg scalar TCI. | ||
| return std::nullopt; |
There was a problem hiding this comment.
Update the pass documentation alongside this behavior change: docs/en/dev/passes/32-init_memref.md still states at lines 10, 48, 286, and 304 that InitMemRef materializes and tests A2/A3 tile.ci scratch, and the corresponding Chinese document repeats those claims. Users and maintainers will therefore expect the allocation that this branch now deliberately suppresses; document the scalar two-argument exception in both languages.
AGENTS.md reference: AGENTS.md:L59-L60
Useful? React with 👍 / 👎.
cc11f30 to
46e0088
Compare
yanghaoran29
changed the title
8213b87 to
0e93c7a
Compare
yanghaoran29
changed the title
0e93c7a to
5273fad
Compare
yanghaoran29
changed the title
5273fad to
82a6240
Compare
当前三参数版本 tile.ci 疑似存在问题（pypto#2558 / pypto-lib#1072）。 hw-native-sys#2523 (Upgrade PTOAS to v0.60) coupled A2/A3 level-3 explicit tmp (InitMemRef compiler-owned scratch for tile.ci / narrowing cast / sort32) with ptoas v0.60. The 3-arg vector TCI+tmp path is PIPE_V while Op::TCI is labeled PIPE_S — packed prefill EP=2 nt=128 NaNs at tok127. This change rolls back that TMP coupling as one unit (per hw-native-sys#2523's own note that toolchain + codegen/scratch must move together): - toolchain/versions.env: PTOAS v0.60 → v0.57 (pre-hw-native-sys#2523 hashes) - Ascend910B RequiresLevel3TmpScratch → false (disables InitMemRef MaterializePtoLevel3ScratchMutator for ci/tcvt/sort32) - Align UT expectations to 2-arg / no compiler scratch - Skip all MX ST samples (matmul_mx / quant_mx / quantized_matmul_mx) - Skip fillpad_expand ST (tfillpad static-shape ABI needs ptoas v0.60) Does not keep the prior _ptoas_preprocess TCI 3→2 workaround. Runtime submodule left at main tip. Formal fix remains correcting vector TCI pipe labeling / ptoas handshake upstream. Verified: focused UTs for handler / InitMemRef / codegen / MemoryReuse.
1 participant
Summary
The current three-argument
tile.cipath is suspected to be broken (pypto#2558 / pypto-lib#1072).#2523 (Upgrade PTOAS to v0.60) coupled A2/A3 level-3 explicit tmp — InitMemRef synthesizing compiler-owned scratch for implicit
tile.ci, narrowingtile.cast, and sometile.sort32— with ptoas v0.60. On the FAIL stack, 24 kernels / 29 calls gaintci+ f32 scratch vs PASS (ptoas 0.57, 2-argtci). The vector TCI+tmp implementation runs onPIPE_VwhileOp::TCIis labeledPIPE_S; packed prefill EP=2 nt=128 NaNs at tok127.This PR does not
git revertall of #2523 (later main commits such as #2559 / #2372 must stay). It rolls back the TMP coupling surface only, as #2523 itself notes toolchain + scratch must move together:toolchain/versions.env: PTOAS v0.60 → v0.57 (pre-Upgrade PTOAS to v0.60 and pin runtime to 0659e29 #2523 wheel hashes)RequiresLevel3TmpScratch→false: InitMemRef no longer runsMaterializePtoLevel3ScratchMutatorfor implicit ci/tcvt/sort32 scratch on A2/A3 (PyPTO / DSA-RP planners). Explicit caller-providedtmp=is unchanged.tile.ci/ 1-arg implicit narrowing cast / 2-arg implicit sort32, and relaxed level3 static-view codegen checks while the flag is offtest_matmul_mx.py,test_quant_mx.py,test_quantized_matmul_mx.py(MX pack/layout needs ptoas v0.60)test_fillpad_expand.py(tfillpad_expandstatic-shape mismatch on v0.57)Not in this PR: the earlier
_ptoas_preprocesspost-ptoas TCI 3→2 rewrite (removed). Runtime submodule stays at current main pin. Upstream fix still needed: correct vector TCI pipe labeling / ptoas fence handshake.Bisect notes (true dumps via
PYPTO_LIB=pypto-lib; 0 dumps = false PASS)tcitmptciTest plan
test_backend_handler,TestPtoLevel3Scratch,TestLevel3StaticViewCodegen,TestMrgSortCodegen::test_static_aligned_sort32_*,test_ci_output_does_not_alias_compiler_scratchpytestmarkskiptoolchain/versions.envOutlineIncoreScopesAssignStmt var is not a Var after mutation(prefill_csa.py:182) — not caused by this TMP revert; needs separate fix before true-dump onboard sign-off