feat(distributed): add all_to_all push-based composite intrinsic (InCore only)#1937
feat(distributed): add all_to_all push-based composite intrinsic (InCore only)#1937georgebisbas wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughAdds a new distributed collective op Changesall_to_all Collective Op
Unrelated Minor Fix
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Rank
participant TargetWindow
participant SignalWindow
participant OutTensor
Rank->>TargetWindow: stage input[dest,:] into target[dest,0]
Rank->>SignalWindow: notify all peers
Rank->>SignalWindow: wait all signals
Rank->>TargetWindow: pld.tile.get peer chunk from target[my_rank,0]
Rank->>OutTensor: write chunk into out[src,0]
Possibly related PRs
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: 052a342433
ℹ️ 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.
Code Review
This pull request introduces the pld.tensor.all_to_all symmetric all-to-all collective operator as a 4-arg InCore composite, complete with C++ registration, lowering rules, Python bindings, and system tests. The review feedback highlights several critical improvements: validating element type compatibility in the type deducer, adding a post-exchange barrier in the lowering rule to prevent Write-After-Read (WAR) hazards, explicitly constructing 2D offsets instead of using MakeSignalOffsets to avoid coupling, and correcting the documentation to classify the operator as a composite rather than a hardware-supported builtin collective.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
python/pypto/ir/op/distributed/tensor_ops.py (1)
321-337: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocstring references
pld.tile.get, but the actual primitive ispld.tile.remote_load.The composite is documented as doing "peer-read via
pld.tile.get" (Line 333), but the sibling hand-rolled DSL test and the intrinsic test both callpld.tile.remote_loadfor the exchange phase. This stale naming could mislead future readers cross-referencing the DSL primitive.📝 Suggested docstring fix
- 3-phase mesh decomposition (stage-in → barrier → peer-read via pld.tile.get). + 3-phase mesh decomposition (stage-in → barrier → peer-read via pld.tile.remote_load).🤖 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 `@python/pypto/ir/op/distributed/tensor_ops.py` around lines 321 - 337, The all_to_all docstring is using the wrong peer-read primitive name, which can mislead readers and cross-references. Update the documentation in all_to_all to describe the exchange phase with the actual primitive name used elsewhere, pld.tile.remote_load, and keep the rest of the composite description 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/distributed_ops.md`:
- Line 36: The `pld.tensor.all_to_all` row is mislabeled as a “builtin
collective,” which implies a HOST-dispatch builtin exists like the other
collectives. Update the wording in the distributed ops table to avoid that
classification for `all_to_all` and make it clear it is not backed by a
`builtin.tensor.all_to_all` HOST op, keeping the description aligned with the
`all_to_all` entry in `distributed_ops.md`.
In `@docs/zh-cn/dev/distributed_ops.md`:
- Line 32: The `pld.tensor.all_to_all` row is mislabeled as `builtin collective`
even though this op does not have a HOST-dispatch builtin counterpart. Update
the hardware/classification entry in the distributed ops table for
`pld.tensor.all_to_all` to reflect InCore-only composite support, matching the
intended labeling used for this operation in the docs.
In `@src/ir/op/distributed/collective.cpp`:
- Around line 303-339: The type deduction in DeduceTensorAllToAllType only
checks each argument’s own rank, but it must also verify that input, target, and
out are mutually consistent. Add cross-argument checks in
DeduceTensorAllToAllType so input_type, target_type, and out_type agree on the
expected [NR, SIZE] shape (especially the SIZE dimension) and, where applicable,
compatible dtypes before returning. Use the existing symbols
DeduceTensorAllToAllType, input_type, target_type, signal_type, and out_type to
enforce these invariants and fail fast on mismatches.
---
Nitpick comments:
In `@python/pypto/ir/op/distributed/tensor_ops.py`:
- Around line 321-337: The all_to_all docstring is using the wrong peer-read
primitive name, which can mislead readers and cross-references. Update the
documentation in all_to_all to describe the exchange phase with the actual
primitive name used elsewhere, pld.tile.remote_load, and keep the rest of the
composite description 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
Run ID: a8baea7e-5949-4453-a35e-dd42da862814
📒 Files selected for processing (9)
docs/en/dev/distributed_ops.mddocs/zh-cn/dev/distributed_ops.mdpython/pypto/backend/pto_backend.pypython/pypto/ir/op/distributed/tensor_ops.pypython/pypto/language/distributed/op/tensor_ops.pysrc/ir/op/distributed/collective.cppsrc/ir/transforms/lower_composite_ops_pass.cpptests/st/distributed/collectives/test_l3_all_to_all.pytests/st/distributed/collectives/test_l3_tensor_all_to_all_intrinsic.py
There was a problem hiding this comment.
Pull request overview
Adds a new distributed collective, pld.tensor.all_to_all, as an InCore-only composite intrinsic that lowers into a 3-phase mesh exchange (stage-in → notify/wait barrier → peer read via pld.tile.get/TGET-style transfer). This extends the distributed DSL/IR surface area, compiler lowering, and system tests to cover an all-to-all personalized exchange pattern.
Changes:
- Register
pld.tensor.all_to_allas a newDistributedOpwith a type deducer in C++. - Add composite lowering in
LowerCompositeOpsimplementing the 3-phase all-to-all decomposition. - Add Python IR builder + DSL wrapper, plus ST coverage (hand-rolled reference + intrinsic test) and docs row updates.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ir/transforms/lower_composite_ops_pass.cpp | Adds LowerTensorAllToAllRule and wires it into the composite lowering dispatch. |
| src/ir/op/distributed/collective.cpp | Registers pld.tensor.all_to_all and introduces DeduceTensorAllToAllType. |
| python/pypto/ir/op/distributed/tensor_ops.py | Adds IR builder helper to construct pld.tensor.all_to_all(...) calls. |
| python/pypto/language/distributed/op/tensor_ops.py | Adds the DSL wrapper all_to_all(input, target, signal, out). |
| tests/st/distributed/collectives/test_l3_all_to_all.py | Adds a hand-rolled 3-phase DSL reference all-to-all system test. |
| tests/st/distributed/collectives/test_l3_tensor_all_to_all_intrinsic.py | Adds a system test exercising the composite intrinsic end-to-end. |
| docs/en/dev/distributed_ops.md | Adds pld.tensor.all_to_all to the distributed ops table and updates op count. |
| docs/zh-cn/dev/distributed_ops.md | Mirrors the EN docs update in Chinese and updates op count. |
| python/pypto/backend/pto_backend.py | Adds a pyright suppression for Traversable import resolution. |
- Add Phase 3.5 post-exchange WAR barrier (AtomicAdd 1 -> wait >= 2) matching allreduce's Phase 3.5 rationale - Replace MakeSignalOffsets with explicit 2D MakeTuple offsets - Add cross-arg dtype and rank validation in DeduceTensorAllToAllType - Add signal 2D shape validation - Fix docs: label all_to_all as composite not builtin (EN+ZH) - Fix docstrings: pld.tile.get -> pld.tile.remote_load (IR), Google-style Args/Returns (DSL), pld.tile.get in test - Switch Tier 1 test Phase 3 to pld.tile.get(stage=...) matching the C++ lowering pattern
- Add Phase 3.5 post-exchange WAR barrier (AtomicAdd 1 -> wait >= 2) matching allreduce's Phase 3.5 rationale - Replace MakeSignalOffsets with explicit 2D MakeTuple offsets - Add cross-arg dtype and rank validation in DeduceTensorAllToAllType - Add signal 2D shape validation - Fix docs: label all_to_all as composite not builtin (EN+ZH) - Fix docstrings: pld.tile.get -> pld.tile.remote_load (IR), Google-style Args/Returns (DSL), pld.tile.get in test - Switch Tier 1 test Phase 3 to pld.tile.get(stage=...) matching the C++ lowering pattern
a64f018 to
18d9a06
Compare
- Add Phase 3.5 post-exchange WAR barrier (AtomicAdd 1 -> wait >= 2) matching allreduce's Phase 3.5 rationale - Replace MakeSignalOffsets with explicit 2D MakeTuple offsets - Add cross-arg dtype and rank validation in DeduceTensorAllToAllType - Add signal 2D shape validation - Fix docs: label all_to_all as composite not builtin (EN+ZH) - Fix docstrings: pld.tile.get -> pld.tile.remote_load (IR), Google-style Args/Returns (DSL), pld.tile.get in test - Switch Tier 1 test Phase 3 to pld.tile.get(stage=...) matching the C++ lowering pattern
Suggestion: a push-based lowering removes the
|
- Add Phase 3.5 post-exchange WAR barrier (AtomicAdd 1 -> wait >= 2) matching allreduce's Phase 3.5 rationale - Replace MakeSignalOffsets with explicit 2D MakeTuple offsets - Add cross-arg dtype and rank validation in DeduceTensorAllToAllType - Add signal 2D shape validation - Fix docs: label all_to_all as composite not builtin (EN+ZH) - Fix docstrings: pld.tile.get -> pld.tile.remote_load (IR), Google-style Args/Returns (DSL), pld.tile.get in test - Switch Tier 1 test Phase 3 to pld.tile.get(stage=...) matching the C++ lowering pattern
18d9a06 to
8c8abfa
Compare
There was a problem hiding this comment.
Implemented. The lowering has been fully rewritten to push-based using pld.tile.remote_store (TSTORE-based). Key changes:
- API: 4-arg → 3-arg
(input, target, signal), returnstargetin-place (window-as-result, same idiom asreduce_scatter/broadcast) - Lowering: 2-phase (push → barrier) instead of 5-phase — no stage-in, no pull loop, no WAR barrier
- Memory:
NR×SIZE(window only) instead of(NR+1)×SIZE(window + out) - Self-rank: local
tile.storefor efficiency
tracked this for a seaprate follow up |
Adds pld.tensor.all_to_all (4-arg InCore composite only): - LowerTensorAllToAllRule: 3-phase decomposition (stage-in loop, notify/wait barrier, exchange loop with pld.tile.get) - DeduceTensorAllToAllType: 4-arg validation - Python IR + DSL: all_to_all(input, target, signal, out) - Test: test_l3_tensor_all_to_all_intrinsic (P=2, P=4 sim verified) No HOST builtin infrastructure — this branch is InCore composite only, self-contained and independent of LowerHostTensorCollectives. style: clang-format line-wrapping fix in LowerTensorAllToAllRule fix: suppress pyright reportMissingImports on importlib.resources.abc (Python 3.10 compat) perf: vectorize test helpers with PyTorch broadcasting (replaces O(N³) loops) test(distributed): add hand-rolled Tier 1 all_to_all test Mirrors simpler's all_to_all_distributed 3-phase pattern using raw DSL primitives: pl.load/pl.store (stage-in), pld.system.notify/wait (barrier), pld.tile.remote_load (exchange). Complements the Tier 2 composite intrinsic test (test_l3_tensor_all_to_all_intrinsic).
- Add Phase 3.5 post-exchange WAR barrier (AtomicAdd 1 -> wait >= 2) matching allreduce's Phase 3.5 rationale - Replace MakeSignalOffsets with explicit 2D MakeTuple offsets - Add cross-arg dtype and rank validation in DeduceTensorAllToAllType - Add signal 2D shape validation - Fix docs: label all_to_all as composite not builtin (EN+ZH) - Fix docstrings: pld.tile.get -> pld.tile.remote_load (IR), Google-style Args/Returns (DSL), pld.tile.get in test - Switch Tier 1 test Phase 3 to pld.tile.get(stage=...) matching the C++ lowering pattern
…ments - Replace pull-based 5-phase lowering with push-based 2-phase using pld.tile.remote_store (TSTORE-based). API changes from 4-arg (input, target, signal, out) to 3-arg (input, target, signal), returning target in-place (window-as-result — same idiom as reduce_scatter/broadcast). - Add signal.shape_[0] cross-check in DeduceTensorAllToAllType (YunjiQin review). Same fix applied to allgather 4-arg deducer. - Update all docstrings to reference pld.tile.remote_store instead of pld.tile.remote_load or pld.tile.get (aligns with actual lowering). - Rewrite Tier 1 and Tier 2 tests for push-based pattern. Sim Docker: 11/11 collective tests pass, zero regressions.
8c8abfa to
bc9096d
Compare
Switch the all_to_all composite push phase from tile.load + pld.tile.remote_store (TSTORE, with an explicit self-rank branch) to a shared VEC staging tile + pld.tile.put (TPUT). The HCCL TPUT engine streams each input[dest, :] row through the staging tile into the peer's window row [my_rank, 0], so rows larger than the tile are auto-chunked by pto-isa, and the self-rank case falls out via HCCL identity mapping (no separate local-store branch). pld.tile.put is emitted directly (not pld.tensor.put) because ConvertTensorToTileOps runs before LowerCompositeOps, mirroring how broadcast/allgather emit pld.tile.get. The 3-arg pld.tensor.all_to_all(input, target, signal) public API is unchanged. Updates the Tier 1 hand-rolled ST, op description, DSL/IR docstrings, and EN/ZH docs to reference pld.tensor.put (TPUT). Sim Docker: collectives regression 11/11 pass (P=2), all_to_all P=4 pass.
Summary
Adds
pld.tensor.all_to_allas a push-based InCore composite intrinsic. Every rank pushes its per-destination chunks directly to every peer's window viapld.tile.put(TPUT), then synchronises with a notify/wait barrier. Returnstargetin-place (window-as-result — same idiom asreduce_scatter/broadcast).Compiler infrastructure
LowerTensorAllToAllRuleinlower_composite_ops_pass.cpp: 2-phase push-based lowering usingpld.tile.put(TPUT-based), emitted directly (notpld.tensor.put) sinceConvertTensorToTileOpsruns beforeLowerCompositeOps— mirroring how broadcast/allgather emitpld.tile.getTests
test_l3_all_to_all.py: Hand-rolled push-based usingpld.tensor.put(TPUT) + notify/wait barrier