Refactor: split PTO2RuntimeArenaLayout into sizing + offsets#1219
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthrough
ChangesPTO2RuntimeArenaLayout struct split and consumer updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
![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 refactors the runtime-arena layout and initialization logic across the a2a3 and a5 platforms, splitting PTO2RuntimeArenaLayout into separate sizing and offsets structures and modularizing bind_callable_to_runtime_impl into distinct helper functions. Feedback on these changes highlights the need to prevent signed integer overflow by accumulating window sizes using a 64-bit integer before casting to int32_t in pto_runtime2_init.cpp. Additionally, it is recommended to validate that the signed integer sig_count is non-negative in stage_device_args to avoid potential out-of-bounds array indexing.
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
refactor/split-arena-layout-sizing-offsets
branch
from
6d84f7b to
7c3060d
Compare
PTO2RuntimeArenaLayout mixed two semantics in one flat struct: the
layout-defining capacities (input) and the computed sub-region offsets
(output). Split into two named halves so each side reads as what it is:
- ArenaSizingKey: task_window/heap/dep_pool sizes + scheduler_timeout_ms
— the input to runtime_reserve_layout, re-read at AICPU boot
- ArenaOffsets: off_sm_handle/orch/sched/off_runtime/off_mailbox +
arena_size — the output, consumed by init_data + wire (the AICPU
re-wires arena-internal pointers from these after rtMemcpy)
PTO2RuntimeArenaLayout now composes the two as `.sizing` / `.offsets`.
All field-access sites are updated across both arches: pto_runtime2.h,
pto_runtime2_init.cpp (the three RuntimeArenaLayout functions only — the
PTO2SchedulerLayout/PTO2OrchestratorLayout siblings have same-named
fields and are deliberately left untouched), aicpu_executor.cpp,
scheduler_dispatch.cpp, runtime_maker.cpp, and the cpp unit test
tests/ut/cpp/{a2a3,a5}/test_shared_memory.cpp.
This is a host<->device ABI change: the layout is embedded in
PTO2Runtime::prebuilt_layout, rtMemcpy'd to device, and re-read at AICPU
boot, so the field reorder must be validated on hardware (sim shares one
address space and cannot catch an offset drift).
Verified: a2a3sim 30 passed/1 skipped, a5sim 20 passed, a2a3 onboard
33 passed/1 skipped, cpp UT test_shared_memory compiles clean. a5 onboard
covered by CI (this box is a2a3 silicon).
ChaoWao
force-pushed
the
refactor/split-arena-layout-sizing-offsets
branch
from
7c3060d to
46431c5
Compare
1 participant
Summary
PTO2RuntimeArenaLayoutmixed two semantics in one flat struct: the layout-defining capacities (input) and the computed sub-region offsets (output). This splits it into two named halves so each side reads as what it is:ArenaSizingKey—task_window_sizes/heap_sizes/dep_pool_capacities+scheduler_timeout_ms. The input toruntime_reserve_layout, re-read at AICPU boot.ArenaOffsets—off_sm_handle/orch/sched/off_runtime/off_mailbox+arena_size. The output, consumed byinit_data_from_layout+wire_arena_pointers(the AICPU re-wires arena-internal pointers from these after rtMemcpy).PTO2RuntimeArenaLayoutnow composes the two as.sizing/.offsets. All field-access sites are updated across both arches:pto_runtime2.h,pto_runtime2_init.cpp,aicpu_executor.cpp,scheduler_dispatch.cpp,runtime_maker.cpp.The
PTO2SchedulerLayout/PTO2OrchestratorLayoutsibling structs have same-named fields (task_window_sizes,dep_pool_capacities) and are deliberately left untouched — only the threePTO2RuntimeArenaLayoutfunctions were migrated.The layout is embedded in
PTO2Runtime::prebuilt_layout, rtMemcpy'd to device, and re-read at AICPU boot — so this field reorder is a host↔device ABI change. Sim cannot catch an offset drift (host + device share one address space); only onboard validates it.Stacked on #1215
Depends on #1215 (B1: bind_callable lifecycle split) — B2's
runtime_maker.cppedits land inside the helpers #1215 introduces. The diff will narrow to this commit once #1215 merges.Testing