feat(codegen): support soft syncall for mix and aic_only core types#1944
Conversation
Extend soft-mode `system.syncall` (previously aiv_only-only) to `core_type="mix"` and `core_type="aic_only"`. - mix carries both a UB (Vec) and a flat L1 (Mat) scratch tile and is duplicated onto both the cube and vector lanes (SHARED); aic_only carries a single flat L1 scratch on the cube lane. Operand order matches pto-isa's soft-mix lowering: [gm_workspace, ub_scratch, l1_scratch, used_cores]. - Add `tile.create(flat_layout=True)` for a flat (slayout=none_box) L1/cbuf staging tile. A boxed NZ Mat tile mis-places the 8-int32 counter slots; stamping memory_space=Mat at creation makes InferTileMemorySpace preserve the none_box view. - Classify `system.syncall` by core_type in core_affinity (mix->SHARED, aic_only->CUBE, aiv_only->VECTOR) so a mix barrier reaches both lanes. - Generalize the codegen operand assembly (3 or 4 operands), widen the convert-to-tile alias check, and round-trip both scratch tiles through the printer. Validated on a2a3: mix (out=(a+1)@b+1), aic_only (out=a@b), and the aiv_only baseline all pass; ptoas compiles both new forms.
There was a problem hiding this comment.
Code Review
This pull request extends the soft-mode system.syncall cross-core barrier to support all core types (aiv_only, aic_only, and mix) rather than just aiv_only. To support this, a new flat_layout option is added to tile.create to allocate a flat, non-fractal L1/cbuf staging tile (slayout=none_box) for cube-participating barriers. The compiler's IR, backend code generation, core affinity classification, and Python printer are updated to handle the varying operands (including multiple scratch tiles for the mix barrier). Extensive system and unit tests are added to verify the new soft barrier modes. There are no review comments to address, so no further feedback is provided.
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.
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR generalizes Changessystem.syncall soft mode generalization
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant syncall as pl.system.syncall
participant TileCreate as pl.tile.create
participant IROp as syncall_soft (IR)
participant Backend as pto_ops_common (MLIR emit)
User->>syncall: syncall(mode="soft", core_type, gm_workspace, used_cores)
syncall->>syncall: validate core_type in allowlist
alt core_type is aiv_only or mix
syncall->>TileCreate: create UB scratch tile
end
alt core_type is aic_only or mix
syncall->>TileCreate: create flat L1 scratch tile (flat_layout=True)
end
syncall->>IROp: emit call with 3 or 4 operands
IROp->>IROp: validate operand arity vs core_type
IROp->>Backend: lower to pto.syncall
Backend->>Backend: validate scratch dtypes, build operand/type lists
Backend-->>User: emitted MLIR pto.syncall op
Possibly related PRs
Suggested labels: 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: 1657cd4725
ℹ️ 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.
🧹 Nitpick comments (1)
tests/ut/language/parser/test_system_ops.py (1)
147-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding round-trip tests for the new
aic_only/mixsoft forms.
test_syncall_soft_round_triponly exercises the 3-argaiv_onlypath. The newpython_printer.cppbranch adds distinct handling foraic_only(3-arg,scratch=is the L1 tile) andmix(4-arg, addsscratch_l1=) — a dedicated unit test per form would pin down that printer/parser contract independently of the full ST/backend pipeline.🤖 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/language/parser/test_system_ops.py` around lines 147 - 172, Add dedicated round-trip tests in test_syncall_soft_round_trip coverage for the new pl.system.syncall soft forms, since the current test only verifies the aiv_only path. Add one case that exercises the aic_only branch in python_printer.cpp and confirms the scratch= L1 tile handling, and another that exercises the mix branch and verifies scratch_l1= is preserved through AsPython/parse_program. Use the existing test_syncall_soft_round_trip, Before.as_python, and pl.parse_program flow to pin down each printer/parser contract independently.
🤖 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.
Nitpick comments:
In `@tests/ut/language/parser/test_system_ops.py`:
- Around line 147-172: Add dedicated round-trip tests in
test_syncall_soft_round_trip coverage for the new pl.system.syncall soft forms,
since the current test only verifies the aiv_only path. Add one case that
exercises the aic_only branch in python_printer.cpp and confirms the scratch= L1
tile handling, and another that exercises the mix branch and verifies
scratch_l1= is preserved through AsPython/parse_program. Use the existing
test_syncall_soft_round_trip, Before.as_python, and pl.parse_program flow to pin
down each printer/parser contract independently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 274943be-fc6c-4773-ba09-e04b931c2501
📒 Files selected for processing (13)
docs/en/dev/ir/05-operators.mddocs/zh-cn/dev/ir/05-operators.mdpython/pypto/ir/op/system_ops.pypython/pypto/ir/op/tile_ops.pypython/pypto/language/op/system_ops.pypython/pypto/language/op/tile_ops.pysrc/backend/common/pto_ops_common.cppsrc/ir/op/tile_ops/memory.cppsrc/ir/transforms/convert_tensor_to_tile_ops_pass.cppsrc/ir/transforms/python_printer.cppsrc/ir/transforms/utils/core_affinity.cpptests/st/runtime/cross_core/test_syncall.pytests/ut/language/parser/test_system_ops.py
- Add missing <string> include in core_affinity.cpp (cpplint) - Wrap over-length soft-syncall core_type error message (ruff E501) - Apply clang-format / ruff-format to changed files
…1944 Addresses review: inserting flat_layout before span shifted span's positional slot, so a direct IR caller passing span positionally (create(shape, dtype, mem, transpose, span)) would bind the Span to flat_layout and fail AnyCast<bool> in DeduceTileCreateTileType. Restore span to its original positional position and make flat_layout keyword-only in both the IR and DSL create helpers.
Summary
Extend soft-mode
system.syncall— previously limited tocore_type="aiv_only"— tocore_type="mix"andcore_type="aic_only". The pto-isa / ptoas side already implements the soft-mix and soft-aic barriers (SYNCALL_SOFT_MIX_IMPL/SYNCALL_SOFT_AIC_IMPL); this wires up the PyPTO side.Changes
aiv_only:[gm_workspace, ub_scratch, used_cores]aic_only:[gm_workspace, l1_scratch, used_cores]mix:[gm_workspace, ub_scratch, l1_scratch, used_cores]— a mix barrier rendezvouses AIC + AIV cores, so it is duplicated onto both lanes (SHARED) and each lane uses its own tile (matching pto-isa's soft-mix lowering).used_coresis the total participant count (AIC blocks + AIV subblocks).tile.create(flat_layout=True)— a new flat (non-fractal,slayout=none_box) L1/cbuf staging tile. A normal boxed NZ Mat tile mis-places the 8-int32 counter slots; stampingmemory_space=Matat creation makesInferTileMemorySpacepreserve thenone_boxview instead of overwriting it with the implicit boxed layout.system.syncallbycore_type(mix→SHARED,aic_only→CUBE,aiv_only→VECTOR) so a mix barrier reaches both lanes rather than being mis-labelled VECTOR from its UB scratch.Testing
mix(out = (a + 1) @ b + 1),aic_only(out = a @ b), and theaiv_onlysoft baseline all pass.ptoascompiles both new forms; ROUNDTRIP verification passes.aiv_only-only assertion updated.Related Issues
N/A