feat(ir): add codegen support for system.sync_src/sync_dst - #2307
feat(ir): add codegen support for system.sync_src/sync_dst#2307Shenggan wants to merge 4 commits into
Conversation
…m.sync_src/sync_dst Adds PTO codegen for a dynamic (ScalarType(INDEX)) event_id operand on system.sync_src/sync_dst, lowering to pto.set_flag_dyn/pto.wait_flag_dyn alongside the existing static int form (pto.set_flag/pto.wait_flag). Mirrors the cross-core system.sync_set/sync_wait design, and is needed for manual multi-buffer pipelining where the flag identity is picked by a runtime slot index rather than known at compile time. Supporting pieces: - IR: op registration and static-xor-dynamic operand type deduction (src/ir/op/sync_ops/sync.cpp); Python IR/DSL wrappers accepting an Expr event_id (python/pypto/ir/op/system_ops.py, python/pypto/language/op/system_ops.py) - Parser: DSL round-trip for the dynamic operand (tests/ut/language/parser/test_system_ops.py) - Tests: IR-level static/dynamic construction and validation (tests/ut/ir/test_sync_flag_ops.py, new), and PTO codegen emission for both forms (TestSyncFlagCodegen in tests/ut/codegen/test_pto_codegen_ops.py)
|
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:
📝 WalkthroughWalkthroughThe change adds static and dynamic event-ID support to ChangesSynchronization event ID support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PythonDSL
participant IRSyncOp
participant PTOCodegen
participant PTOInstructions
PythonDSL->>IRSyncOp: submit sync_src or sync_dst with event_id
IRSyncOp->>IRSyncOp: validate event_id and pipe attributes
IRSyncOp->>PTOCodegen: lower synchronization operation
PTOCodegen->>PTOInstructions: emit static or dynamic flag instruction
Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/backend/common/pto_ops_memory.cpp`:
- Around line 775-787: Update the validation in the pipe setup logic before the
casts used to build pipe_operands, ensuring set_pipe and wait_pipe are each
recognized supported PipeType values rather than only non-negative integers.
Reuse the existing PipeType definitions or validation utility, and reject
unknown values through INTERNAL_CHECK_SPAN before calling ir::PipeTypeToString.
🪄 Autofix
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: af4056a8-0177-4cc7-8064-8cebb958340c
📒 Files selected for processing (7)
python/pypto/ir/op/system_ops.pypython/pypto/language/op/system_ops.pysrc/backend/common/pto_ops_memory.cppsrc/ir/op/sync_ops/sync.cpptests/ut/codegen/test_pto_codegen_ops.pytests/ut/ir/test_sync_flag_ops.pytests/ut/language/parser/test_system_ops.py
![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: c73551d2a8
ℹ️ 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".
…est parametrize Reject out-of-range pipe attribute values before casting to ir::PipeType, instead of only checking non-negativity, so an invalid value fails with INTERNAL_CHECK_SPAN rather than an unlabeled TypeError from PipeTypeToString. Also splits the sync flag test's parametrize call to stay under the Ruff line-length limit.
Adds support for manual pipelining, as demonstrated in:
https://github.com/Shenggan/pypto-gemm/blob/main/hpgemm_step4_manual_pipeline.py
Performance results are available in:
https://github.com/Shenggan/pypto-gemm/blob/main/README.md
Adds PTO codegen for static or dynamic (ScalarType(INDEX)) event_id operand on system.sync_src/sync_dst, lowering to pto.set_flag/pto.wait_flag.
Supporting pieces: