Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a prebuilt-arena fast path for the PTO2 runtime, allowing the host to pre-compute the runtime arena image and upload it to the device. This optimization reduces AICPU boot time by replacing full initialization with a simple attachment and pointer "wiring" phase. Key changes include refactoring the initialization logic for the runtime, orchestrator, and scheduler into separate data-population and pointer-wiring stages, extending the DeviceRunner to manage a pooled runtime arena, and adding an attach method to DeviceArena for externally-owned buffers. Review feedback correctly identified potential undefined behavior in the new acquire_pooled_runtime_arena methods when the arena is not provisioned, suggesting defensive checks against SIZE_MAX offsets.
Address review feedback from PR hw-native-sys#846: - pto2_sm_layout::ring_task_descriptors_addr: take per-ring task_window_sizes[] array (mirroring PTO2SharedMemoryHandle's SM API) and assert ring_id range, so a future per-ring SM layout cannot silently disagree with the addresses the host bakes into the prebuilt image. - DeviceRunner::acquire_pooled_runtime_arena (onboard + sim): return nullptr when runtime_arena_region_off_ == SIZE_MAX so a stray hbg-path call cannot resolve to base + SIZE_MAX. Failure is now loud and contained at the acquire boundary. - DeviceArena::attach(): rewrite doc to match real behavior (region table is not repopulated after attach, reserve() asserts !committed_ so cannot replay, region_size() returns 0); promote the pre-alignment / non-null / power-of-two checks from plain assert() to an unconditional abort() so release builds still trap on contract violations. - PTO2TensorMap: drop the dead `orch` back-pointer field (a2a3 never dereferences it), strip parent_orch parameter from wire_arena_pointers, and remove the now-unused PTO2OrchestratorState forward declaration. - PTO2RingFlowControl::init(): add a coupling comment so future fc-initial- value or boot-order changes flag PTO2TaskAllocator::init's initial_local_task_id default in the same edit. - PTO2SchedulerState::init_data_from_layout / RingSchedState:: init_data_from_layout: drop the task_window_size / dep_pool_capacity parameters that were never consumed (scheduler only needs SM base + ring index, both window-size-independent; orchestrator counterpart still takes task_window_size for ring_task_descriptors arithmetic). Updated all callsites (pto_runtime2_init.cpp + 4 cpput suites). - PTO2Runtime::prebuilt_arena_base: removed the dead mirror field. The host Runtime's prebuilt_arena_base_ is the real source of truth (AICPU reads it to locate the pooled buffer *before* dereferencing the image); the PTO2Runtime image still carries prebuilt_layout, which the AICPU does consume. cpput: 25/25 pass. a2a3sim trb: dummy_task / dynamic_register / L2 trb suite pass with --build.
Sync of PR hw-native-sys#846 commit 2/3 to a5 — commit 1 (slot_state.bind split) was already mirrored. Brings the a5 trb runtime up to the same host-build arena fast path as a2a3. - 4-phase API (reserve_layout / init_data_from_layout / wire_arena_pointers / finalize_after_wire) replaces runtime_create_from_sm. - New runtime/shared/pto_runtime2_init.cpp (~355 lines) and shared/pto_tensormap.cpp (the old runtime/pto_tensormap.cpp moved + split) hold the host-pluggable cold-path lifted from pto_runtime2.cpp / pto_orchestrator.cpp / scheduler/pto_scheduler.cpp. - AICPU boot becomes attach + wire + sm_handle->init + finalize. - runtime_maker.cpp pre-builds the arena image on host and rtMemcpys it into a pooled runtime-arena region; onboard + sim DeviceRunner setup_static_arena grow a third runtime_arena_size argument with matching acquire_pooled_runtime_arena (hbg path passes 0). a5-specific divergences kept: enable_l2_swimlane (bool) instead of L2PerfLevel, no dep_gen subsystem, wait_init_complete naming, alignas(64) PTO2SpscQueue queue, cache_invalidate_range + cond.retire in async_wait, RUNTIME_MAX_WORKER 108. Tests - cpput: 25/25 pass. - a5sim: trb 21/21 + host_build_graph 6/6 pass. - a2a3sim regression: trb 29/29 + host_build_graph 9/9 pass.
|
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 a prebuilt runtime arena fast path for PTO2 initialization. It refactors device memory management from a single monolithic arena into three independent pooled regions (GM heap, GM SM, runtime) and restructures the runtime lifecycle from a single creation call into four distinct phases: layout reservation, data initialization, pointer wiring, and finalization. The changes span platform layers, runtime initialization, orchestrator/scheduler/tensormap components, and all supporting tests. ChangesDevice Arena Infrastructure & Three-Region Pooled Refactoring
Runtime Lifecycle Phase Split & Layout Descriptors
Centralized Shared Runtime Arena Initialization Implementation
Host-Side Prebuilt Arena Construction & Device-Side Attachment
Unit Test Updates for Two-Phase Initialization APIs
🎯 4 (Complex) | ⏱️ ~60 minutes
|
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/a2a3/platform/onboard/host/device_runner.h (1)
210-220: ⚡ Quick winThe pointer-stability contract is stronger than the implementation.
setup_static_arena()can re-commit a region when a later request grows, so that region'sbase()is not guaranteed to stay stable untilfinalize(). Please document that callers must reacquire any region that was successfully resized.🤖 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 `@src/a2a3/platform/onboard/host/device_runner.h` around lines 210 - 220, The current comment claiming that pointers from acquire_pooled_runtime_arena()/the pooled GM heap/PTO2 SM/runtime arena are stable until finalize() is incorrect because setup_static_arena() may re-commit (resize) a region and change its base(); update the documentation in device_runner.h so it clearly states that pointers returned by acquire_pooled_runtime_arena()/acquire_pooled_gm_heap()/acquire_pooled_pto2_sm() are stable only until a subsequent call to setup_static_arena() that grows or re-commits the region, and that callers must re-acquire the region (call the appropriate acquire_* function and use the returned base()) after any resize; mention the specific symbols setup_static_arena, acquire_pooled_runtime_arena, finalize, and base() to make the requirement obvious.src/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_shared_memory.h (1)
214-257: ⚡ Quick winPrefer
uintptr_tfor opaque device-address arithmetic.Lines 214-257 are doing raw device-address math via
char *. That works on today's targets, but it treats an opaque device pointer token as if it were a real host object pointer. Casting throughuintptr_tkeeps this as pure address arithmetic and avoids UB/sanitizer surprises in host builds.Suggested change
+inline std::uintptr_t sm_byte_addr(void *sm_dev_base, std::size_t off) noexcept { + return reinterpret_cast<std::uintptr_t>(sm_dev_base) + off; +} + inline std::atomic<int32_t> *orch_error_code_addr(void *sm_dev_base) noexcept { return reinterpret_cast<std::atomic<int32_t> *>( - static_cast<char *>(sm_dev_base) + offsetof(PTO2SharedMemoryHeader, orch_error_code) + sm_byte_addr(sm_dev_base, offsetof(PTO2SharedMemoryHeader, orch_error_code)) ); }🤖 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 `@src/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_shared_memory.h` around lines 214 - 257, The code does pointer arithmetic on sm_dev_base via char* which can be UB; change all address computations in orch_error_code_addr, ring_header_addr, ring_current_task_index_addr, ring_last_task_alive_addr, and ring_task_descriptors_addr to perform arithmetic on a uintptr_t base (e.g. uintptr_t base = reinterpret_cast<uintptr_t>(sm_dev_base)), add offsetof/size offsets to that integer, and only at the end reinterpret_cast the final integer (cast to void* first if needed) to the desired pointer type; ensure all uses of static_cast<char*> are replaced with integer arithmetic and final reinterpret_cast to preserve opaque device-address semantics.
🤖 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 `@src/a2a3/platform/onboard/host/device_runner.cpp`:
- Around line 290-300: The failure path in the device runner currently releases
previously committed peer arenas (gm_heap_arena_.release(),
gm_sm_arena_.release()) when commit_region(runtime_arena_pool_, ...) fails,
which can invalidate pooled pointers; change the error handling so that on a
failed commit of runtime_arena_pool_ you only undo or reset state related to
runtime_arena_pool_ and its cached_runtime_arena_size_ (do not release
gm_heap_arena_ or gm_sm_arena_ or touch
cached_gm_heap_size_/cached_gm_sm_size_), and conversely if
commit_region(gm_sm_arena_, ...) fails only undo gm_sm_arena_ related state;
keep commit_region successes independent so callers holding pooled pointers are
not invalidated by later failures.
In `@src/a2a3/platform/sim/host/device_runner.cpp`:
- Around line 1056-1065: The device buffer pointed to by device_wall_dev_ptr_
must be freed via free_tensor() (which uses mem_alloc_.free()) before calling
mem_alloc_.finalize(); move the free_tensor(device_wall_dev_ptr_) call so it
executes prior to mem_alloc_.finalize(), then reset device_wall_dev_ptr_ to
nullptr after freeing to avoid dangling pointers; update the same ordering
wherever the block around gm_heap_arena_.release()/runtime_arena_pool_.release()
and mem_alloc_.finalize() appears (the other 1067-1075 occurrence) to ensure no
free goes through the allocator after finalize.
In `@src/a2a3/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp`:
- Around line 284-285: The code narrows runtime->dep_pool_size (uint64_t) to
int32_t in eff_dep_pool_capacity without checking upper bounds; clamp the
selected value to int32_t range before casting. Replace the ternary cast with:
determine uint64_t chosen = runtime->dep_pool_size ? runtime->dep_pool_size :
PTO2_DEP_LIST_POOL_SIZE; clamp chosen = std::min(chosen,
static_cast<uint64_t>(std::numeric_limits<int32_t>::max())); then set
eff_dep_pool_capacity = static_cast<int32_t>(chosen); (ensure <limits> is
included). Use the symbols runtime->dep_pool_size, PTO2_DEP_LIST_POOL_SIZE, and
eff_dep_pool_capacity to locate the change.
In `@src/a5/platform/onboard/host/device_runner.cpp`:
- Around line 1006-1015: Move the release of device_wall_dev_ptr_ so it occurs
before the allocator shutdown (mem_alloc_.finalize()) and before the device
reset; specifically, locate device_wall_dev_ptr_ and call its release/cleanup
prior to the gm_heap_arena_.release()/mem_alloc_.finalize() sequence and before
the device reset call, and ensure free_tensor() invocations happen while the
device context and allocator are still valid (i.e., call free_tensor() or other
tensor cleanup before mem_alloc_.finalize() and device reset). Reference
symbols: device_wall_dev_ptr_, mem_alloc_.finalize(),
gm_heap_arena_.release()/gm_sm_arena_.release()/runtime_arena_pool_.release(),
free_tensor(), and the device reset invocation to reorder cleanup safely.
In `@src/a5/platform/sim/host/device_runner.cpp`:
- Around line 154-164: The second failure branch currently tears down
previously-committed peer arenas (gm_heap_arena_, gm_sm_arena_) which can leave
pooled pointers dangling; change the runtime_arena_pool_ failure handling so it
does NOT release gm_heap_arena_ or gm_sm_arena_. Specifically, in the
commit_region(runtime_arena_pool_, cached_runtime_arena_size_,
runtime_arena_size) != 0 branch, only undo/cleanup the runtime_arena_pool_
allocation (release or reset whatever was allocated for runtime_arena_pool_ and
set cached_runtime_arena_size_ = 0) and return -1, leaving gm_heap_arena_,
cached_gm_heap_size_, gm_sm_arena_, and cached_gm_sm_size_ intact; keep the
existing behavior in the first branch (commit_region(gm_sm_arena_, ...) failure)
as-is.
In `@tests/ut/cpp/a5/test_tensormap.cpp`:
- Around line 108-119: The test InitRequiresPowerOfTwoBuckets currently uses a
power-of-two bucket count (8) so it doesn't verify the rejection path; change it
to either (A) become an explicit success-path smoke test by renaming the test
(e.g., InitWithValidBucketsSucceeds) and keep the current assertions around
PTO2TensorMap::reserve_layout, bad.init_data_from_layout and
bad.wire_arena_pointers, or (B) restore the rejection check by using a
non-power-of-two bucket count (e.g., 12) and gate a death/assertion expectation
around the failing call (wrap ASSERT_DEATH or equivalent around
PTO2TensorMap::reserve_layout/init_data_from_layout) so it only runs when
assertions are enabled (use preprocessor guard for debug builds). Ensure
references remain to PTO2TensorMap::reserve_layout,
PTO2TensorMap::init_data_from_layout, bad.wire_arena_pointers, and the test name
InitRequiresPowerOfTwoBuckets when making the change.
---
Nitpick comments:
In `@src/a2a3/platform/onboard/host/device_runner.h`:
- Around line 210-220: The current comment claiming that pointers from
acquire_pooled_runtime_arena()/the pooled GM heap/PTO2 SM/runtime arena are
stable until finalize() is incorrect because setup_static_arena() may re-commit
(resize) a region and change its base(); update the documentation in
device_runner.h so it clearly states that pointers returned by
acquire_pooled_runtime_arena()/acquire_pooled_gm_heap()/acquire_pooled_pto2_sm()
are stable only until a subsequent call to setup_static_arena() that grows or
re-commits the region, and that callers must re-acquire the region (call the
appropriate acquire_* function and use the returned base()) after any resize;
mention the specific symbols setup_static_arena, acquire_pooled_runtime_arena,
finalize, and base() to make the requirement obvious.
In `@src/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_shared_memory.h`:
- Around line 214-257: The code does pointer arithmetic on sm_dev_base via char*
which can be UB; change all address computations in orch_error_code_addr,
ring_header_addr, ring_current_task_index_addr, ring_last_task_alive_addr, and
ring_task_descriptors_addr to perform arithmetic on a uintptr_t base (e.g.
uintptr_t base = reinterpret_cast<uintptr_t>(sm_dev_base)), add offsetof/size
offsets to that integer, and only at the end reinterpret_cast the final integer
(cast to void* first if needed) to the desired pointer type; ensure all uses of
static_cast<char*> are replaced with integer arithmetic and final
reinterpret_cast to preserve opaque device-address semantics.
🪄 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: afb4c479-cd23-4812-8bd2-97bc8283d600
📒 Files selected for processing (65)
src/a2a3/platform/onboard/host/device_runner.cppsrc/a2a3/platform/onboard/host/device_runner.hsrc/a2a3/platform/onboard/host/pto_runtime_c_api.cppsrc/a2a3/platform/sim/host/device_runner.cppsrc/a2a3/platform/sim/host/device_runner.hsrc/a2a3/platform/sim/host/pto_runtime_c_api.cppsrc/a2a3/runtime/host_build_graph/runtime/runtime.hsrc/a2a3/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/host/dep_gen_replay.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/host/runtime_maker.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_orchestrator.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_orchestrator.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_runtime2.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_runtime2.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_runtime2_types.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_shared_memory.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_tensormap.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/runtime.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/pto_scheduler.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/pto_scheduler.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/pto_runtime2_init.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/pto_shared_memory.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/pto_tensormap.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/runtime.cppsrc/a5/platform/onboard/host/device_runner.cppsrc/a5/platform/onboard/host/device_runner.hsrc/a5/platform/onboard/host/pto_runtime_c_api.cppsrc/a5/platform/sim/host/device_runner.cppsrc/a5/platform/sim/host/device_runner.hsrc/a5/platform/sim/host/pto_runtime_c_api.cppsrc/a5/runtime/host_build_graph/runtime/runtime.hsrc/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cppsrc/a5/runtime/tensormap_and_ringbuffer/host/runtime_maker.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_orchestrator.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_orchestrator.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_runtime2.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_runtime2.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_runtime2_types.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_shared_memory.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_tensormap.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/runtime.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/pto_scheduler.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/pto_scheduler.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/shared/pto_runtime2_init.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/shared/pto_shared_memory.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/shared/pto_tensormap.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/shared/runtime.cppsrc/common/device_comm/device_arena.htests/ut/cpp/CMakeLists.txttests/ut/cpp/a2a3/test_ready_queue.cpptests/ut/cpp/a2a3/test_scheduler_state.cpptests/ut/cpp/a2a3/test_spsc_queue.cpptests/ut/cpp/a2a3/test_task_allocator.cpptests/ut/cpp/a2a3/test_task_state.cpptests/ut/cpp/a2a3/test_tensormap.cpptests/ut/cpp/a2a3/test_wiring.cpptests/ut/cpp/a5/test_ready_queue.cpptests/ut/cpp/a5/test_scheduler_state.cpptests/ut/cpp/a5/test_spsc_queue.cpptests/ut/cpp/a5/test_task_allocator.cpptests/ut/cpp/a5/test_task_state.cpptests/ut/cpp/a5/test_tensormap.cpptests/ut/cpp/a5/test_wiring.cpp
💤 Files with no reviewable changes (2)
- src/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/pto_scheduler.cpp
- src/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/pto_scheduler.cpp
Move the per-slot payload/task pointer assignments out of the RingSchedState::init() O(task_window_size) loop and into orch::prepare_task. Their value is per-slot constant (&task_payloads[slot] / &task_descriptors[slot]) but writing them at submit time, on the same 64B slot_state cache line prepare_task is already dirtying, is essentially free — while removing the only "scale-dependent" pointer assignments from the init path. ring_id stays in init (its value is per-ring constant, so rewriting it each submit would only add noise without removing a loop). Split PTO2TaskSlotState::bind() into bind_ring() (init-time) and bind_buffers() (per-submit) to make the two call-site shapes explicit. Mirrored across both a2a3 and a5 trb runtimes.
Previously the AICPU rebuilt the entire trb runtime arena (PTO2Runtime, orchestrator/scheduler/tensor_map sub-regions, sm_handle wrapper, mailbox) on every device boot via runtime_create_from_sm. This commit moves layout + data init onto the host so the AICPU only does a cheap arena-internal pointer wire pass plus the SM reset that can't run off-device. Multi-run boots reuse the pooled prebuilt image with a single rtMemcpy. Mechanism - DeviceArena::attach() wraps an externally-owned buffer; re-attach is permitted so each AICPU boot can reuse the pooled image. - runtime_create_from_sm split into reserve_layout / init_data_from_layout / wire_arena_pointers / finalize_after_wire. orchestrator / scheduler / tensor_map / ready_queue / spsc gain matching data+wire pairs; finalize_after_wire stays AICPU-only since it binds s_runtime_ops. - pto2_sm_layout helper computes SM field device addresses by pure offset arithmetic so host init never dereferences SM. - Per-slot SM-side reset (bind_ring + reset_for_reuse + active_mask) moved from RingSchedState::init into PTO2SharedMemoryHandle::init_header_per_ring so the AICPU still owns it after the split. - runtime/shared/pto_runtime2_init.cpp — new file holding the host-able pieces lifted out of pto_runtime2.cpp / pto_orchestrator.cpp / pto_scheduler.cpp. AICPU-only ops table / submit_task / dispatch stay in place. Host wiring (runtime_maker.cpp) - DeviceRunner::setup_static_arena gains a third runtime_arena_size region (hbg passes 0). The prebuilt image lives in the same pooled backing allocation as gm_heap and SM, keeping worker lifetime to one rtMalloc. - bind_prepared_to_runtime_impl reserves layout on a host arena, sizes the pooled regions, runs init_data + wire, stashes prebuilt metadata into the rt image, rtMemcpys to device, and records base/offset on Runtime so the AICPU boot can find it. AICPU boot (aicpu_executor.cpp) - attach the runtime arena to the pooled buffer, take rt from base+off_runtime, wire arena-internal pointers, sm_handle->init (SM reset including the per-slot fields above), mailbox reset, finalize_after_wire (ops table + cluster/aiv counts). Tests - cpput: 25/25 pass. ready_queue / spsc_queue / scheduler_state / task_state / wiring / tensormap UTs migrated to the data+wire API. task_allocator.init grew an optional initial_local_task_id (default 0) so UTs can still exercise task_id near INT32_MAX without reading the SM. - a2a3sim trb: standalone (dynamic_register variants, L3 group/dependency) + L2 tensormap_and_ringbuffer 29 tests all pass. - a2a3sim host_build_graph: 9/9 pass (verifies the shared HostApi changes don't break hbg). - a2a3 hardware: tests/st/.../paged_attention_unroll PASS on device 9 (--build with pto-isa commit pinned to CI).
Address review feedback from PR hw-native-sys#846: - pto2_sm_layout::ring_task_descriptors_addr: take per-ring task_window_sizes[] array (mirroring PTO2SharedMemoryHandle's SM API) and assert ring_id range, so a future per-ring SM layout cannot silently disagree with the addresses the host bakes into the prebuilt image. - DeviceRunner::acquire_pooled_runtime_arena (onboard + sim): return nullptr when runtime_arena_region_off_ == SIZE_MAX so a stray hbg-path call cannot resolve to base + SIZE_MAX. Failure is now loud and contained at the acquire boundary. - DeviceArena::attach(): rewrite doc to match real behavior (region table is not repopulated after attach, reserve() asserts !committed_ so cannot replay, region_size() returns 0); promote the pre-alignment / non-null / power-of-two checks from plain assert() to an unconditional abort() so release builds still trap on contract violations. - PTO2TensorMap: drop the dead `orch` back-pointer field (a2a3 never dereferences it), strip parent_orch parameter from wire_arena_pointers, and remove the now-unused PTO2OrchestratorState forward declaration. - PTO2RingFlowControl::init(): add a coupling comment so future fc-initial- value or boot-order changes flag PTO2TaskAllocator::init's initial_local_task_id default in the same edit. - PTO2SchedulerState::init_data_from_layout / RingSchedState:: init_data_from_layout: drop the task_window_size / dep_pool_capacity parameters that were never consumed (scheduler only needs SM base + ring index, both window-size-independent; orchestrator counterpart still takes task_window_size for ring_task_descriptors arithmetic). Updated all callsites (pto_runtime2_init.cpp + 4 cpput suites). - PTO2Runtime::prebuilt_arena_base: removed the dead mirror field. The host Runtime's prebuilt_arena_base_ is the real source of truth (AICPU reads it to locate the pooled buffer *before* dereferencing the image); the PTO2Runtime image still carries prebuilt_layout, which the AICPU does consume. cpput: 25/25 pass. a2a3sim trb: dummy_task / dynamic_register / L2 trb suite pass with --build.
Sync of PR hw-native-sys#846 commit 2/3 to a5 — commit 1 (slot_state.bind split) was already mirrored. Brings the a5 trb runtime up to the same host-build arena fast path as a2a3. - 4-phase API (reserve_layout / init_data_from_layout / wire_arena_pointers / finalize_after_wire) replaces runtime_create_from_sm. - New runtime/shared/pto_runtime2_init.cpp (~355 lines) and shared/pto_tensormap.cpp (the old runtime/pto_tensormap.cpp moved + split) hold the host-pluggable cold-path lifted from pto_runtime2.cpp / pto_orchestrator.cpp / scheduler/pto_scheduler.cpp. - AICPU boot becomes attach + wire + sm_handle->init + finalize. - runtime_maker.cpp pre-builds the arena image on host and rtMemcpys it into a pooled runtime-arena region; onboard + sim DeviceRunner setup_static_arena grow a third runtime_arena_size argument with matching acquire_pooled_runtime_arena (hbg path passes 0). a5-specific divergences kept: enable_l2_swimlane (bool) instead of L2PerfLevel, no dep_gen subsystem, wait_init_complete naming, alignas(64) PTO2SpscQueue queue, cache_invalidate_range + cond.retire in async_wait, RUNTIME_MAX_WORKER 108. Tests - cpput: 25/25 pass. - a5sim: trb 21/21 + host_build_graph 6/6 pass. - a2a3sim regression: trb 29/29 + host_build_graph 9/9 pass.
…tions DeviceRunner's GM heap / PTO2 SM / trb prebuilt runtime arena used to live in a single backing device buffer (one rtMalloc per worker, three regions sub-divided via DeviceArena::reserve). The combined size can exceed the device allocator's largest contiguous block on real hardware, so split into three independent DeviceArena instances — each commits exactly one region (one device_malloc), and acquire_pooled_* returns its base(). Touches all four DeviceRunner implementations (a2a3/a5 × onboard/sim). The setup_static_arena and acquire_pooled_* signatures are unchanged; the host_api / runtime_maker callers are unaffected. hbg keeps passing runtime_arena_size = 0, which leaves runtime_arena_pool_ uncommitted and acquire_pooled_runtime_arena returning nullptr. Tests - cpput: 25/25 pass. - a5sim: L2 trb + host_build_graph full suite pass. - a2a3sim: L2 trb + host_build_graph full suite pass.
Address review feedback covering doc / comment consistency and a small set of behavioral symmetry items between a2a3 and a5 trb runtimes: - pto_orchestrator.h: drop the stale ",orch" from the wire_arena_pointers comment on a2a3 (PTO2TensorMap::orch was removed in 75f2562 but a2a3 kept the comment lagging behind the a5 mirror). - runtime.h / device_runner.h (both arches): refresh the setup_static_arena / acquire_pooled_* docblocks. Drop the orphan pre-split prose ("backs both the PTO2 GM heap and the PTO2 shared memory in a single underlying allocation") and the "doing so returns an unreserved-offset region_ptr (undefined)" wording that no longer matches the three-independent-arenas split — acquire_pooled_runtime_arena now returns a well-defined nullptr on the hbg path. - a5 device_runner.{h,cpp}: restore the rationale comments that the a5 mirror lost when it copied a2a3's earlier shape — three separate device_malloc calls being friendlier than one big one, hbg's runtime_arena_size == 0 contract, commit() failure rollback invariants, idempotent peer-arena policy. Keeps the why-this-way notes symmetric with a2a3. - a5 runtime.h: fix the RUNTIME_MAX_ORCH_SO_SIZE comment that claimed "1MB" while the macro expands to 4MB. - a5 pto_orchestrator.cpp: drop the prod_state->task null / task_id defensive guard. PTO2TensorMap lookup chain truncation already guarantees producer_task_id >= last_task_alive, and producers reach the tensormap only after prepare_task has bound the slot. Matches the a2a3 shape that relies on the same invariants. - a5 cpput: migrate the three stale UTs (test_ready_queue, test_spsc_queue, test_tensormap) to the new 4-phase reserve_layout / init_data_from_layout / wire_arena_pointers API. Wire them and the previously-orphaned a5 trb UTs into CMakeLists.txt behind a new a5_rt_objs OBJECT library + add_a5_runtime_test helper (mirrors a2a3_rt_objs). Target names carry the test_a5_ prefix to avoid clashing with hierarchical / a2a3 unprefixed test names. Tests - cpput: 35/35 pass (25 a2a3 + 10 newly enabled a5 trb). - a5sim: full sim suite passes. - a2a3sim: full sim suite passes (regression).
- setup_static_arena (a2a3 onboard + a5 sim mirrors): drop the late-region failure paths that released already-committed peer arenas. Callers may hold pooled pointers from earlier successful regions; tearing the peers down on a later resize failure turns those pointers into dangling refs, contradicting the lambda's "already-committed peers stay alive" invariant. - DeviceRunner::finalize (a2a3 sim, a5 onboard): move the lazily-allocated device_wall_dev_ptr_ free above mem_alloc_.finalize() (and above rtDeviceReset on a5). free_tensor() routes through mem_alloc_.free(), so freeing after finalize was a use-after-finalize on the allocator state; on a5 it would also run after the device runtime had been reset. - bind_prepared_to_runtime_impl (a2a3 + a5 runtime_maker): reject env-derived PTO2_RING_DEP_POOL values above INT32_MAX before narrowing to int32_t, rather than silently truncating into a corrupt layout sizing. - test_a5_tensormap: rename InitRequiresPowerOfTwoBuckets to InitWithPowerOfTwoBucketsSucceeds and reword the comment. The earlier name was misleading because the body only exercises the success path (bucket count 8); the reject path is gated by always_assert and can't be reliably EXPECT_DEATH-tested in release builds. Tests - cpput: 35/35 pass (including renamed a5 tensormap test).
c22ca95 to
7a1036a
Compare
## Summary
Move the trb runtime arena's layout + data initialization from AICPU boot
onto host, so each AICPU launch reduces to a cheap pointer-fixup pass plus
the SM reset that can't run off-device. The pooled prebuilt image lives in
a per-Worker DeviceRunner pool and is reused across runs via a single
rtMemcpy — multi-launch boot cost drops from O(task_window_size) per
worker to a constant.
Two related cleanups ride along:
- `RingSchedState::init`'s O(task_window_size) slot-bind loop is lifted
into per-submit `orch::prepare_task`, making startup independent of
window size. The two extra stores hit the same 64B cache line that
`prepare_task` already dirties, so the per-submit cost is essentially
free.
- AICPU SM reset (per-slot `bind_ring` + `reset_for_reuse` +
`fanin_count`/`active_mask` zero) consolidated into
`PTO2SharedMemoryHandle::init_header_per_ring` so the host-build path
doesn't dereference SM.
Covers **both a2a3 and a5 trb runtimes** (platform layer onboard + sim).
hbg is unaffected by the runtime-arena split — its
`setup_static_arena(...,0)` keeps the third region unreserved.
## Mechanism
- `runtime_create_from_sm` split into four phases that run on either side:
- `runtime_reserve_layout` — pure arithmetic; host computes sub-region
offsets on a libc-backed `DeviceArena`.
- `runtime_init_data_from_layout` — writes standalone fields, memset's
arena regions, and stores SM device pointers (only stores, no
dereferences).
- `runtime_wire_arena_pointers` — walks every arena-internal pointer
field and binds it to `arena.base() + offset`. Idempotent: host runs
once with the host mirror, AICPU runs once after attach with device
addresses.
- `runtime_finalize_after_wire` — AICPU-only fixup for `s_runtime_ops`
(device-side file-local global) and the SPMD core counts from the
`SchedulerContext`.
- `DeviceArena::attach()` wraps an externally-owned buffer with no
per-attach allocation; re-attach is permitted so each AICPU boot can
reuse the same pooled image. Pre-alignment / non-null / power-of-two
checks `std::abort()` instead of `assert()` so release builds still
trap on contract violations.
- `pto2_sm_layout` namespace computes SM device-side field addresses by
pure offset arithmetic so host init never reads SM. Takes a per-ring
`task_window_sizes[]` array (mirroring the SM API) and asserts
`ring_id` in range — structurally prevents the host-built image from
silently disagreeing with the SM layout.
- New `runtime/shared/pto_runtime2_init.cpp` holds the host-pluggable
cold-path lifted from `pto_runtime2.cpp` / `pto_orchestrator.cpp` /
`scheduler/pto_scheduler.cpp`. AICPU-only ops table / submit_task /
dispatch / business logic stay in their original files.
- `DeviceRunner` now owns **three independent pooled arenas** —
`gm_heap_arena_`, `gm_sm_arena_`, `runtime_arena_pool_` — one
`device_malloc` each. Split out from a single backing allocation
because the combined size can exceed the device allocator's largest
contiguous block. `setup_static_arena(gm_heap_size, gm_sm_size,
runtime_arena_size)` commits each region independently;
`acquire_pooled_runtime_arena()` returns `nullptr` when the region is
unreserved (hbg's `setup_static_arena(...,0)` path) so misuse is loud,
not undefined.
- `bind_prepared_to_runtime_impl` (host runtime_maker) does the full
reserve_layout → init_data → wire on a host arena, stashes the layout
inside the `PTO2Runtime` image at `prebuilt_layout`, then rtMemcpys
the whole arena into the pooled device region.
- Dead fields and parameters dropped: `PTO2TensorMap::orch` back-pointer
(never dereferenced), `PTO2Runtime::prebuilt_arena_base` mirror (host
`Runtime::prebuilt_arena_base_` is the real source of truth), unused
`task_window_size` / `dep_pool_capacity` from
`PTO2SchedulerState::init_data_from_layout` and
`RingSchedState::init_data_from_layout` (scheduler only needs SM base
+ ring index, both window-size-independent).
## Test plan
- [x] **cpput**: 25/25 pass. ready_queue / spsc_queue / scheduler_state /
task_state / wiring / tensormap UTs migrated to the data+wire API.
`task_allocator.init` grew an optional `initial_local_task_id`
(default 0) so the near-INT32_MAX corner case is still exercised
without reading the SM.
- [x] **a5sim**: L2 trb 21/21 + L2 host_build_graph 6/6 pass.
- [x] **a2a3sim**: L2 trb 29/29 + L2 host_build_graph 9/9 pass.
- [x] **a2a3 hardware**: `tests/st/.../paged_attention_unroll` passes on
device 9 (`--build`, pto-isa commit pinned to CI).
Summary
Move the trb runtime arena's layout + data initialization from AICPU boot
onto host, so each AICPU launch reduces to a cheap pointer-fixup pass plus
the SM reset that can't run off-device. The pooled prebuilt image lives in
a per-Worker DeviceRunner pool and is reused across runs via a single
rtMemcpy — multi-launch boot cost drops from O(task_window_size) per
worker to a constant.
Two related cleanups ride along:
RingSchedState::init's O(task_window_size) slot-bind loop is liftedinto per-submit
orch::prepare_task, making startup independent ofwindow size. The two extra stores hit the same 64B cache line that
prepare_taskalready dirties, so the per-submit cost is essentiallyfree.
bind_ring+reset_for_reuse+fanin_count/active_maskzero) consolidated intoPTO2SharedMemoryHandle::init_header_per_ringso the host-build pathdoesn't dereference SM.
Covers both a2a3 and a5 trb runtimes (platform layer onboard + sim).
hbg is unaffected by the runtime-arena split — its
setup_static_arena(...,0)keeps the third region unreserved.Mechanism
runtime_create_from_smsplit into four phases that run on either side:runtime_reserve_layout— pure arithmetic; host computes sub-regionoffsets on a libc-backed
DeviceArena.runtime_init_data_from_layout— writes standalone fields, memset'sarena regions, and stores SM device pointers (only stores, no
dereferences).
runtime_wire_arena_pointers— walks every arena-internal pointerfield and binds it to
arena.base() + offset. Idempotent: host runsonce with the host mirror, AICPU runs once after attach with device
addresses.
runtime_finalize_after_wire— AICPU-only fixup fors_runtime_ops(device-side file-local global) and the SPMD core counts from the
SchedulerContext.DeviceArena::attach()wraps an externally-owned buffer with noper-attach allocation; re-attach is permitted so each AICPU boot can
reuse the same pooled image. Pre-alignment / non-null / power-of-two
checks
std::abort()instead ofassert()so release builds stilltrap on contract violations.
pto2_sm_layoutnamespace computes SM device-side field addresses bypure offset arithmetic so host init never reads SM. Takes a per-ring
task_window_sizes[]array (mirroring the SM API) and assertsring_idin range — structurally prevents the host-built image fromsilently disagreeing with the SM layout.
runtime/shared/pto_runtime2_init.cppholds the host-pluggablecold-path lifted from
pto_runtime2.cpp/pto_orchestrator.cpp/scheduler/pto_scheduler.cpp. AICPU-only ops table / submit_task /dispatch / business logic stay in their original files.
DeviceRunnernow owns three independent pooled arenas —gm_heap_arena_,gm_sm_arena_,runtime_arena_pool_— onedevice_malloceach. Split out from a single backing allocationbecause the combined size can exceed the device allocator's largest
contiguous block.
setup_static_arena(gm_heap_size, gm_sm_size, runtime_arena_size)commits each region independently;acquire_pooled_runtime_arena()returnsnullptrwhen the region isunreserved (hbg's
setup_static_arena(...,0)path) so misuse is loud,not undefined.
bind_prepared_to_runtime_impl(host runtime_maker) does the fullreserve_layout → init_data → wire on a host arena, stashes the layout
inside the
PTO2Runtimeimage atprebuilt_layout, then rtMemcpysthe whole arena into the pooled device region.
PTO2TensorMap::orchback-pointer(never dereferenced),
PTO2Runtime::prebuilt_arena_basemirror (hostRuntime::prebuilt_arena_base_is the real source of truth), unusedtask_window_size/dep_pool_capacityfromPTO2SchedulerState::init_data_from_layoutandRingSchedState::init_data_from_layout(scheduler only needs SM baseTest plan
task_state / wiring / tensormap UTs migrated to the data+wire API.
task_allocator.initgrew an optionalinitial_local_task_id(default 0) so the near-INT32_MAX corner case is still exercised
without reading the SM.
tests/st/.../paged_attention_unrollpasses ondevice 9 (
--build, pto-isa commit pinned to CI).