Skip to content

Refactor: drop vestigial orch_built_on_host flag#766

Merged
poursoul merged 1 commit into
hw-native-sys:mainfrom
ChaoWao:refactor/drop-orch-built-on-host
May 13, 2026
Merged

Refactor: drop vestigial orch_built_on_host flag#766
poursoul merged 1 commit into
hw-native-sys:mainfrom
ChaoWao:refactor/drop-orch-built-on-host

Conversation

@ChaoWao
Copy link
Copy Markdown
Collaborator

@ChaoWao ChaoWao commented May 13, 2026

Summary

Runtime::orch_built_on_host_ used to distinguish the host-built-graph runtime
from the (since removed, #701) aicpu_build_graph one. With only
host_build_graph and tensormap_and_ringbuffer left it is a per-runtime
constant, and after #760 removed the last platform-layer caller it is also
unreferenced outside the runtime internals. This drops it:

  • host_build_graph (a2a3 + a5): get_orch_built_on_host() was hard-coded
    to true and has no callers — deleted.
  • tensormap_and_ringbuffer (a2a3 + a5): bind_prepared_to_runtime_impl
    unconditionally sets the flag to false before the runtime struct reaches
    any reader (the ctor's = true was always overwritten first), so every
    get_orch_built_on_host() site already took the device-orchestration path.
    Inlined that:
    • drop the orch_built_on_host_ field + getter + setter (runtime.h,
      runtime/shared/runtime.cpp) and the set_orch_built_on_host(false) call
      in host/runtime_maker.cpp;
    • aicpu/aicpu_executor.cpp: the if (get_orch_built_on_host()) { /* host orchestration, no-op */ } else { ... } becomes a plain scope (kept as a
      block so the per-callable dlopen / SO-table locals stay scoped); the
      SM-header spin-wait and the rt-destroy guard lose their always-true
      !get_orch_built_on_host() prefix;
    • runtime/scheduler/scheduler_cold_path.cpp: orchestrator_done_ init
      becomes a literal false.

No behavior change — every removed branch was statically unreachable.

Testing

  • a2a3 + a5 sim libhost_runtime.so (both runtimes) rebuild cleanly;
    pre-commit (clang-format, clang-tidy, cpplint) passes.
  • Hardware: a2a3 + a5 tensormap_and_ringbuffer ST (e.g. an AICPU-orch
    example) still runs to completion — the touched code is the AICPU
    executor's orchestrator/scheduler path.

`Runtime::orch_built_on_host_` distinguished the host-built-graph runtime
from the (removed) aicpu_build_graph one. With only host_build_graph and
tensormap_and_ringbuffer left, the flag is a per-runtime constant:

- host_build_graph hard-coded `get_orch_built_on_host()` to `true` and, since
  hw-native-sys#760 removed the last platform-layer caller, nothing reads it — delete it.
- tensormap_and_ringbuffer always sets it to `false` in
  bind_prepared_to_runtime_impl (the runtime ctor's `= true` was overwritten
  before any reader saw it), so every `get_orch_built_on_host()` site there
  takes the device-orchestration branch. Inline that: drop the field, getter,
  setter; the AICPU executor's "host orchestration, no-op" dead branch
  becomes a plain scope; the SM-header spin-wait and the rt-destroy guard
  lose their always-true `!get_orch_built_on_host()` prefix; the scheduler's
  `orchestrator_done_` init becomes a literal `false`.

No behavior change — every removed branch was statically unreachable.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the orch_built_on_host flag and its associated logic from the a2a3 and a5 runtimes, simplifying the AicpuExecutor and scheduler by consolidating the orchestration path. Review feedback suggests implementing RAII guards or try-catch blocks for resources like dlopen handles within the refactored orchestration scopes to prevent potential resource leaks.

Comment thread src/a2a3/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp
Comment thread src/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp
@poursoul poursoul merged commit bb4942f into hw-native-sys:main May 13, 2026
14 checks passed
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.

2 participants