perf(runtime): remake of #1214 -- Single-round-trip AICore handshake#1310
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:
📝 WalkthroughWalkthroughThis PR removes the Changesa2a3 handshake protocol update
a5 handshake protocol and init lifecycle refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AicpuExecutor
participant SchedulerContext
participant AICore
AicpuExecutor->>SchedulerContext: pre_handshake_init(runtime, ...)
AicpuExecutor->>SchedulerContext: handshake_partition(tidx, nthreads)
SchedulerContext->>AICore: publish payload pointers, set aicpu_ready
AICore->>SchedulerContext: report physical_core_id, core_type, aicore_done
SchedulerContext->>SchedulerContext: platform_init_aicore_regs(reg_addr)
AicpuExecutor->>SchedulerContext: post_handshake_init(runtime)
SchedulerContext->>SchedulerContext: rebuild worker-id lists, assign_cores_to_threads
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 parallelizes the AICore handshake process across multiple AICPU threads to reduce preamble latency, splitting the initialization into pre-handshake, partitioned handshake, and post-handshake phases, while simplifying the handshake protocol by removing redundant flags. The review feedback highlights critical issues regarding bare busy-spins in the thread synchronization loops of AicpuExecutor::init that could lead to CPU starvation or livelocks in simulation environments, recommending the use of SPIN_WAIT_HINT() and relaxed memory ordering. Additionally, it points out a potential out-of-bounds vulnerability in SchedulerContext::pre_handshake_init due to an incomplete bounds check on the signed cores_total_num_ variable, which should be updated to reject negative values.
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: 2
🤖 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/aicpu-kernel-launch-mechanisms.md`:
- Around line 195-205: Update the remaining references from
SchedulerContext::handshake_all_cores to SchedulerContext::handshake_partition
in the affected documentation snippet so the naming matches the renamed
scheduler function and the already-updated hardware doc. Keep the surrounding
explanation intact, and ensure any mention of the handshake flow in this section
consistently uses handshake_partition alongside the updated aicore_done field
name.
In `@src/a5/runtime/tensormap_and_ringbuffer/docs/RUNTIME_LOGIC.md`:
- Line 602: The “Public surface” table is stale because it still describes
SchedulerContext::init(runtime, aicpu_thread_num, sched_thread_num, regs_base)
even though the lifecycle was split into pre_handshake_init,
handshake_partition, and post_handshake_init. Update that table entry to reflect
the new public methods and remove or replace the old init() reference so it
matches the current SchedulerContext surface and the scheduler_cold_path.cpp
lifecycle split.
🪄 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: efb67873-2118-4fc4-81a0-b617be4c3327
📒 Files selected for processing (13)
docs/aicpu-kernel-launch-mechanisms.mddocs/hardware/mmio-performance.mddocs/investigations/2026-06-pa-unroll-207001-optimeout-window.mddocs/troubleshooting/sim-oversubscription-hang.mdsrc/a2a3/runtime/tensormap_and_ringbuffer/aicore/aicore_executor.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/runtime.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cppsrc/a5/runtime/tensormap_and_ringbuffer/aicore/aicore_executor.cppsrc/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cppsrc/a5/runtime/tensormap_and_ringbuffer/docs/RUNTIME_LOGIC.mdsrc/a5/runtime/tensormap_and_ringbuffer/runtime/runtime.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_context.h
Collapse the AICore init handshake from a three-phase ping-pong
(aicore_regs_ready -> aicpu_regs_ready -> aicore_done) to a single round
trip. AICore now publishes {physical_core_id, core_type, aicore_done} in
one write, then polls its own DATA_MAIN_BASE SPR. The AICPU opens the
core's register window (platform_init_aicore_regs writes
DATA_MAIN_BASE=IDLE) only after observing aicore_done, so that
window-open write doubles as the release acknowledgement -- the dedicated
aicpu_regs_ready / aicore_regs_ready fields are removed from struct
Handshake.
a2a3 (already parallel via hw-native-sys#1279): within each thread's core slice,
sweep-poll on aicore_done -- a GM read, not the strictly-serial nGnRE
COND poll -- so per-core wakeups overlap (~max, not sum), and batch the
release barrier (one OUT_OF_ORDER_STORE_BARRIER per slice instead of one
per core).
a5: port the parallel-handshake split (pre_handshake_init /
handshake_partition / post_handshake_init) alongside the single-round-trip
protocol; a5 previously handshaked serially on one thread.
Reproduces the approach of upstream PR hw-native-sys#1214 on a fresh branch off main.
Verified a2a3 onboard (device 3): paged_attention Case1 200/200 golden
rounds, zero mismatches; preamble median 27us vs 50us baseline (-46%, device 3, CaseSmall1,
n=100). a5 compiles clean
(onboard + sim); a5 onboard testing deferred to CI.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MqeALZTPEnDXTnbYfcnPfq
9bcfca8 to
91055f0
Compare
…+ single-round-trip handshake (hw-native-sys#1310) Rebasing the polling-scheduler branch onto upstream/main pulled in three runtime changes the branch predates. Two needed no action — hw-native-sys#1308 (MIX per-core placement) patches classify_mix_cluster / prepare_block_for_dispatch, both removed by the polling redesign, and hw-native-sys#1294 only relocates tests. The other two require reconciliation, done here: - hw-native-sys#1268 renamed the profiling/DFX/timeout config macros to the SIMPLER_ prefix and removed the PTO2_ spellings. Update the three surviving references in redesigned code: PTO2_ORCH_PROFILING/PTO2_SCHED_PROFILING, PTO2_TENSORMAP_PROFILING, and PTO2_PROFILING -> SIMPLER_DFX. - hw-native-sys#1310 collapsed the AICore init handshake to a single round trip and dropped Handshake::aicore_regs_ready / aicpu_regs_ready. The AICore side (aicore_executor.cpp) and the Handshake struct are upstream-clean after the rebase, but the AICPU driver kept in the redesign (SchedulerContext::handshake_partition / emergency_shutdown, moved from scheduler_cold_path.cpp into scheduler_context.h) still ran the old three-phase protocol against the deleted fields. Port the single-round-trip transform: batch the Step 1 release barrier, sweep-poll aicore_done in Step 2 with platform_init_aicore_regs (DATA_MAIN_BASE=IDLE) doubling as the window-open acknowledgement, and drop the aicpu_regs_ready write from emergency shutdown. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gbuffer hbg's scheduler was a copy of tmr@fb28fd71 and had fallen a full generation behind. This absorbs tmr's post-fork scheduler evolution while preserving hbg's host-orchestration invariants (single ring, no execution-time reclaim, flat Runtime, host-side orchestrator). Absorbed from tmr: - Remove PTO2LocalReadyBuffer local-first dispatch and its local_bufs threading (hw-native-sys#1245); pop/dispatch now take PTO2ReadyQueue* directly. - Ready-claim completion protocol: ready_state / mark_completed / mark_any_subtask_deferred (replaces the any_subtask_deferred bool). - Rename speculative -> early_dispatch and remove the auto-chain (spec_chain_active/depth, PTO2_SPEC_CHAIN_MAX); move allow_early_resolve from payload onto slot_state. - Per-shape early_dispatch_queues[] (was a single queue) + ready_sync_queues[] Tier-0 sync_start staging (hw-native-sys#1319); run_staging_order template; try_early_dispatch / early_dispatch_shape; has_any_free_slot predicates. - MIX subtasks placed per-core instead of uniform cluster placement (hw-native-sys#1308). - Remove the on-device wiring subsystem (PTO2SpscQueue / WiringState / drain_wiring_queue / wire_task, hw-native-sys#1263): the host orchestrator now wires fanout adjacency inline during submit and seeds readiness via the scheduler's push_ready_routed / route_ready_once, with no device-side wiring queue. - Single-round-trip / parallel AICore handshake (hw-native-sys#1279/hw-native-sys#1310): handshake_all_cores -> pre_handshake_init + per-thread handshake_partition + post_handshake_init, with the matching aicpu_executor boot restructure and AICore-worker protocol. - Drop the per-run per-core memset in scheduler init/deinit (hw-native-sys#1212); gate dep_gen init under SIMPLER_DFX; compile out pmu_active when SIMPLER_DFX is off; remove the redundant second completion poll; remove the dead device-orch core transition (handle_core_transition / reassign_cores_for_all_threads / orch_to_sched_). Remove vestigial device-orch synchronization: with host orchestration the orchestrator runs to completion on the host before the device boots, so the device-side orchestrator_done_ flag is always set before the scheduler threads start (they gate on runtime_init_ready_, whose release/acquire already publishes total_tasks_). Removed orchestrator_done_, the always-true completion guard, and the never-called wait_for_orchestration_done_before_dispatch / orchestration_done() carried over from tmr's concurrent device-orch model. Preserved host-orch divergences (not overwritten by tmr): PTO2_MAX_RING_DEPTH==1 with rings[]/ring_id removed; no advance_ring_pointers / reset_for_reuse / check_and_handle_consumed (completion via completed_tasks_, consumer wait keys on fanout_refcount); flat runtime->workers; hbg's swimlane behavior (no on_aicore_ack hook — the just-filled buffer drains via the completion-before-dispatch invariant and the next-rotation / run-end backstop flushes). Skipped as not applicable to hbg: stall-classification publishing (shared-memory lacks the sched_stall_* fields; last_task_alive semantics differ); per-device scheduler timeout (aicpu_device_config.h / get_scheduler_timeout_ms absent); runtime->dev.* Runtime nesting. get_reg_ptr weak host fallback: route_ready_once transitively ODR-uses the early-dispatch doorbell inline (ring_one_doorbell -> get_reg_ptr) in the host lib, but no core is gated during host graph-build so it never fires; a weak hidden fallback satisfies the loader (same pattern as get_sys_cnt_aicpu). Verified: all 8 runtime targets build -Werror; a2a3sim host_build_graph scene suite 10/10; a2a3 onboard host_build_graph suite 10 passed / 1 skipped (paged_attention exercises the new handshake, route_ready_once wiring, and ready-claim path). tensormap_and_ringbuffer and the a5 runtimes are untouched.
…gbuffer hbg's scheduler was a copy of tmr@fb28fd71 and had fallen a full generation behind. This absorbs tmr's post-fork scheduler evolution while preserving hbg's host-orchestration invariants (single ring, no execution-time reclaim, flat Runtime, host-side orchestrator). Absorbed from tmr: - Remove PTO2LocalReadyBuffer local-first dispatch and its local_bufs threading (hw-native-sys#1245); pop/dispatch now take PTO2ReadyQueue* directly. - Ready-claim completion protocol: ready_state / mark_completed / mark_any_subtask_deferred (replaces the any_subtask_deferred bool). - Rename speculative -> early_dispatch and remove the auto-chain (spec_chain_active/depth, PTO2_SPEC_CHAIN_MAX); move allow_early_resolve from payload onto slot_state. - Per-shape early_dispatch_queues[] (was a single queue) + ready_sync_queues[] Tier-0 sync_start staging (hw-native-sys#1319); run_staging_order template; try_early_dispatch / early_dispatch_shape; has_any_free_slot predicates. - MIX subtasks placed per-core instead of uniform cluster placement (hw-native-sys#1308). - Remove the on-device wiring subsystem (PTO2SpscQueue / WiringState / drain_wiring_queue / wire_task, hw-native-sys#1263): the host orchestrator now wires fanout adjacency inline during submit and seeds readiness via the scheduler's push_ready_routed / route_ready_once, with no device-side wiring queue. - Single-round-trip / parallel AICore handshake (hw-native-sys#1279/hw-native-sys#1310): handshake_all_cores -> pre_handshake_init + per-thread handshake_partition + post_handshake_init, with the matching aicpu_executor boot restructure and AICore-worker protocol. Drop the now-dead two-round-trip Handshake fields aicpu_regs_ready / aicore_regs_ready (aligning the struct to tmr's). - Drop the per-run per-core memset in scheduler init/deinit (hw-native-sys#1212); gate dep_gen init under SIMPLER_DFX; compile out pmu_active when SIMPLER_DFX is off; remove the redundant second completion poll; remove the dead device-orch core transition (handle_core_transition / reassign_cores_for_all_threads / orch_to_sched_). Remove vestigial device-orch synchronization: with host orchestration the orchestrator runs to completion on the host before the device boots, so the device-side orchestrator_done_ flag is always set before the scheduler threads start (they gate on runtime_init_ready_, whose release/acquire already publishes total_tasks_). Removed orchestrator_done_, the always-true completion guard, and the never-called wait_for_orchestration_done_before_dispatch / orchestration_done() carried over from tmr's concurrent device-orch model. Preserved host-orch divergences (not overwritten by tmr): PTO2_MAX_RING_DEPTH==1 with rings[]/ring_id removed; no advance_ring_pointers / reset_for_reuse / check_and_handle_consumed (completion via completed_tasks_, consumer wait keys on fanout_refcount); flat runtime->workers; hbg's swimlane behavior (no on_aicore_ack hook — the just-filled buffer drains via the completion-before-dispatch invariant and the next-rotation / run-end backstop flushes). Skipped as not applicable to hbg: stall-classification publishing (shared-memory lacks the sched_stall_* fields; last_task_alive semantics differ); per-device scheduler timeout (aicpu_device_config.h / get_scheduler_timeout_ms absent); runtime->dev.* Runtime nesting. get_reg_ptr weak host fallback: route_ready_once transitively ODR-uses the early-dispatch doorbell inline (ring_one_doorbell -> get_reg_ptr) in the host lib, but no core is gated during host graph-build so it never fires; a weak hidden fallback satisfies the loader (same pattern as get_sys_cnt_aicpu). Verified: all 8 runtime targets build -Werror; a2a3sim host_build_graph scene suite 10/10; a2a3 onboard host_build_graph suite 10 passed / 1 skipped (paged_attention exercises the new handshake, route_ready_once wiring, and ready-claim path). tensormap_and_ringbuffer and the a5 runtimes are untouched.
…gbuffer (#1327) hbg's scheduler was a copy of tmr@fb28fd71 and had fallen a full generation behind. This absorbs tmr's post-fork scheduler evolution while preserving hbg's host-orchestration invariants (single ring, no execution-time reclaim, flat Runtime, host-side orchestrator). Absorbed from tmr: - Remove PTO2LocalReadyBuffer local-first dispatch and its local_bufs threading (#1245); pop/dispatch now take PTO2ReadyQueue* directly. - Ready-claim completion protocol: ready_state / mark_completed / mark_any_subtask_deferred (replaces the any_subtask_deferred bool). - Rename speculative -> early_dispatch and remove the auto-chain (spec_chain_active/depth, PTO2_SPEC_CHAIN_MAX); move allow_early_resolve from payload onto slot_state. - Per-shape early_dispatch_queues[] (was a single queue) + ready_sync_queues[] Tier-0 sync_start staging (#1319); run_staging_order template; try_early_dispatch / early_dispatch_shape; has_any_free_slot predicates. - MIX subtasks placed per-core instead of uniform cluster placement (#1308). - Remove the on-device wiring subsystem (PTO2SpscQueue / WiringState / drain_wiring_queue / wire_task, #1263): the host orchestrator now wires fanout adjacency inline during submit and seeds readiness via the scheduler's push_ready_routed / route_ready_once, with no device-side wiring queue. - Single-round-trip / parallel AICore handshake (#1279/#1310): handshake_all_cores -> pre_handshake_init + per-thread handshake_partition + post_handshake_init, with the matching aicpu_executor boot restructure and AICore-worker protocol. Drop the now-dead two-round-trip Handshake fields aicpu_regs_ready / aicore_regs_ready (aligning the struct to tmr's). - Drop the per-run per-core memset in scheduler init/deinit (#1212); gate dep_gen init under SIMPLER_DFX; compile out pmu_active when SIMPLER_DFX is off; remove the redundant second completion poll; remove the dead device-orch core transition (handle_core_transition / reassign_cores_for_all_threads / orch_to_sched_). Remove vestigial device-orch synchronization: with host orchestration the orchestrator runs to completion on the host before the device boots, so the device-side orchestrator_done_ flag is always set before the scheduler threads start (they gate on runtime_init_ready_, whose release/acquire already publishes total_tasks_). Removed orchestrator_done_, the always-true completion guard, and the never-called wait_for_orchestration_done_before_dispatch / orchestration_done() carried over from tmr's concurrent device-orch model. Preserved host-orch divergences (not overwritten by tmr): PTO2_MAX_RING_DEPTH==1 with rings[]/ring_id removed; no advance_ring_pointers / reset_for_reuse / check_and_handle_consumed (completion via completed_tasks_, consumer wait keys on fanout_refcount); flat runtime->workers; hbg's swimlane behavior (no on_aicore_ack hook — the just-filled buffer drains via the completion-before-dispatch invariant and the next-rotation / run-end backstop flushes). Skipped as not applicable to hbg: stall-classification publishing (shared-memory lacks the sched_stall_* fields; last_task_alive semantics differ); per-device scheduler timeout (aicpu_device_config.h / get_scheduler_timeout_ms absent); runtime->dev.* Runtime nesting. get_reg_ptr weak host fallback: route_ready_once transitively ODR-uses the early-dispatch doorbell inline (ring_one_doorbell -> get_reg_ptr) in the host lib, but no core is gated during host graph-build so it never fires; a weak hidden fallback satisfies the loader (same pattern as get_sys_cnt_aicpu). Verified: all 8 runtime targets build -Werror; a2a3sim host_build_graph scene suite 10/10; a2a3 onboard host_build_graph suite 10 passed / 1 skipped (paged_attention exercises the new handshake, route_ready_once wiring, and ready-claim path). tensormap_and_ringbuffer and the a5 runtimes are untouched.
Human Summary
Currently, the AICPU cores are initializing AI vector and cube cores by performing a 3-step handshaking procedure. This PR takes the ideas of #1214 by @ChaoWao and adapts it to the current main, where the handshaking is already parallelized (see: #1279). The main benefits:
As with #1279, this optimization benefits the short/coarsed-grained-task kernels the most, reporting from 1.7% to 8.7% speedup on simpler benchmark set.
Note: I don't have a A5 machine for development, so I'm counting on the CI to check this works.
AI Summary
tl;dr for humans: The AICore init handshake currently costs three
round-trips per core between the AICPU and each of the 72 AICores. This PR
collapses it to one by folding the "registers are ready" acknowledgement
into the register-window-open write that already had to happen. Two handshake
fields are deleted, a5 gets the parallel-handshake split it was missing, and
the per-core preamble shrinks. Net effect: every benchmark case runs faster
on Device (−1.7% to −8.7%, median ≈ −5%) with zero correctness regressions
(200/200 golden rounds). It's a pure preamble/latency win — orchestration and
scheduling are untouched.
What changes
Collapse the AICore init handshake from a three-phase ping-pong
(
aicore_regs_ready→aicpu_regs_ready→aicore_done) to a single roundtrip:
Single-round-trip protocol. AICore publishes
{physical_core_id, core_type, aicore_done}in one write, then polls its ownDATA_MAIN_BASESPR. The AICPU opens that core's register window(
platform_init_aicore_regswritesDATA_MAIN_BASE = AICPU_IDLE_TASK_ID,non-zero) only after observing
aicore_done— so the window-open writedoubles as the release acknowledgement. The dedicated
aicpu_regs_ready/aicore_regs_readyfields are removed fromstruct Handshake.Sweep polling (a2a3, already parallel across AICPU threads). Within each
thread's core slice, poll every outstanding core per pass on
aicore_done(a GM read — not the strictly-serial nGnRE
CONDMMIO poll) and servicewhichever have reported, so per-core wakeups overlap (≈ max, not Σ) instead
of blocking core
ibefore looking at corei+1.Batched release barrier. One
OUT_OF_ORDER_STORE_BARRIERper sliceinstead of one per core.
a5. Gains the parallel-handshake split (
pre_handshake_init/handshake_partition/post_handshake_init) for the first time — a5previously handshaked serially on one thread — plus the single-round-trip
protocol.
This is not work-pipelining: the handshake is still a full barrier before
dispatch. It just makes that barrier cheaper.
Why it's safe
The single-round-trip rests on a silicon-timing invariant: a kernel launch
resets
DATA_MAIN_BASEto 0; the AICPU writes it non-zero to open FAST_PATH;and the AICPU spends microseconds in
assign_cores_to_threadsbetween openingthe window and the first dispatch, so the core's poll observes the IDLE value
long before any real
task_id— it cannot miss it. Verified below with a heavygolden run.
Verification (a2a3 onboard)
Correctness —
paged_attention Case1, 200/200 golden rounds, 0mismatches (golden checked per round).
Performance — 8-case set, 100 rounds
--skip-golden, back-to-back on thesame device. Device µs (the metric;
Effective/Orch/Schedstay flatwithin noise because they exclude the preamble):
The delta is a near-constant ~45–74µs across all workloads — the signature of a
fixed per-round preamble reduction (larger % on smaller workloads). Preamble
span itself: median 50µs → 27µs (−46%).
a5: compiles clean (onboard + sim); a5 onboard testing deferred to CI
(no a5 silicon on the dev box used).
Files
runtime.h(struct),aicore/aicore_executor.cpp,runtime/scheduler/scheduler_cold_path.cppruntime.h,aicore/aicore_executor.cpp,aicpu/aicpu_executor.cpp,runtime/scheduler/scheduler_cold_path.cpp,scheduler_context.h, docsNotes for reviewers
task-submiton the box), so thesub-2% cases (batch) are "improved but within noise"; the overall direction —
every case faster, constant-offset — is unambiguous. Worth a CI-locked re-run.
emergency_shutdownno longer broadcastsaicpu_regs_ready; exit is nowdelivered via each core's register block (
platform_deinit_aicore_regswritesDATA_MAIN_BASE = EXIT), which also releases a core still polling for itswindow. This path is only hit on handshake failure and isn't covered by the
golden run — reviewed by inspection.
🤖 Generated with Claude Code