Skip to content

Refactor: remove dead gm_heap_ptr_ and slot_states_ptr_ from trb Runtime#1224

Merged
ChaoWao merged 1 commit into
hw-native-sys:mainfrom
ChaoWao:cleanup/purge-dead-runtime-ptrs
Jul 1, 2026
Merged

Refactor: remove dead gm_heap_ptr_ and slot_states_ptr_ from trb Runtime#1224
ChaoWao merged 1 commit into
hw-native-sys:mainfrom
ChaoWao:cleanup/purge-dead-runtime-ptrs

Conversation

@ChaoWao

@ChaoWao ChaoWao commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-up cleanup to #1216. Two Runtime fields in the tensormap_and_ringbuffer runtime were carried but never read — remove them (both arches, lockstep).

  • slot_states_ptr_ (in the device-copied dev descriptor): only ever written nullptr (ctor + one aicpu_executor site) and never read anywhere. Removed the field, its setter, and the nullptr-write call site. dev is alignas(64), so the sizeof % 64 == 0 static_assert still holds.
  • gm_heap_ptr_ (host-only tail): written via set_gm_heap in runtime_maker but never read. The GM-heap buffer is owned by DeviceRunnerBase::gm_heap_arena_ and released in finalize(), independent of this field — runtime_maker keeps the local StaticArenaPtrs::gm_heap that feeds init_data_from_layout, so the acquire_pooled_gm_heap() acquisition stays. Removed the field, its getter/setter, and the set_gm_heap call site.

host_build_graph is untouched (it has its own Runtime and never referenced these trb accessors); the shared platform layer never called them either.

Testing

  • Simulation tests pass — a2a3sim prepared_callable + dummy_task + orch_so_cache (9 passed)
  • Hardware tests pass — a2a3 onboard prepared_callable + dummy_task + mixed_example (9 passed)
  • Builds clean for a2a3 + a5, sim + onboard, trb + hbg (the sizeof % 64 == 0 / is_trivially_copyable_v static_asserts still hold)

Follow-up cleanup to hw-native-sys#1216. Two runtime fields were carried but never read:

- `slot_states_ptr_` (in the device-copied `dev` descriptor): only ever
  written nullptr (ctor + one aicpu_executor site) and never read anywhere.
  Removed the field, its setter, and the nullptr-write call site. Since `dev`
  is alignas(64), the sizeof%64==0 static_assert still holds.
- `gm_heap_ptr_` (host-only tail): written via set_gm_heap in runtime_maker
  but never read — the GM heap buffer is owned by DeviceRunnerBase's
  gm_heap_arena_ and released in finalize(), independent of this field
  (runtime_maker keeps the local StaticArenaPtrs::gm_heap that feeds
  init_data_from_layout). Removed the field, its getter/setter, and the
  set_gm_heap call site; the acquire_pooled_gm_heap() acquisition stays.

Both arches (a2a3 + a5) lockstep. hbg is untouched (it has its own Runtime and
never referenced these trb accessors).

Verified: a2a3+a5 sim+onboard builds (trb+hbg); a2a3sim prepared_callable +
dummy_task + orch_so_cache; a2a3 onboard prepared_callable + dummy_task +
mixed_example.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 183cf548-75e6-4bb1-a99d-a682fbedda49

📥 Commits

Reviewing files that changed from the base of the PR and between 62adb13 and 7526853.

📒 Files selected for processing (8)
  • src/a2a3/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp
  • src/a2a3/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp
  • src/a2a3/runtime/tensormap_and_ringbuffer/runtime/runtime.h
  • src/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/runtime.cpp
  • src/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp
  • src/a5/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp
  • src/a5/runtime/tensormap_and_ringbuffer/runtime/runtime.h
  • src/a5/runtime/tensormap_and_ringbuffer/runtime/shared/runtime.cpp
💤 Files with no reviewable changes (8)
  • src/a2a3/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp
  • src/a5/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp
  • src/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp
  • src/a5/runtime/tensormap_and_ringbuffer/runtime/shared/runtime.cpp
  • src/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/runtime.cpp
  • src/a2a3/runtime/tensormap_and_ringbuffer/runtime/runtime.h
  • src/a5/runtime/tensormap_and_ringbuffer/runtime/runtime.h
  • src/a2a3/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp

📝 Walkthrough

Walkthrough

This change removes slot-states pointer and GM heap pointer state from the Runtime class in both a2a3 and a5 runtime variants. The DeviceRuntimeLaunchDesc struct drops slot_states_ptr_, Runtime drops gm_heap_ptr_ and their associated accessor/mutator methods, and corresponding call sites in executor and runtime-maker code are removed.

Runtime State Field Removal

Layer / File(s) Summary
Runtime struct/class contract changes
src/a2a3/.../runtime/runtime.h, src/a5/.../runtime/runtime.h
DeviceRuntimeLaunchDesc loses the slot_states_ptr_ field and Runtime loses the gm_heap_ptr_ member plus get_gm_heap_ptr, set_gm_heap, and set_slot_states_ptr declarations in both variants.
Runtime constructor and accessor implementation changes
src/a2a3/.../runtime/shared/runtime.cpp, src/a5/.../runtime/shared/runtime.cpp
Constructors no longer initialize dev.slot_states_ptr_ or gm_heap_ptr_, and the corresponding accessor definitions are removed, leaving only gm_sm_ptr/orch_args accessors.
Call site cleanup in executor and runtime maker
src/a2a3/.../aicpu/aicpu_executor.cpp, src/a5/.../aicpu/aicpu_executor.cpp, src/a2a3/.../host/runtime_maker.cpp, src/a5/.../host/runtime_maker.cpp
Removes the set_slot_states_ptr(nullptr) calls in executor run paths and the set_gm_heap(out->gm_heap) calls in ensure_static_arenas.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • hw-native-sys/simpler#1132: Both PRs touch slot_states_ptr plumbing into runtime/allocator setup, one removing it while the other adds it for deadlock detection.
  • hw-native-sys/simpler#1199: Both PRs modify AicpuExecutor::run and slot-state lifecycle handling in the AICPU shared-memory path.
  • hw-native-sys/simpler#1216: Both PRs touch the DeviceRuntimeLaunchDesc/Runtime device-state ABI, with this PR removing fields the other introduced.

Poem

A pointer once held, now let it go free,
No more slot-states clutter for Runtime to see. 🐇
The GM heap setter has hopped out of sight,
Two variants trimmed, both a2a3 and a5, light.
Cleaner burrows make for a swifter build night! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: removing dead gm_heap_ptr_ and slot_states_ptr_ from the trb Runtime.
Description check ✅ Passed The description directly matches the cleanup and explains the removed fields, setters, and call sites.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ChaoWao ChaoWao merged commit da79b70 into hw-native-sys:main Jul 1, 2026
16 checks passed
@ChaoWao ChaoWao deleted the cleanup/purge-dead-runtime-ptrs branch July 1, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant