Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughLegalizeTileCast adds backend-aware cast-chain legalization, exposes it through C++ and Python, inserts it into optimization pipelines, and adds focused tests. Pass documentation and cross-references are updated for the new pipeline numbering. ChangesLegalizeTileCast
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PassManager
participant LegalizeTileCast
participant BackendHandler
participant IR
PassManager->>LegalizeTileCast: run tile.cast legalization
LegalizeTileCast->>BackendHandler: resolve backend cast profile
LegalizeTileCast->>LegalizeTileCast: find safe shortest native chain
LegalizeTileCast->>IR: replace unsupported cast with native assignments
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff61142f7f
ℹ️ 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".
8966190 to
1804057
Compare
d0aac23 to
6a362ad
Compare
Conversions this PR newly makes compilableEach pair below previously lowered to a single
A5 (Ascend950) — 84 pairs
A2A3 (Ascend910B) — 29 pairs
Example chains (A5): Nothing that already worked changedThe pass only rewrites pairs the arch table marks non-native, so natively-supported
In particular Pairs that remain unreachable are now rejected in pypto with src/dst/arch named, Measured by running the pass over all ordered pairs of |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a362ade38
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
tests/ut/ir/transforms/test_legalize_tile_cast.py (1)
210-212: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winWeak exception-message assertion.
"cast" in str(excinfo.value).lower()is satisfied by almost any exception mentioning casting (including the pass's own name, "LegalizeTileCast"), so this wouldn't actually catch a regression where the narrowing-rejection path breaks and some unrelated exception is raised instead. Consider asserting on a more specific substring from the actualCHECK_SPANmessage (e.g."no native cast path").♻️ Suggested tightening
with pytest.raises(Exception) as excinfo: _run(Before, BackendType.Ascend950) - assert "cast" in str(excinfo.value).lower() + assert "no native cast path" in str(excinfo.value).lower()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ut/ir/transforms/test_legalize_tile_cast.py` around lines 210 - 212, Strengthen the exception assertion in the test around `_run(Before, BackendType.Ascend950)` by checking for the specific narrowing-rejection message from the `CHECK_SPAN` path, such as “no native cast path,” instead of the generic “cast” substring. Keep the `pytest.raises(Exception)` expectation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/en/dev/passes/24-normalize_return_order.md`:
- Around line 298-300: Update the pipeline pass ordering documentation near
NormalizeReturnOrder to list SkewCrossCorePipeline as the intervening pass and
identify LowerPipelineLoops as the pass that immediately follows it, while
preserving the existing DeriveCallDirections entry.
In `@docs/zh-cn/dev/codegen/01-orchestration_codegen.md`:
- Around line 110-114: 更新该文档段落及其后续示例,使示例明确反映 MaterializeRuntimeScopes 会为每个
for/if 体插入显式 RuntimeScopeStmt,并由 codegen 生成嵌套 PTO2_SCOPE。确保普通 for 示例不再暗示缺少嵌套
scope,并保持 manual scope 的降级说明一致。
In `@docs/zh-cn/dev/passes/10-convert_tensor_to_tile_ops.md`:
- Line 198: Correct the pass numbering in the paragraph describing
LowerAutoVectorSplit and ExpandMixedKernel: change the LowerAutoVectorSplit
reference to pass 19 and the ExpandMixedKernel reference to pass 20, matching
their linked or renamed documentation pages while leaving the described
transformation unchanged.
In `@docs/zh-cn/dev/passes/15-auto_tile_matmul_l0.md`:
- Line 13: Synchronize the documented pass ordering: in
docs/zh-cn/dev/passes/15-auto_tile_matmul_l0.md:13, state that AutoTileMatmulL0
follows LegalizeTileCast; in docs/en/dev/passes/17-infer_tile_memory_space.md:18
and docs/zh-cn/dev/passes/17-infer_tile_memory_space.md:18, remove the claim
that InferTileMemorySpace immediately follows FlattenTileNdTo2D and reflect the
intervening LegalizeTileCast, AutoTileMatmulL0, and CanonicalizeTileSlice
passes.
In `@docs/zh-cn/dev/passes/20-expand_mixed_kernel.md`:
- Line 56: Synchronize pass-number references in the specified documentation:
update LowerAutoVectorSplit from pass 18 to pass 19 in
docs/zh-cn/dev/passes/20-expand_mixed_kernel.md lines 56 and 172-173,
docs/en/dev/passes/20-expand_mixed_kernel.md lines 79-82 and 207-210, and update
both LowerAutoVectorSplit to 19 and ExpandMixedKernel to 20 in
docs/en/dev/passes/10-convert_tensor_to_tile_ops.md line 207.
In `@docs/zh-cn/dev/passes/33-fold_no_op_reshape.md`:
- Around line 41-42: Synchronize the pipeline-order descriptions by removing or
correcting the contradictory hard-coded ordinals: update FoldNoOpReshape in
docs/zh-cn/dev/passes/33-fold_no_op_reshape.md lines 41-42 and
FuseCreateAssembleToSlice in
docs/zh-cn/dev/passes/34-fuse_create_assemble_to_slice.md line 16. Prefer the
current pipeline indices, or omit numbers and describe the neighboring pass
order using AllocateMemoryAddr and FuseCreateAssembleToSlice.
In `@docs/zh-cn/dev/passes/37-expand_manual_phase_fence.md`:
- Around line 34-35: Update the pipeline diagram near ExpandManualPhaseFence to
include the subsequent MaterializeDistTensorCtx and MaterializeRuntimeScopes
passes after Simplify, and remove the “最终” label from Simplify if it is no
longer the terminal pass. Keep the documented ordering consistent with the
corresponding pass documentation.
---
Nitpick comments:
In `@tests/ut/ir/transforms/test_legalize_tile_cast.py`:
- Around line 210-212: Strengthen the exception assertion in the test around
`_run(Before, BackendType.Ascend950)` by checking for the specific
narrowing-rejection message from the `CHECK_SPAN` path, such as “no native cast
path,” instead of the generic “cast” substring. Keep the
`pytest.raises(Exception)` expectation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: da74cb6c-c812-43ae-ade5-8c38ba1c47ca
📒 Files selected for processing (90)
.claude/rules/pass-doc-ordering.mdCMakeLists.txtdocs/en/dev/codegen/00-pto_codegen.mddocs/en/dev/codegen/01-orchestration_codegen.mddocs/en/dev/distributed_ops.mddocs/en/dev/ir/02-types.mddocs/en/dev/language/00-python_syntax.mddocs/en/dev/passes/00-pass_manager.mddocs/en/dev/passes/08-outline_incore_scopes.mddocs/en/dev/passes/10-convert_tensor_to_tile_ops.mddocs/en/dev/passes/14-legalize_tile_cast.mddocs/en/dev/passes/15-auto_tile_matmul_l0.mddocs/en/dev/passes/16-canonicalize_tile_slice.mddocs/en/dev/passes/17-infer_tile_memory_space.mddocs/en/dev/passes/18-resolve_backend_op_layouts.mddocs/en/dev/passes/19-lower_auto_vector_split.mddocs/en/dev/passes/20-expand_mixed_kernel.mddocs/en/dev/passes/21-inject_gm_pipe_buffer.mddocs/en/dev/passes/22-split_vector_kernel.mddocs/en/dev/passes/23-stamp_tfree_split.mddocs/en/dev/passes/24-normalize_return_order.mddocs/en/dev/passes/25-skew_cross_core_pipeline.mddocs/en/dev/passes/26-lower_pipeline_loops.mddocs/en/dev/passes/27-canonicalize_io_order.mddocs/en/dev/passes/28-materialize_tensor_strides.mddocs/en/dev/passes/29-init_memref.mddocs/en/dev/passes/30-materialize_semantic_aliases.mddocs/en/dev/passes/31-memory_reuse.mddocs/en/dev/passes/32-allocate_memory_addr.mddocs/en/dev/passes/33-fold_no_op_reshape.mddocs/en/dev/passes/34-fuse_create_assemble_to_slice.mddocs/en/dev/passes/35-derive_call_directions.mddocs/en/dev/passes/36-auto_derive_task_dependencies.mddocs/en/dev/passes/37-expand_manual_phase_fence.mddocs/en/dev/passes/38-synthesize_allreduce_signals.mddocs/en/dev/passes/39-materialize_comm_domain_scopes.mddocs/en/dev/passes/40-lower_host_tensor_collectives.mddocs/en/dev/passes/41-materialize_dist_tensor_ctx.mddocs/en/dev/passes/42-materialize_runtime_scopes.mddocs/en/dev/passes/43-classify_iter_arg_carry.mddocs/en/user/01-language_guide.mddocs/zh-cn/dev/codegen/00-pto_codegen.mddocs/zh-cn/dev/codegen/01-orchestration_codegen.mddocs/zh-cn/dev/distributed_ops.mddocs/zh-cn/dev/ir/02-types.mddocs/zh-cn/dev/language/00-python_syntax.mddocs/zh-cn/dev/passes/00-pass_manager.mddocs/zh-cn/dev/passes/08-outline_incore_scopes.mddocs/zh-cn/dev/passes/10-convert_tensor_to_tile_ops.mddocs/zh-cn/dev/passes/14-legalize_tile_cast.mddocs/zh-cn/dev/passes/15-auto_tile_matmul_l0.mddocs/zh-cn/dev/passes/16-canonicalize_tile_slice.mddocs/zh-cn/dev/passes/17-infer_tile_memory_space.mddocs/zh-cn/dev/passes/18-resolve_backend_op_layouts.mddocs/zh-cn/dev/passes/19-lower_auto_vector_split.mddocs/zh-cn/dev/passes/20-expand_mixed_kernel.mddocs/zh-cn/dev/passes/21-inject_gm_pipe_buffer.mddocs/zh-cn/dev/passes/22-split_vector_kernel.mddocs/zh-cn/dev/passes/23-stamp_tfree_split.mddocs/zh-cn/dev/passes/24-normalize_return_order.mddocs/zh-cn/dev/passes/25-skew_cross_core_pipeline.mddocs/zh-cn/dev/passes/26-lower_pipeline_loops.mddocs/zh-cn/dev/passes/27-canonicalize_io_order.mddocs/zh-cn/dev/passes/28-materialize_tensor_strides.mddocs/zh-cn/dev/passes/29-init_memref.mddocs/zh-cn/dev/passes/30-materialize_semantic_aliases.mddocs/zh-cn/dev/passes/31-memory_reuse.mddocs/zh-cn/dev/passes/32-allocate_memory_addr.mddocs/zh-cn/dev/passes/33-fold_no_op_reshape.mddocs/zh-cn/dev/passes/34-fuse_create_assemble_to_slice.mddocs/zh-cn/dev/passes/35-derive_call_directions.mddocs/zh-cn/dev/passes/36-auto_derive_task_dependencies.mddocs/zh-cn/dev/passes/37-expand_manual_phase_fence.mddocs/zh-cn/dev/passes/38-synthesize_allreduce_signals.mddocs/zh-cn/dev/passes/39-materialize_comm_domain_scopes.mddocs/zh-cn/dev/passes/40-lower_host_tensor_collectives.mddocs/zh-cn/dev/passes/41-materialize_dist_tensor_ctx.mddocs/zh-cn/dev/passes/42-materialize_runtime_scopes.mddocs/zh-cn/dev/passes/43-classify_iter_arg_carry.mddocs/zh-cn/user/01-language_guide.mdinclude/pypto/ir/transforms/pass_properties.hinclude/pypto/ir/transforms/passes.hinclude/pypto/ir/transforms/utils/attrs.hpython/bindings/modules/passes.cpppython/pypto/ir/pass_manager.pypython/pypto/pypto_core/passes.pyisrc/ir/transforms/legalize_tile_cast_pass.cpptests/ut/ir/transforms/test_legalize_tile_cast.pytests/ut/ir/transforms/test_materialize_tensor_strides_pass.pytests/ut/ir/transforms/test_pass_manager.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 7
🧹 Nitpick comments (1)
tests/ut/ir/transforms/test_legalize_tile_cast.py (1)
210-212: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winWeak exception-message assertion.
"cast" in str(excinfo.value).lower()is satisfied by almost any exception mentioning casting (including the pass's own name, "LegalizeTileCast"), so this wouldn't actually catch a regression where the narrowing-rejection path breaks and some unrelated exception is raised instead. Consider asserting on a more specific substring from the actualCHECK_SPANmessage (e.g."no native cast path").♻️ Suggested tightening
with pytest.raises(Exception) as excinfo: _run(Before, BackendType.Ascend950) - assert "cast" in str(excinfo.value).lower() + assert "no native cast path" in str(excinfo.value).lower()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ut/ir/transforms/test_legalize_tile_cast.py` around lines 210 - 212, Strengthen the exception assertion in the test around `_run(Before, BackendType.Ascend950)` by checking for the specific narrowing-rejection message from the `CHECK_SPAN` path, such as “no native cast path,” instead of the generic “cast” substring. Keep the `pytest.raises(Exception)` expectation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/en/dev/passes/24-normalize_return_order.md`:
- Around line 298-300: Update the pipeline pass ordering documentation near
NormalizeReturnOrder to list SkewCrossCorePipeline as the intervening pass and
identify LowerPipelineLoops as the pass that immediately follows it, while
preserving the existing DeriveCallDirections entry.
In `@docs/zh-cn/dev/codegen/01-orchestration_codegen.md`:
- Around line 110-114: 更新该文档段落及其后续示例,使示例明确反映 MaterializeRuntimeScopes 会为每个
for/if 体插入显式 RuntimeScopeStmt,并由 codegen 生成嵌套 PTO2_SCOPE。确保普通 for 示例不再暗示缺少嵌套
scope,并保持 manual scope 的降级说明一致。
In `@docs/zh-cn/dev/passes/10-convert_tensor_to_tile_ops.md`:
- Line 198: Correct the pass numbering in the paragraph describing
LowerAutoVectorSplit and ExpandMixedKernel: change the LowerAutoVectorSplit
reference to pass 19 and the ExpandMixedKernel reference to pass 20, matching
their linked or renamed documentation pages while leaving the described
transformation unchanged.
In `@docs/zh-cn/dev/passes/15-auto_tile_matmul_l0.md`:
- Line 13: Synchronize the documented pass ordering: in
docs/zh-cn/dev/passes/15-auto_tile_matmul_l0.md:13, state that AutoTileMatmulL0
follows LegalizeTileCast; in docs/en/dev/passes/17-infer_tile_memory_space.md:18
and docs/zh-cn/dev/passes/17-infer_tile_memory_space.md:18, remove the claim
that InferTileMemorySpace immediately follows FlattenTileNdTo2D and reflect the
intervening LegalizeTileCast, AutoTileMatmulL0, and CanonicalizeTileSlice
passes.
In `@docs/zh-cn/dev/passes/20-expand_mixed_kernel.md`:
- Line 56: Synchronize pass-number references in the specified documentation:
update LowerAutoVectorSplit from pass 18 to pass 19 in
docs/zh-cn/dev/passes/20-expand_mixed_kernel.md lines 56 and 172-173,
docs/en/dev/passes/20-expand_mixed_kernel.md lines 79-82 and 207-210, and update
both LowerAutoVectorSplit to 19 and ExpandMixedKernel to 20 in
docs/en/dev/passes/10-convert_tensor_to_tile_ops.md line 207.
In `@docs/zh-cn/dev/passes/33-fold_no_op_reshape.md`:
- Around line 41-42: Synchronize the pipeline-order descriptions by removing or
correcting the contradictory hard-coded ordinals: update FoldNoOpReshape in
docs/zh-cn/dev/passes/33-fold_no_op_reshape.md lines 41-42 and
FuseCreateAssembleToSlice in
docs/zh-cn/dev/passes/34-fuse_create_assemble_to_slice.md line 16. Prefer the
current pipeline indices, or omit numbers and describe the neighboring pass
order using AllocateMemoryAddr and FuseCreateAssembleToSlice.
In `@docs/zh-cn/dev/passes/37-expand_manual_phase_fence.md`:
- Around line 34-35: Update the pipeline diagram near ExpandManualPhaseFence to
include the subsequent MaterializeDistTensorCtx and MaterializeRuntimeScopes
passes after Simplify, and remove the “最终” label from Simplify if it is no
longer the terminal pass. Keep the documented ordering consistent with the
corresponding pass documentation.
---
Nitpick comments:
In `@tests/ut/ir/transforms/test_legalize_tile_cast.py`:
- Around line 210-212: Strengthen the exception assertion in the test around
`_run(Before, BackendType.Ascend950)` by checking for the specific
narrowing-rejection message from the `CHECK_SPAN` path, such as “no native cast
path,” instead of the generic “cast” substring. Keep the
`pytest.raises(Exception)` expectation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: da74cb6c-c812-43ae-ade5-8c38ba1c47ca
📒 Files selected for processing (90)
.claude/rules/pass-doc-ordering.mdCMakeLists.txtdocs/en/dev/codegen/00-pto_codegen.mddocs/en/dev/codegen/01-orchestration_codegen.mddocs/en/dev/distributed_ops.mddocs/en/dev/ir/02-types.mddocs/en/dev/language/00-python_syntax.mddocs/en/dev/passes/00-pass_manager.mddocs/en/dev/passes/08-outline_incore_scopes.mddocs/en/dev/passes/10-convert_tensor_to_tile_ops.mddocs/en/dev/passes/14-legalize_tile_cast.mddocs/en/dev/passes/15-auto_tile_matmul_l0.mddocs/en/dev/passes/16-canonicalize_tile_slice.mddocs/en/dev/passes/17-infer_tile_memory_space.mddocs/en/dev/passes/18-resolve_backend_op_layouts.mddocs/en/dev/passes/19-lower_auto_vector_split.mddocs/en/dev/passes/20-expand_mixed_kernel.mddocs/en/dev/passes/21-inject_gm_pipe_buffer.mddocs/en/dev/passes/22-split_vector_kernel.mddocs/en/dev/passes/23-stamp_tfree_split.mddocs/en/dev/passes/24-normalize_return_order.mddocs/en/dev/passes/25-skew_cross_core_pipeline.mddocs/en/dev/passes/26-lower_pipeline_loops.mddocs/en/dev/passes/27-canonicalize_io_order.mddocs/en/dev/passes/28-materialize_tensor_strides.mddocs/en/dev/passes/29-init_memref.mddocs/en/dev/passes/30-materialize_semantic_aliases.mddocs/en/dev/passes/31-memory_reuse.mddocs/en/dev/passes/32-allocate_memory_addr.mddocs/en/dev/passes/33-fold_no_op_reshape.mddocs/en/dev/passes/34-fuse_create_assemble_to_slice.mddocs/en/dev/passes/35-derive_call_directions.mddocs/en/dev/passes/36-auto_derive_task_dependencies.mddocs/en/dev/passes/37-expand_manual_phase_fence.mddocs/en/dev/passes/38-synthesize_allreduce_signals.mddocs/en/dev/passes/39-materialize_comm_domain_scopes.mddocs/en/dev/passes/40-lower_host_tensor_collectives.mddocs/en/dev/passes/41-materialize_dist_tensor_ctx.mddocs/en/dev/passes/42-materialize_runtime_scopes.mddocs/en/dev/passes/43-classify_iter_arg_carry.mddocs/en/user/01-language_guide.mddocs/zh-cn/dev/codegen/00-pto_codegen.mddocs/zh-cn/dev/codegen/01-orchestration_codegen.mddocs/zh-cn/dev/distributed_ops.mddocs/zh-cn/dev/ir/02-types.mddocs/zh-cn/dev/language/00-python_syntax.mddocs/zh-cn/dev/passes/00-pass_manager.mddocs/zh-cn/dev/passes/08-outline_incore_scopes.mddocs/zh-cn/dev/passes/10-convert_tensor_to_tile_ops.mddocs/zh-cn/dev/passes/14-legalize_tile_cast.mddocs/zh-cn/dev/passes/15-auto_tile_matmul_l0.mddocs/zh-cn/dev/passes/16-canonicalize_tile_slice.mddocs/zh-cn/dev/passes/17-infer_tile_memory_space.mddocs/zh-cn/dev/passes/18-resolve_backend_op_layouts.mddocs/zh-cn/dev/passes/19-lower_auto_vector_split.mddocs/zh-cn/dev/passes/20-expand_mixed_kernel.mddocs/zh-cn/dev/passes/21-inject_gm_pipe_buffer.mddocs/zh-cn/dev/passes/22-split_vector_kernel.mddocs/zh-cn/dev/passes/23-stamp_tfree_split.mddocs/zh-cn/dev/passes/24-normalize_return_order.mddocs/zh-cn/dev/passes/25-skew_cross_core_pipeline.mddocs/zh-cn/dev/passes/26-lower_pipeline_loops.mddocs/zh-cn/dev/passes/27-canonicalize_io_order.mddocs/zh-cn/dev/passes/28-materialize_tensor_strides.mddocs/zh-cn/dev/passes/29-init_memref.mddocs/zh-cn/dev/passes/30-materialize_semantic_aliases.mddocs/zh-cn/dev/passes/31-memory_reuse.mddocs/zh-cn/dev/passes/32-allocate_memory_addr.mddocs/zh-cn/dev/passes/33-fold_no_op_reshape.mddocs/zh-cn/dev/passes/34-fuse_create_assemble_to_slice.mddocs/zh-cn/dev/passes/35-derive_call_directions.mddocs/zh-cn/dev/passes/36-auto_derive_task_dependencies.mddocs/zh-cn/dev/passes/37-expand_manual_phase_fence.mddocs/zh-cn/dev/passes/38-synthesize_allreduce_signals.mddocs/zh-cn/dev/passes/39-materialize_comm_domain_scopes.mddocs/zh-cn/dev/passes/40-lower_host_tensor_collectives.mddocs/zh-cn/dev/passes/41-materialize_dist_tensor_ctx.mddocs/zh-cn/dev/passes/42-materialize_runtime_scopes.mddocs/zh-cn/dev/passes/43-classify_iter_arg_carry.mddocs/zh-cn/user/01-language_guide.mdinclude/pypto/ir/transforms/pass_properties.hinclude/pypto/ir/transforms/passes.hinclude/pypto/ir/transforms/utils/attrs.hpython/bindings/modules/passes.cpppython/pypto/ir/pass_manager.pypython/pypto/pypto_core/passes.pyisrc/ir/transforms/legalize_tile_cast_pass.cpptests/ut/ir/transforms/test_legalize_tile_cast.pytests/ut/ir/transforms/test_materialize_tensor_strides_pass.pytests/ut/ir/transforms/test_pass_manager.py
🛑 Comments failed to post (2)
docs/zh-cn/dev/passes/33-fold_no_op_reshape.md (1)
41-42: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Synchronize the hard-coded Default pipeline ordinals.
The two adjacent pass pages assign contradictory numbers after renumbering:
docs/zh-cn/dev/passes/33-fold_no_op_reshape.md#L41-L42: replace or correct theFoldNoOpReshapepass-29 claim.docs/zh-cn/dev/passes/34-fuse_create_assemble_to_slice.md#L16-L16: replace or correct theFuseCreateAssembleToSlicepass-27 claim.Prefer the current pipeline index or omit ordinal numbers and link the neighboring stages.
📍 Affects 2 files
docs/zh-cn/dev/passes/33-fold_no_op_reshape.md#L41-L42(this comment)docs/zh-cn/dev/passes/34-fuse_create_assemble_to_slice.md#L16-L16🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/zh-cn/dev/passes/33-fold_no_op_reshape.md` around lines 41 - 42, Synchronize the pipeline-order descriptions by removing or correcting the contradictory hard-coded ordinals: update FoldNoOpReshape in docs/zh-cn/dev/passes/33-fold_no_op_reshape.md lines 41-42 and FuseCreateAssembleToSlice in docs/zh-cn/dev/passes/34-fuse_create_assemble_to_slice.md line 16. Prefer the current pipeline indices, or omit numbers and describe the neighboring pass order using AllocateMemoryAddr and FuseCreateAssembleToSlice.docs/zh-cn/dev/passes/37-expand_manual_phase_fence.md (1)
34-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include the post-Expand passes in the pipeline diagram.
Lines [34-35] stop at
Simplify(最终), but the documented pipeline continues throughMaterializeDistTensorCtxandMaterializeRuntimeScopes. This contradictsdocs/zh-cn/dev/passes/41-materialize_dist_tensor_ctx.mdanddocs/zh-cn/dev/passes/42-materialize_runtime_scopes.md; either extend the diagram or remove the “final” label.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/zh-cn/dev/passes/37-expand_manual_phase_fence.md` around lines 34 - 35, Update the pipeline diagram near ExpandManualPhaseFence to include the subsequent MaterializeDistTensorCtx and MaterializeRuntimeScopes passes after Simplify, and remove the “最终” label from Simplify if it is no longer the terminal pass. Keep the documented ordering consistent with the corresponding pass documentation.
daa9bd5 to
2c8ea49
Compare
49cb757 to
f953ce9
Compare
pto.tcvt only supports a profile-dependent subset of (src, dst) dtype
pairs, and the subset differs per architecture. A5 (Ascend950) has no
native INT32 -> FP16 conversion, while A2/A3 provides it as a deq
instruction. A DSL-level pl.cast(x_i32, pl.FP16) therefore compiles fine
on A2/A3 but fails the kernel C++ compile on A5 with
error: no matching function for call to 'castData_2D_NoPostUpdate'
LegalizeTileCast rewrites each non-native tile.cast into the shortest
chain of native casts, found by BFS over a per-arch ISA adjacency table
(A5 INT32 -> FP16 becomes INT32 -> FP32 -> FP16). Among equal-length
paths it prefers "same byte-width -> float, then adjust width". Casts
that are already native, including FIXPIPE-foldable FP32 -> BF16/FP16
with mode=rint, are left untouched. It runs after FlattenTileNdTo2D so
casts inserted by that pass are legalized too, and before
AutoTileMatmulL0.
This unblocks the DeepSeek V4-Pro INT8 quantization chain
(fp32 -> i32(rint) -> fp16(round) -> i8(trunc)) on A5, which is used at
14 sites across the model and previously could not be compiled at all.
Shortest is not sufficient on its own: an intermediate that cannot hold
what the destination can would silently drop values a direct conversion
would have kept. A5 has no native UINT32 -> FP32, and the shortest
routes all pass through INT16 / UINT16 / UINT8, so 40000 -- exactly
representable in FP32 -- would come back as garbage. Intermediates that
provably narrow relative to the destination are therefore excluded from
the search, and a pair with no faithful chain is reported as an error
rather than lowered to a lossy one. The check only rejects what it can
prove, so an unfamiliar dtype stays admissible instead of turning a
working lowering into a hard failure.
The native-conversion table is a backend fact, so it lives on
BackendHandler as GetTcvtAdjacency() and each handler owns its own --
per pass-context-config.md, passes never branch on the backend. The pass
resolves the handler from the PassContext, falling back to the global
BackendConfig, and holds no architecture knowledge of its own: a new
backend ships a table and this pass is unchanged. Previously it carried a
private CastArch enum recovered from GetPtoTargetArch() string equality,
which silently gave any unrecognised arch the A2A3 table -- and A2A3 has
edges A5 lacks, so a third backend would have seen casts wrongly declared
native and failed in the kernel C++ compile, the exact error this pass
exists to prevent.
With no backend configured the pass is now a no-op rather than defaulting
to A2A3; several codegen tests drive passes without one, and leaving the
IR untouched is the honest behaviour when there is no table to legalize
against. Both lookups CHECK-fail when no backend is set, so
BackendConfig::IsConfigured() is probed first.
The move is behaviour-preserving: over all 420 (arch, src, dst) triples
across both backends and 15 dtypes, the emitted chain is identical before
and after.
Docs are numbered by execution order per pass-doc-ordering.md. The pass runs
between FlattenTileNdTo2D (13) and AutoTileMatmulL0, so it takes 14 and the
existing 14..42 shift to 15..43 in both languages; slot 43 was freed by
d64380c removing 43-insert_comm_fence.md, so the cascade terminates without a
collision. Cross-references, the rule's own ordering table, and the
00-pass_manager.md properties table and pipeline list are updated to match, as
are pass numbers written out in prose and the adjacency claims that inserting a
pass between FlattenTileNdTo2D and AutoTileMatmulL0 invalidates.
The pass docs carry per-architecture tables of which (src, dst) pairs are a
single hardware tcvt and which expand into a chain, with the hop count, so the
performance and rounding consequences are visible to users rather than implicit.
A chain is bit-identical to a direct conversion only when each intermediate is
exact over the destination's range, so the cases that do double-round are named
with their measured rate. The language guide points at the tables from pl.cast.
f953ce9 to
270cd81
Compare
…-level CommRemoteOffset helper (#2168) > **Rebased onto `main` (`a55399d4`); #2135 is closed, so its 5 toolchain commits (`update PTOAS to v0.51` … `fix(runtime): Keep the _ptoas_binary seam`) now land here.** The PTOAS pin moved on to **v0.54** in the process. ## What 1. Pin PTOAS **v0.54**. 2. Reland the `InsertCommFence` pass (reverts #2138). 3. Collapse the `system.cacheinvalid` codegen down to a single path. 4. Restore the module-level `@CommRemoteOffset_<dtype>` helper that #2161 inlined. ## 1. PTOAS v0.54 `toolchain/versions.env` is the single source of truth — every workflow reads it through the `toolchain` lead job, so the bump needs no workflow change. Both digests were verified by downloading the release assets and hashing them locally: | Arch | sha256 | | ---- | ------ | | aarch64 | `011e980dbc46c796e31a1b213051d943ba8eb4c67d356ae6bda2148fe512964a` | | x86_64 | `4e3acb9623384c18fe264610525777210095f2ba24f6c52b9823bf1cb81d7a99` | The release tarball layout changed at v0.51 (`<root>/ptoas` went from an executable launcher script to a Python package directory). `python/pypto/backend/_ptoas_locate.py` probes each candidate for being an executable file rather than keying off the release version, so it is version-agnostic and needed no change for v0.52 or v0.54. ## 2. Reland InsertCommFence (reverts #2138) #2138 backed out #2076 because ptoas could not lower the publish-side region `cacheinvalid`: | ptoas | `pto.cmo.cacheinvalid <partition_tensor_view> single_cache_line` | | ----- | --------------------------------------------------------------- | | 0.50 | parsed, but **no call emitted** — the marker never reached the device | | 0.51 | emitted `PTOAS__DCCI_SINGLE_CACHE_LINE(<GlobalTensor>)`, whose body casts to `__gm__ void*` — a conversion `GlobalTensor` does not have, so every kernel carrying the op failed to compile | hw-native-sys/PTOAS#1001 fixes this with a `GlobalTensor` overload that takes the address via `tensor.data()`, shipped in v0.52 and carried by the v0.54 pin. Verified against the real 0.52 binary — the emitted C++ now carries both overloads and binds the object one: ```cpp static AICORE inline void PTOAS__DCCI_SINGLE_CACHE_LINE( pto::GlobalTensor<Element, Shape, Stride, TensorLayout> &tensor) { dcci((__gm__ void*)tensor.data(), cache_line_t::SINGLE_CACHE_LINE); } ``` This is a plain revert of #2138 except for three deliberate deviations: - `toolchain/versions.env` stays on this branch's pin (now v0.54); the revert's restore of the v0.50 pin is dropped. - The pass doc is renumbered **43 → 44**. #2141 landed `LegalizeTileCast` at slot 14 and shifted everything below it, so `classify_iter_arg_carry` now owns 43 and `InsertCommFence` — still dead last in the pipeline — takes 44. - The revert's CommRemoteOffset inlining is **not** relanded, because #2161 landed the same inlining on `main` independently. See §4 — this PR takes that emission the other way. `44-insert_comm_fence.md` is wired into the mkdocs nav, `passes/index.md` and `00-pass_manager.md` (en + zh). The docs became an MkDocs site in #2193, and `mkdocs build --strict` fails on a page absent from the nav; `docs/zh-cn/` was also renamed to `docs/zh/` there, which the rebase picked up. ## 3. Route every `cacheinvalid` region through `partition_view` The scalar-write branch of `system.cacheinvalid` codegen emitted a bare pointer operand. Measured against ptoas 0.52: | operand | result | | ------- | ------ | | `!pto.ptr`, no type annotation ← **what we emitted** | parse error: `expected ':'` | | `!pto.ptr`, with type annotation | lowering: `addptr must feed make_tensor_view, ...` | | `!pto.tensor_view<1xf32>` | compiles | | `!pto.partition_tensor_view<1x1xf32>` | compiles | So that branch has never produced working code. It is reachable from the DSL (`pl.system.cacheinvalid(t, [1, 1], off)`) and, with this reland, from `InsertCommFence` too — `MakeCacheInvalid` uses the target's full shape, so any published tensor that is itself 1x1 lands there. The special case only existed because the region path was broken on ptoas <= 0.51. Now that it lowers correctly, a 1x1 `partition_view` is right: verified on the real binary, a `[1, 1]` region at offsets `[0, 8]` over a `[16, 16]` f32 tensor emits `GlobalTensor<float, Shape<1,1,1,1,1>, Stride<16,16,16,16,1>>` at `v1 + 8`. End-to-end, pypto's own generated `.pto` for that case now compiles where it previously hit the parse error. The branch is therefore deleted — one construction, one emit site. `GetFlatOffsetSSA` and `GetTensorBasePtr` remain in use elsewhere, so nothing is orphaned. **This supersedes #2137**, which fixes the same bug by routing the scalar case through `tensor_view<1xT>` and converging the two branches on a shared emit. Both work on 0.52; this one removes the branch entirely. Closing one of the two is a call for the authors — see "Open questions". `test_cacheinvalid_scalar_write_emits_ptr` asserted the broken form (`"partition_tensor_view" not in cmo_line`) and could not survive the fix as written. It is folded into a parametrized `test_cacheinvalid_region_emits_partition_view` covering both sizes; the dynamic-offset test now asserts the partition-view operand instead of `pto.addptr`. ## 4. Restore the module-level CommRemoteOffset helper #2161 ("Complete arbitrary-length allreduce support") inlined the distributed peer-address calculation at every remote-op call site and deleted the per-dtype `@CommRemoteOffset_<dtype>` helper. Its stated reason was that ptoas' `pto-memory-consistency` pass rejected a `func.call` to a callee holding the CommContext `pto.load_scalar` reads. **ptoas no longer performs that check**, so the helper form is viable again, and this PR restores it: - Op lowering registers the dtype via `PTOCodegen::RegisterCommRemoteOffsetHelper` and emits one `func.call @CommRemoteOffset_<dtype>(ctx, peer) -> index`. - `EmitCommRemoteOffsetHelpers` flushes one `func.func private` body per registered dtype at module end; MLIR resolves the forward references whole-module. - Sharing the CommContext field reads and the byte→element division across call sites keeps the emitted kernels smaller than the inlined form. `pto.addptr` and `pto.make_tensor_view` stay at the call site, as they did both before and after #2161 — PTOAS verifies per-function that `addptr` feeds `make_tensor_view`, and a tensor view cannot cross a func boundary because its lowered memref is strided. Returning the element offset is the only shape that satisfies both constraints. **Everything else from #2161 is preserved**: ragged-tail handling, DN / column-vector stride derivation, the tightened valid-shape inference, and the `allow_physical_tail_padding` attr on `pld.tile.remote_load`. Docs (en/zh), op descriptions, DSL docstrings, binding comments and the codegen unit-test assertions move back to the helper wording in lockstep. ## 5. Document that ptoas does not gate the markers The pass doc claimed that removing the wait-side `cacheinvalid all` from the ring-allreduce `.pto` "is rejected". That was measured on 0.50 and no longer holds. Re-run on 0.52 against the same `ring_step` kernel from `tests/st/distributed/collectives/test_l3_allreduce_ring.py`: | variant | ptoas 0.52 | | ------- | ---------- | | unmodified | accepted | | wait-side `cacheinvalid all` removed | accepted | | every `cacheinvalid` **and** `system.fence` removed | accepted | All three compile with no diagnostic; the instructions are simply absent from the generated C++. The doc now qualifies the original claim with the version it was measured on and adds a section stating there is no compile-time gate — a missing marker is a data race, not a build error — with an explicit warning against reading a green test run as evidence a marker is unnecessary. ## Testing - [x] Full unit suite on the rebased tree: `pytest tests/ut/ -n auto` → **8116 passed, 2 skipped** - [x] One pre-existing failure unrelated to this branch: `test_benchmark.py::test_benchmark_l3_ignores_prepare_setup_groups` — the test double's `prepare()` predates the `persistent=` kwarg added by #2163; `bench.py` and that test file are byte-identical to `main` here - [x] `tests/ut/ir/transforms/test_insert_comm_fence.py` + `tests/ut/codegen/distributed/` → 95 passed - [x] `ruff check` / `ruff format --check` / `clang-format --dry-run --Werror` clean on the diff - [x] Docs updated en + zh; new pass doc wired into the nav and both indices - [x] The ptoas claims in §2/§3/§5 were measured against the real v0.52 binary, not inferred - [ ] **`dist-system-tests` not run** — needs a 2-device host. This is the meaningful acceptance gate: those 36 cases are what #2138 backed the pass out for, and they are also what would exercise the restored `func.call` form from §4 on device. ## Open questions 1. **Overlap with #2137.** Same bug, same file, two fixes. One should close. 2. **The pass's benefit is inferred, not observed.** Under the 0.50 pin the publish-side region `cacheinvalid` emitted no call at all (hw-native-sys/PTOAS#995), so it never reached the device — and the distributed suite was green throughout. Combined with §5 (ptoas does not check for the markers), no existing test demonstrates what this pass fixes. Confirming it needs a case built to expose the race — large transfers, multiple ranks, repeated runs. Relates to #2076, #2138, #2161, hw-native-sys/PTOAS#995, hw-native-sys/PTOAS#1001
Summary
Adds a
LegalizeTileCastpass that expandstile.castdtype pairs the targetISA cannot emit as a single
pto.tcvtinto the shortest chain of native casts.Ported from the fork branch
lwDavid/a5-early-resolve-guard(7f702b39+aab3ac06), which was never merged.Motivation
pto.tcvtsupports only a profile-dependent subset of(src, dst)dtype pairs,and the subset differs per architecture. A5 (Ascend950) has no native
INT32 -> FP16; A2/A3 provides it as a deq instruction. So a DSL-levelpl.cast(x_i32, pl.FP16, mode="round")compiles on A2/A3 and fails the kernelC++ compile on A5:
DeepSeek V4-Pro's INT8 quantization chain is
fp32 -> i32(rint) -> fp16(round) -> i8(trunc)at 14 sites across the model, sowithout this pass none of those kernels can be built for A5 at all.
Approach
Each non-native
tile.castis rewritten into the shortest chain of native casts,found by BFS over a per-arch ISA adjacency table — on A5,
INT32 -> FP16becomesINT32 -> FP32 -> FP16. Among equal-length paths it prefers "same byte-width →float, then adjust width". Already-native casts, including FIXPIPE-foldable
FP32 -> BF16/FP16withmode=rint, are left untouched.Position: after
FlattenTileNdTo2D(so casts that pass inserts are legalizedtoo), before
AutoTileMatmulL0.Numerics are unchanged for the case that motivated it: both hops are exact over
the
|q| <= 127range the quantization chain produces (INT32 -> FP32exactbelow 2^24,
FP32 -> FP16exact for integers below 2^11).A census of the generated
.ptofor the DeepSeek kernels shows they emit 8distinct dtype pairs, of which
INT32 -> FP16is the only non-native A5 pair, socoverage is complete for those models; post-fix builds contain zero
i32 -> f16.Faithfulness. Shortest alone is not enough: an intermediate that cannot hold
what the destination can would silently drop values a direct conversion keeps. A5
has no native
UINT32 -> FP32and every shortest route passes throughINT16/UINT16/UINT8, so 40000 — exactly representable in FP32 — would comeback as garbage. Intermediates that provably narrow relative to the destination
are excluded from the search, and a pair with no faithful chain is reported rather
than lowered to a lossy one. Only provable narrowing is rejected, so an unfamiliar
dtype stays admissible instead of turning a working lowering into a hard failure.
Arch resolution. The profile comes from the
PassContextBackendHandler,falling back to the global
BackendConfig, and finally to the A2A3 table. Thefallback matters: several existing codegen tests drive passes with neither
configured, and the A2A3 table is a superset of the pairs in question, so the
pass is a no-op there rather than an error.
Complexity is O(N) in IR size.
PassPropertiesis empty (the pass neitherrequires nor invalidates a property).
Testing
python -m pytest tests/ut/→ 7710 passed, 2 skipped.tests/ut/ir/transforms/test_legalize_tile_cast.py(8 cases): A5 bridgeexpansion, A2/A3 left native,
FP16 -> BF16via FP32, native pair untouched,idempotency on an already-bridged chain, factory smoke test, plus the two
review-driven cases below — a rejected narrowing bridge and the no-backend
fallback.
tests/ut/ir/transforms/test_pass_manager.py:"LegalizeTileCast"added toboth expected pipeline lists.
On a real Ascend 950 host, all 27 single-card
models/deepseek/v4-pro/*.pyfrom pypto-lib
56b59dd, run serially on one card with ptoas 0.48:runtime=8cdb306c)This pass is necessary but not sufficient on its own: it unblocks the kernel
compile, after which the remaining kernels fail later in the pipeline for two
reasons that live in the pinned runtime, not here. Of the 20 still failing in the
first row: 15 fail the orchestration compile on
set_allow_early_resolve, 4 arenumerical, and 1 is the known-flaky
prefill_sparse_attn(AICPU507018, ~1 passin 5, unrelated to this change).
Follow-up: the runtime pin
The two remaining causes are both already fixed on simpler
main, after ourpinned
8cdb306c(2026-07-20):d4071fe1) enables-mllvm -cce-vf-aa-between-iters=trueforA5. Without that stricter inter-iteration alias analysis, BiSheng VF fusion
reorders same-V vector ops (expand / abs / sinkhorn) and silently miscompiles
~15 V4-Pro kernels — they build and run and return wrong values (
hc_pre'scombcomes back all-NaN). That is the 4 numerical failures above plus othersmasked behind the orchestration failure.
13197445) portsallow_early_resolve/ early-dispatch tothe A5 tmr runtime, fixing the 15 orchestration-compile failures.
I originally included that bump here and backed it out: it fails
dist-system-tests, because the same span contains simpler#1436("simplify NEXT_LEVEL scheduling with explicit targets"), whose new contract is
"the runtime never selects a NEXT_LEVEL worker on the caller's behalf".
pypto's
src/codegen/distributed/distributed_codegen.cpp:1087still emits-1("you pick") for comm-less dispatches, which the new API rejects:
11 L3 tests fail that way. Porting pypto's L3 dispatch to explicit targets is a
design decision about where a comm-less dispatch should land, and #1436 precedes
#1446 so it cannot be avoided by picking an earlier revision. It belongs in its
own PR, from someone who can exercise the distributed path — the host I used has
no working HCCL. Happy to file the issue if that helps.
Doc numbering
The pass runs between
FlattenTileNdTo2D(doc 13) andAutoTileMatmulL0, so per.claude/rules/pass-doc-ordering.mdit takes 14 and the existing14..42shiftto
15..43in both languages. Slot 43 was freed byd64380cbremoving43-insert_comm_fence.md, so the cascade terminates without a collision.That accounts for most of the diff: 60 renames (29 per language plus the new file),
247 cross-reference updates, the ordering table in the rule itself (numbers, ordinals,
and two in-prose "pass 18" references that now mean 19), and the properties table plus
pipeline list in
00-pass_manager.mdin both languages.Verified: all 392 internal doc links resolve, the pipeline range is contiguous
01..43with no gaps, en/zh parity holds (82 paired paths), and no
14areference survives.