Refactor: holistic L2 perf type naming + drop dead fields#916
Refactor: holistic L2 perf type naming + drop dead fields#916hw-native-sys-bot wants to merge 1 commit into
Conversation
|
Warning Review limit reached
More reviews will be available in 10 minutes and 46 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
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 performs a comprehensive renaming of the L2PerfBufferState structure to AicpuBufferState across the codebase, including documentation, headers, and source files for both the a2a3 and a5 platforms. This refactoring improves semantic clarity by explicitly associating the buffer state with the AICPU. No review comments were provided, and there is no additional feedback to offer on these changes.
A holistic pass over the L2 perf type names in src/{a2a3,a5}/platform/.
Renames consolidate the misleading parts and drop two fields that have
been dead since hw-native-sys#878 / hw-native-sys#921. No on-device behaviour change beyond the
field removals.
== Step 1: buffer name symmetry ==
L2PerfBuffer -> L2PerfAicpuBuffer
PhaseBuffer -> AicpuPhaseBuffer
The producer is now in the name: AICPU-written buffers carry `Aicpu`,
mirroring the existing `L2PerfAicoreBuffer` for the AICore-written one.
On a2a3, AicpuPhaseBuffer also moves from a hand-written struct to the
existing `TypedBuffer<R, N>` template -- matches L2PerfAicpuBuffer and
L2PerfAicoreBuffer.
== Step 4: pool struct rename ==
AicpuBufferState -> L2PerfAicpuPool
L2PerfAicoreBufferState -> L2PerfAicorePool
PhaseBufferState -> AicpuPhasePool (alias of L2PerfAicpuPool)
Replaces the misleading `BufferState` suffix (it was a *pool* state
holding free_queue + counters, not a buffer) and gives the Phase usage
its own explicit type name even though it shares layout with
L2PerfAicpuPool.
== Step 2: BufferKind enum ==
ReadyQueueEntry::is_phase (uint32_t) -> kind (BufferKind, uint32_t underlying)
enum class BufferKind : uint32_t {
L2PerfAicpu = 0,
Phase = 1,
L2PerfAicore = 2,
};
The field was already a 3-value discriminator (despite the bool-shaped
name); enqueue sites and the host's switch site now read in terms of
the named enum instead of magic numbers. uint32_t underlying preserves
the byte layout -- no ABI break.
== Step 3: drop dead fields (a2a3 only) ==
L2PerfAicpuPool::aicore_ring_ptr -- removed
L2PerfAicpuPool::mismatch_record_count -- removed
Both fields have been dead since:
- hw-native-sys#878 moved AICore writes to a per-core L2PerfAicoreBuffer pool with
its own state (L2PerfAicorePool), so `aicore_ring_ptr` no longer
points anywhere live.
- hw-native-sys#921 moved the rotation-table fill from host to AICPU, removing the
last code path that even read `aicore_ring_ptr` (host's flush join
now indexes the AICore pool directly).
- `mismatch_record_count` was last written before hw-native-sys#878 (legacy ring/
task_id invariant violation) and is no longer written by anyone;
the host reconcile() loop's reading-and-warning on a counter that
can never advance is removed.
Pad is adjusted to keep `sizeof(L2PerfAicpuPool) == 192` (static_assert
remains green). Host reconcile() loop simplifies to `total/dropped` only.
a5 keeps both fields -- its AICore pipeline still uses the legacy
staging-ring design where the host's per-core `aicore_ring_ptr` and the
mismatch counter are both live.
== Step 5: deferred ==
Collapsing AicpuPhaseHeader into the root L2PerfDataHeader is a
shared-memory layout change and is left for a focused follow-up PR.
== Test plan ==
- All four platform variants (a2a3, a2a3sim, a5, a5sim) build clean
- tests/st/.../l2_swimlane pass on a2a3sim
- Same ST passes on a2a3 device 1
- a5sim smoke (spmd_basic) passes
hw-native-sys-bot
force-pushed
the
refactor/aicpu-buffer-state-rename
branch
from
ea8b37e to
3e368b4
Compare
hw-native-sys-bot
changed the title
2 participants
Summary
A holistic pass over the L2 perf type names in
src/{a2a3,a5}/platform/.Renames consolidate the misleading parts and drop two fields that have
been dead since #878 / #921. No on-device behaviour change beyond the
field removals.
Replaces the earlier single-name
L2PerfBufferState→AicpuBufferStaterename — that landing as the final name had less explanatory power than
the holistic
L2PerfAicpuPoolchosen here.Step 1: buffer name symmetry
L2PerfBufferL2PerfAicpuBufferPhaseBufferAicpuPhaseBufferProducer is now in the name: AICPU-written buffers carry
Aicpu,mirroring the existing
L2PerfAicoreBufferfor the AICore-written one.On a2a3,
AicpuPhaseBufferalso switches from a hand-written struct tothe existing
TypedBuffer<R, N>template — matches the other two.Step 4: pool struct rename
AicpuBufferStateL2PerfAicpuPoolL2PerfAicoreBufferStateL2PerfAicorePoolPhaseBufferStateAicpuPhasePool(alias ofL2PerfAicpuPool)Replaces the
BufferStatesuffix (it was a pool state holdingfree_queue+ counters, not a buffer) and gives the Phase usage itsown explicit type name even though it shares layout with
L2PerfAicpuPool.Step 2:
BufferKindenumReadyQueueEntry::is_phase(uint32_t) →kind(BufferKind, uint32_tunderlying):
The field was already a 3-value discriminator (despite the bool-shaped
name); enqueue sites and the host's switch site now read in terms of
the named enum instead of magic numbers.
uint32_tunderlying preservesthe byte layout — no ABI break.
Step 3: drop dead fields (a2a3 only)
L2PerfAicpuPool::aicore_ring_ptr— removedL2PerfAicpuPool::mismatch_record_count— removedBoth fields have been dead since:
L2PerfAicoreBufferpoolwith its own state (
L2PerfAicorePool), soaicore_ring_ptrnolonger points anywhere live.
the last code path that even read
aicore_ring_ptr(host's flushjoin now indexes the AICore pool directly).
mismatch_record_countwas last written before Refactor: AICore-as-producer for L2 swimlane (skip per-task staging read) #878 (legacyring/task_id invariant violation) and is no longer written by anyone;
the host
reconcile()loop's reading-and-warning on a counter thatcan never advance is removed.
Pad is adjusted to keep
sizeof(L2PerfAicpuPool) == 192(static_assertremains green). Host
reconcile()loop simplifies tototal/droppedonly.
a5 keeps both fields — its AICore pipeline still uses the legacy
staging-ring design where the host's per-core
aicore_ring_ptrand themismatch counter are both live.
Step 5: deferred
Collapsing
AicpuPhaseHeaderinto the rootL2PerfDataHeaderis ashared-memory layout change and is left for a focused follow-up PR.
Test plan
a2a3,a2a3sim,a5,a5sim) build cleantests/st/.../l2_swimlanepass ona2a3sima2a3device 1a5simsmoke (spmd_basic) passesScope
16 files, ~700-line diff (mostly mechanical renames). No behaviour
change beyond the dead-field removal on a2a3.