Refactor: drop _prepared_ prefix from internal DeviceRunner callable methods#911
Conversation
… methods
Internal DeviceRunner methods carried a `_prepared_` infix that historically
distinguished "register-via-the-prepare_callable-path" from a now-removed
eager-register path. With only one path remaining, the prefix is dead
naming and creates an asymmetry with the public C API
(`prepare_callable` / `unregister_callable` / `run_prepared`).
Renames (pure mechanical, no behavior change):
- `DeviceRunner::register_prepared_callable[_host_orch]` → `register_callable[_host_orch]`
- `DeviceRunner::has_prepared_callable` → `has_callable`
- `DeviceRunner::unregister_prepared_callable` → `unregister_callable`
- `DeviceRunner::bind_prepared_callable_to_runtime` → `bind_callable_to_runtime`
- `prepared_callables_` → `callables_`
- `PreparedCallableState` → `CallableState`
- `BindPreparedCallableResult` → `BindCallableResult`
- `PreparedCallableArtifacts` → `CallableArtifacts`
- `bind_prepared_to_runtime_impl` → `bind_callable_to_runtime_impl` (runtime_maker)
Public Python / c_api entry points (`prepare_callable`, `run_prepared`,
`aicpu_dlopen_count`, `host_dlopen_count`) and the public
`unregister_callable` free function are unchanged — those names have real
semantic meaning ("prepare the callable", "run something previously
prepared").
Both arches built clean (onboard + sim, both runtimes). a2a3 local smoke
(dummy_task, prepared_callable suite) 7/7 passed in 11s.
📝 WalkthroughWalkthroughThis PR systematically renames the callable-registration and binding APIs from "prepared_callable" to "callable" terminology across all DeviceRunner platform implementations (a2a3 onboard, a2a3 simulation, a5 onboard, a5 simulation), updating type names, method signatures, internal state storage, runtime-binding implementations, and documentation. ChangesCallable API Rename
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Code Review
This pull request performs a comprehensive refactoring across the codebase to simplify terminology by renaming 'prepared callable' references to simply 'callable'. This includes renaming classes (e.g., PreparedCallableState to CallableState, PreparedCallableArtifacts to CallableArtifacts), functions (e.g., register_prepared_callable to register_callable, bind_prepared_callable_to_runtime to bind_callable_to_runtime), member variables, and associated comments across Python, C++, and test files. No review comments were provided, so there is no additional feedback to address.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/a2a3/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp (1)
97-109:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the stale
@param runtimeentry from this docblock.
prepare_callable_implno longer takes aRuntime *, so this comment now documents a nonexistent parameter.🤖 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/host/runtime_maker.cpp` around lines 97 - 109, Remove the stale "`@param` runtime" line from the docblock above prepare_callable_impl since the function signature no longer accepts a Runtime pointer; update the comment to only document the current parameters (callable, upload_fn, out) and return value so the docblock matches the prepare_callable_impl declaration and related symbols like CallableArtifacts and ChipCallable.src/a5/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp (1)
97-109:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrop the stale
@param runtimeline here too.The comment still documents a
Runtime *parameter that is no longer part ofprepare_callable_impl.🤖 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/a5/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp` around lines 97 - 109, The function comment above prepare_callable_impl still documents a removed Runtime* parameter; update the docblock for prepare_callable_impl by removing the stale "`@param` runtime" line (and any accompanying description) so the comment accurately reflects the current signature (prepare_callable_impl(const ChipCallable*, uint64_t (*upload_fn)(const void *), CallableArtifacts*)). Ensure remaining `@param` entries match the actual parameters (callable, upload_fn, out) and keep the rest of the explanatory text unchanged.
🧹 Nitpick comments (1)
src/a5/runtime/tensormap_and_ringbuffer/runtime/runtime.h (1)
300-303: ⚡ Quick winFinish the terminology rename in this comment.
After switching the API reference to
bind_callable_to_runtime, "prepared kernel binaries" is now the odd term out in the same sentence. Consider renaming it to "callable kernel binaries" for consistency.🤖 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/a5/runtime/tensormap_and_ringbuffer/runtime/runtime.h` around lines 300 - 303, Update the comment in runtime.h to use consistent terminology: replace "prepared kernel binaries" with "callable kernel binaries" in the comment that documents replaying a previously-uploaded kernel address onto a fresh Runtime (the block referencing bind_callable_to_runtime, registered_kernel_func_ids_, and validate_runtime_impl) so the phrasing matches the API rename to bind_callable_to_runtime.
🤖 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.
Outside diff comments:
In `@src/a2a3/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp`:
- Around line 97-109: Remove the stale "`@param` runtime" line from the docblock
above prepare_callable_impl since the function signature no longer accepts a
Runtime pointer; update the comment to only document the current parameters
(callable, upload_fn, out) and return value so the docblock matches the
prepare_callable_impl declaration and related symbols like CallableArtifacts and
ChipCallable.
In `@src/a5/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp`:
- Around line 97-109: The function comment above prepare_callable_impl still
documents a removed Runtime* parameter; update the docblock for
prepare_callable_impl by removing the stale "`@param` runtime" line (and any
accompanying description) so the comment accurately reflects the current
signature (prepare_callable_impl(const ChipCallable*, uint64_t
(*upload_fn)(const void *), CallableArtifacts*)). Ensure remaining `@param`
entries match the actual parameters (callable, upload_fn, out) and keep the rest
of the explanatory text unchanged.
---
Nitpick comments:
In `@src/a5/runtime/tensormap_and_ringbuffer/runtime/runtime.h`:
- Around line 300-303: Update the comment in runtime.h to use consistent
terminology: replace "prepared kernel binaries" with "callable kernel binaries"
in the comment that documents replaying a previously-uploaded kernel address
onto a fresh Runtime (the block referencing bind_callable_to_runtime,
registered_kernel_func_ids_, and validate_runtime_impl) so the phrasing matches
the API rename to bind_callable_to_runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e2c7584-1e2e-4643-9112-34c944aef3d1
📒 Files selected for processing (26)
python/simpler/worker.pysrc/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/host/runtime_maker.cppsrc/a2a3/runtime/host_build_graph/runtime/runtime.hsrc/a2a3/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/host/runtime_maker.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/runtime.hsrc/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/host/runtime_maker.cppsrc/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cppsrc/a5/runtime/tensormap_and_ringbuffer/host/runtime_maker.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/runtime.hsrc/common/task_interface/callable_protocol.hsrc/common/task_interface/prepare_callable_common.htests/st/a2a3/host_build_graph/prepared_callable/test_prepared_callable.pytests/ut/py/test_worker/test_host_worker.py
Summary
Pure rename PR. Drops the meaningless
_prepared_infix from internalDeviceRunnermethods and supporting types. With only one registration path remaining today, the prefix is dead naming and creates an awkward asymmetry with the public C API (prepare_callable/unregister_callable/run_prepared).Renames
DeviceRunner::register_prepared_callable[_host_orch]register_callable[_host_orch]DeviceRunner::has_prepared_callablehas_callableDeviceRunner::unregister_prepared_callableunregister_callable(matches c_api)DeviceRunner::bind_prepared_callable_to_runtimebind_callable_to_runtimeprepared_callables_(map)callables_PreparedCallableState(struct)CallableStateBindPreparedCallableResult(struct)BindCallableResultPreparedCallableArtifacts(struct)CallableArtifactsbind_prepared_to_runtime_impl(free fn)bind_callable_to_runtime_implWhat's unchanged
Public API surface keeps its original names because the verb "prepared" carries real meaning there:
prepare_callable(public Python + c_api — "prepare this callable")run_prepared(public Python + c_api — "run something previously prepared")aicpu_dlopen_count/host_dlopen_count(public diagnostics)unregister_callable(public c_api — already had this name, now method matches)Verification
Test plan