Fix: ACK-gate L2 swimlane rotation after moving AICore FIN early#1293
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 introduces an AICPU-side ACK gate for L2 swimlane AICore buffer rotation, deferring release of a rotated buffer until an observed ACK confirms the prior tail record's drain via a reg_task_id token threaded through dispatch and completion hooks. It also reorders AICore executor timestamp/FIN sequencing and adds two investigation documents plus an index update. ChangesACK-Gated Swimlane Rotation
Estimated code review effort: 4 (Complex) | ~60 minutes 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements an AICPU-side ACK-gate on buffer rotation and reorders operations in the AICore executor to sample end_time after an early FIN write, which decouples the record write from the completion-signal path. It also adds detailed investigation documents explaining these design choices and why other micro-optimizations were dropped. The reviewer identified a critical issue in l2_swimlane_collector_aicpu.cpp where the static global array s_aicore_pending_enqueue is not reset during initialization, which can lead to state leakage and memory corruption across runs on the AICPU platform.
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.
ChaoWao
force-pushed
the
l2-swimlane-fin-early-ack-gate
branch
4 times, most recently
from
dae7f24 to
fb6eb0c
Compare
Move the tensormap_and_ringbuffer AICore FIN write ahead of the swimlane
record (order: ACK -> pmu_begin -> start -> execute -> FIN -> end ->
pmu_end -> dump -> record) so completion is signalled before the ~0.5us
record write-back, and gate the three swimlane timestamps on
l2_swimlane_enabled.
FIN now precedes the record's dcci+dsb, so the completion-before-dispatch
invariant no longer proves the old buffer's tail record has drained at a
BUFFER_SIZE rotation. AICPU therefore stops enqueueing the just-filled
buffer at rotation: it stashes the buffer and releases it to the host
only when AICore ACKs the new buffer's first task (that ACK is emitted
after the previous task's record dcci+dsb, proving it drained).
host_build_graph writes the record before FIN and has no such window; it
shares the rotate but relies on the next-rotation / run-end backstop.
- reorder + timestamp gating: a2a3 tmr aicore_executor.cpp
- ACK-gate: shared l2_swimlane_collector_aicpu.{cpp,h}, new
l2_swimlane_aicpu_on_aicore_ack wired into the a2a3 tmr
scheduler_completion path; the deferred-enqueue stash is reset in
l2_swimlane_aicpu_init so it cannot leak a dangling buffer across launches
- on_aicore_dispatch gains the gate reg_task_id; all callers updated
(a2a3 + a5, tmr scheduler_dispatch + hbg aicpu_executor) and the a5
aicpu collector header brought in line with the shared signature
- docs/investigations: switch-overhead decomposition (the gap is mostly
profiling self-cost - get_sys_cnt reads + record write-back)
Verified: a2a3 L2 swimlane ST smoke + qwen3_14b_decode level 4 pass on
silicon; a2a3sim + a5 sim/plain swimlane smokes pass; a2a3/a5 runtimes
and sims rebuild clean. The boundary path (>1024 tasks/core) is not
exercised by these workloads.
1 participant
Summary
FINwrite ahead of the swimlane record so op order isACK → pmu_begin → start → execute → FIN → end → pmu_end → dump → record. Completion is now signalled before the ~0.5 µs record write-back, and the three swimlane timestamps are gated onl2_swimlane_enabled(zero counter reads when profiling is off).FINnow precedes the record'sdcci+dsb, the completion-before-dispatch invariant no longer proves the old buffer's tail record has drained at aBUFFER_SIZErotation. At rotation AICPU now only publishes the new buffer and stashes the just-filled one; it releases the buffer to the host only when AICore ACKs the new buffer's first task (reg_task_id == gate). By AICore's single-threaded program order that ACK is emitted only after the previous task'srecord + dsb, so the tail record is proven drained. Newl2_swimlane_aicpu_on_aicore_ackwired into the tmr completion path;on_aicore_dispatchgains the gatereg_task_id.FIN, so it has no such window — it shares the rotate but relies on the next-rotation / run-end backstop (no ACK hook wired). Its dispatch caller is updated for the new signature.get_sys_cntreads + the record write-back — not a real hardware task switch; the WAIT gap p99 ~700 µs dominates decode) plus the previously-dropped head-poll micro-opt writeup.Why
The ~1 µs inter-task overhead that only shows up on the swimlane (not in early dispatch) is the profiling instrumentation itself. Moving
FINearly lets AICPU observe completion ~0.5 µs sooner (before the record write) — but that reorder makesFIN-gated buffer rotation unsafe, so the ACK-gate is required to avoid publishing an undrained tail record (which the host would otherwise discard asstart_time == 0).Testing
--enable-l2-swimlane 4 --enable-dep-gen)qwen3_14b_decodelevel 4 passes on a2a3 silicon