feat(a5/tmr): port allow_early_resolve / early-dispatch from a2a3#1423
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA5 Runtime 2 gains early-dispatch scheduling with gated AICore execution, doorbell synchronization, lifecycle tracking, staged sync-start draining, updated payload layouts, and new regular, sync-start, and MIX spill tests. ChangesA5 Early Dispatch
Estimated code review effort: 5 (Critical) | ~120 minutes 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 |
There was a problem hiding this comment.
Code Review
This pull request implements the speculative early-dispatch model for the a5 runtime, allowing tasks to be pre-staged on idle or busy cores and gated via a high-32 doorbell on the DATA_MAIN_BASE register. The changes span the AICore execution loop, scheduler dispatch and completion logic, orchestrator dependency wiring, and documentation. Feedback on the changes suggests updating the simulation implementation of read_dmb_high32 to use an atomic acquire load (__atomic_load_n with __ATOMIC_ACQUIRE) instead of a raw volatile dereference to establish a proper happens-before relationship and prevent data races under TSAN.
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.
Bring a5 tensormap_and_ringbuffer in line with a2a3 early-dispatch (hw-native-sys#989/hw-native-sys#1079/hw-native-sys#1285/hw-native-sys#1288/hw-native-sys#1297/hw-native-sys#1304/hw-native-sys#1326/hw-native-sys#1329/hw-native-sys#1336/hw-native-sys#1405/hw-native-sys#1340): prepare/publish, src_payload gate + DMB high32 doorbell (sim uses __atomic_load_n ACQUIRE), direct-only eligibility, spare-slot Phase 4b, sync_start early rendezvous, and dep_gen early_dispatch truth. Document the a5 model in RUNTIME_LOGIC §8.6. Add a5 STs: plain early_dispatch plus sync_start early_dispatch and mix_spill ports (EarlyOn/Off) for board validation.
cbc9612 to
b66a556
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/kernels/orchestration/spmd_sync_start_mix_spill_orch.cpp (1)
78-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWrap task submission in a manual scope.
Add
rt_scope_begin(PTO2ScopeMode::MANUAL)andrt_scope_end()around the task submissions to ensure both tasks are batched and published to the scheduler simultaneously. This matches the behavior of the other early-dispatch tests in this PR and prevents non-deterministic staging behaviors.🛠️ Proposed fix
- PTO2TaskId prod = submit_aiv_producer(ext_output, 72, 0, early_on); - submit_mix_sync_consumer(ext_output, 24, 72, prod); + rt_scope_begin(PTO2ScopeMode::MANUAL); + PTO2TaskId prod = submit_aiv_producer(ext_output, 72, 0, early_on); + submit_mix_sync_consumer(ext_output, 24, 72, prod); + rt_scope_end();🤖 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/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/kernels/orchestration/spmd_sync_start_mix_spill_orch.cpp` around lines 78 - 79, Wrap the task submissions in the orchestration flow with a manual runtime scope: call rt_scope_begin(PTO2ScopeMode::MANUAL) before submit_aiv_producer, keep submit_mix_sync_consumer within the same scope, and call rt_scope_end() afterward so both tasks are published together.tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/test_spmd_sync_start_mix_spill.py (1)
64-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate
kernel_spmd_write_slow.cpp.The
spmd_sync_start_mix_spilltest introduces a duplicate copy ofkernel_spmd_write_slow.cpp. To eliminate code duplication, it can reuse the existing kernel fromspmd_sync_start_early_dispatch(astest_spmd_early_dispatch.pyalready does).
tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/test_spmd_sync_start_mix_spill.py#L64-L70: Update the"source"path forSPMD_WRITE_AIVto"../spmd_sync_start_early_dispatch/kernels/aiv/kernel_spmd_write_slow.cpp".tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/kernels/aiv/kernel_spmd_write_slow.cpp#L1-L77: Delete this duplicate file entirely.🤖 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/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/test_spmd_sync_start_mix_spill.py` around lines 64 - 70, Deduplicate the SPMD write kernel by updating SPML_WRITE_AIV in tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/test_spmd_sync_start_mix_spill.py lines 64-70 to reference ../spmd_sync_start_early_dispatch/kernels/aiv/kernel_spmd_write_slow.cpp, then delete the duplicate file at tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/kernels/aiv/kernel_spmd_write_slow.cpp lines 1-77.tests/st/a5/tensormap_and_ringbuffer/spmd_early_dispatch/test_spmd_early_dispatch.py (1)
34-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueResolve static analysis warnings for mutable class attributes.
The
CALLABLEandCASESclass attributes have mutable default values (dict/list), which triggers Ruff'sRUF012warning. Please annotate them withtyping.ClassVarto resolve this.
tests/st/a5/tensormap_and_ringbuffer/spmd_early_dispatch/test_spmd_early_dispatch.py#L34-L64: ImportClassVarfromtypingand annotateCALLABLEandCASES(e.g.,CALLABLE: ClassVar[dict] = {...}).tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_early_dispatch/test_spmd_sync_start_early_dispatch.py#L33-L84: AnnotateCALLABLEandCASESwithClassVar.tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/test_spmd_sync_start_mix_spill.py#L36-L87: AnnotateCALLABLEandCASESwithClassVar.🤖 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/st/a5/tensormap_and_ringbuffer/spmd_early_dispatch/test_spmd_early_dispatch.py` around lines 34 - 64, Mutable class attributes trigger Ruff RUF012 because CALLABLE and CASES are unannotated dict/list values. Import ClassVar from typing and annotate both attributes in tests/st/a5/tensormap_and_ringbuffer/spmd_early_dispatch/test_spmd_early_dispatch.py (34-64), tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_early_dispatch/test_spmd_sync_start_early_dispatch.py (33-84), and tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/test_spmd_sync_start_mix_spill.py (36-87), preserving their existing values and behavior.Source: Linters/SAST tools
🤖 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/st/a5/tensormap_and_ringbuffer/spmd_early_dispatch/test_spmd_early_dispatch.py`:
- Around line 34-64: Mutable class attributes trigger Ruff RUF012 because
CALLABLE and CASES are unannotated dict/list values. Import ClassVar from typing
and annotate both attributes in
tests/st/a5/tensormap_and_ringbuffer/spmd_early_dispatch/test_spmd_early_dispatch.py
(34-64),
tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_early_dispatch/test_spmd_sync_start_early_dispatch.py
(33-84), and
tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/test_spmd_sync_start_mix_spill.py
(36-87), preserving their existing values and behavior.
In
`@tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/kernels/orchestration/spmd_sync_start_mix_spill_orch.cpp`:
- Around line 78-79: Wrap the task submissions in the orchestration flow with a
manual runtime scope: call rt_scope_begin(PTO2ScopeMode::MANUAL) before
submit_aiv_producer, keep submit_mix_sync_consumer within the same scope, and
call rt_scope_end() afterward so both tasks are published together.
In
`@tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/test_spmd_sync_start_mix_spill.py`:
- Around line 64-70: Deduplicate the SPMD write kernel by updating
SPML_WRITE_AIV in
tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/test_spmd_sync_start_mix_spill.py
lines 64-70 to reference
../spmd_sync_start_early_dispatch/kernels/aiv/kernel_spmd_write_slow.cpp, then
delete the duplicate file at
tests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/kernels/aiv/kernel_spmd_write_slow.cpp
lines 1-77.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52f2929d-a58b-4c35-8a96-342af3cc54ab
📒 Files selected for processing (26)
src/a5/platform/onboard/aicore/inner_kernel.hsrc/a5/platform/sim/aicore/inner_kernel.hsrc/a5/runtime/tensormap_and_ringbuffer/aicore/aicore_executor.cppsrc/a5/runtime/tensormap_and_ringbuffer/docs/RUNTIME_LOGIC.mdsrc/a5/runtime/tensormap_and_ringbuffer/orchestration/pto_arg_with_deps.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto2_dispatch_payload.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_orchestrator.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_runtime2_types.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_types.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/pto_scheduler.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_completion.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_context.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_dispatch.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_types.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/shared/pto_runtime2_init.cpptests/st/a5/tensormap_and_ringbuffer/spmd_early_dispatch/kernels/orchestration/spmd_early_dispatch_orch.cpptests/st/a5/tensormap_and_ringbuffer/spmd_early_dispatch/test_spmd_early_dispatch.pytests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_early_dispatch/kernels/aiv/kernel_spmd_write_slow.cpptests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_early_dispatch/kernels/orchestration/spmd_sync_start_early_dispatch_orch.cpptests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_early_dispatch/test_spmd_sync_start_early_dispatch.pytests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/kernels/aic/kernel_spmd_mix_slow.cpptests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/kernels/aiv/kernel_spmd_mix_slow.cpptests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/kernels/aiv/kernel_spmd_write_slow.cpptests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/kernels/orchestration/spmd_sync_start_mix_spill_orch.cpptests/st/a5/tensormap_and_ringbuffer/spmd_sync_start_mix_spill/test_spmd_sync_start_mix_spill.py
Remove the a5-only early_dispatch / sync_start early ST scenes added for board experiments; keep the runtime port focused for CI.
Remove accidental local helpers, migrate patches, and the acc_c2v example that were committed with the UT payload-pool fix. Keep the early-dispatch runtime port, a2a3-ported sync_start STs, and a5 UT payload/task pool binding.
Bring a5 tensormap_and_ringbuffer in line with a2a3 early-dispatch (#989/#1079/#1285/#1288/#1297/#1304/#1326/#1329/#1336/#1405/#1340): prepare/publish, src_payload gate + DMB high32 doorbell, direct-only eligibility, spare-slot Phase 4b, sync_start early rendezvous, and dep_gen early_dispatch truth. Document the a5 model in RUNTIME_LOGIC §8.6.