From 90fb9e5420a58ffcd70d9fa6297e375b955b431b Mon Sep 17 00:00:00 2001 From: Chao Wang <26245345+ChaoWao@users.noreply.github.com> Date: Mon, 3 Aug 2026 01:51:18 -0700 Subject: [PATCH] Fix: state the post-B4 runner geometry, stream, and TLS contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1587 moved three contracts without moving the text that described them, and left one failure mode expressed as a noexcept violation. Geometry. resolve_block_dim() and prepare_launch_shape() no longer write block_dim_ or worker_count_; activate_launch_shape() latches both on the executor thread immediately before run(). The comment and the LOG_ERROR in each onboard run() still named prepare_launch_shape, so the one diagnostic a future reader greps pointed at a function that latches nothing. #1521 later edited the line directly below that comment and left it standing, which is how a stale comment survives. The simulation runners keep their wording: SimDeviceRunnerBase::prepare_launch_shape does still assign block_dim_. Streams. RunStreamSlots became a two-thread class when native prepare started provisioning the successor's slot while the executor retires the predecessor's. Per-slot handles are safe — admission gives each slot one owner — but created_count_ is shared across owners and is also read from an unrelated thread through get_run_stream_set_create_count, so it is now atomic and the ownership rule is stated on the class. Thread selection. restore_native_run_thread_selection was noexcept while run_selection() could throw: on a thread created by create_thread the per-thread block does not exist yet, so installation allocates. Split out a non-throwing try_run_selection() and let restore abort with a message on the unrecoverable path. Returning instead would leave the thread on the default slot and bank, addressing storage another lease owns, and a freshly started thread has no channel to report the failure through. B6c removes the mechanism outright; until then the failure is diagnosable rather than a bare terminate. Symbol loading. Since every required pipeline symbol became a strict load, the dominant cause of a dlsym failure is a host runtime out of sync with the tree that consumes it. Say so in the error, which otherwise reports only the missing name. Also spell the successor-already-staged test as occupied > 1, since the loop above it has already rejected every predecessor that may not carry one, and record that simulation discards native-run identity by design. --- .../platform/onboard/host/device_runner.cpp | 6 ++-- .../platform/onboard/host/device_runner.cpp | 6 ++-- .../platform/include/host/run_stream_slots.h | 16 ++++++++-- .../onboard/host/device_runner_base.cpp | 29 ++++++++++++++----- .../onboard/host/device_runner_base.h | 6 ++++ src/common/platform/sim/host/c_api_shared.cpp | 5 ++++ src/common/worker/chip_worker.cpp | 5 +++- 7 files changed, 56 insertions(+), 17 deletions(-) diff --git a/src/a2a3/platform/onboard/host/device_runner.cpp b/src/a2a3/platform/onboard/host/device_runner.cpp index 8409eca5fe..c326a521ba 100644 --- a/src/a2a3/platform/onboard/host/device_runner.cpp +++ b/src/a2a3/platform/onboard/host/device_runner.cpp @@ -259,8 +259,8 @@ int DeviceRunner::run(Runtime &runtime, const CallConfig &config) { // Latch this run's diagnostic enables onto the runner before the collector // paths below read them; block_dim/aicpu_thread_num are consumed locally. apply_call_config(config); - // prepare_launch_shape() resolved block_dim before the graph was built, so - // the geometry this run launches with is already on the runner. + // activate_launch_shape() latches this run's geometry onto the runner on the + // executor thread immediately before run(), so block_dim_ is this run's. const int block_dim = block_dim_; int launch_aicpu_num = config.aicpu_thread_num; // A prior AICore launch/sync error poisoned the device context and the @@ -291,7 +291,7 @@ int DeviceRunner::run(Runtime &runtime, const CallConfig &config) { ensure_device_wall_buffer(); if (block_dim < 1) { - LOG_ERROR("run() reached with unresolved block_dim; prepare_launch_shape must run first"); + LOG_ERROR("run() reached with unresolved block_dim; activate_launch_shape must run first"); return -1; } int num_aicore = block_dim * cores_per_blockdim_; diff --git a/src/a5/platform/onboard/host/device_runner.cpp b/src/a5/platform/onboard/host/device_runner.cpp index 0438eb54a6..3081049aa6 100644 --- a/src/a5/platform/onboard/host/device_runner.cpp +++ b/src/a5/platform/onboard/host/device_runner.cpp @@ -141,8 +141,8 @@ int DeviceRunner::run(Runtime &runtime, const CallConfig &config) { // Latch this run's diagnostic enables onto the runner before the collector // paths below read them; block_dim/aicpu_thread_num are consumed locally. apply_call_config(config); - // prepare_launch_shape() resolved block_dim before the graph was built, so - // the geometry this run launches with is already on the runner. + // activate_launch_shape() latches this run's geometry onto the runner on the + // executor thread immediately before run(), so block_dim_ is this run's. const int block_dim = block_dim_; int launch_aicpu_num = config.aicpu_thread_num; // A prior AICore launch/sync error poisoned the device context and the @@ -175,7 +175,7 @@ int DeviceRunner::run(Runtime &runtime, const CallConfig &config) { ensure_device_wall_buffer(); if (block_dim < 1) { - LOG_ERROR("run() reached with unresolved block_dim; prepare_launch_shape must run first"); + LOG_ERROR("run() reached with unresolved block_dim; activate_launch_shape must run first"); return -1; } int num_aicore = block_dim * cores_per_blockdim_; diff --git a/src/common/platform/include/host/run_stream_slots.h b/src/common/platform/include/host/run_stream_slots.h index 194a164747..d5b18c499b 100644 --- a/src/common/platform/include/host/run_stream_slots.h +++ b/src/common/platform/include/host/run_stream_slots.h @@ -13,6 +13,7 @@ #define SRC_COMMON_PLATFORM_INCLUDE_HOST_RUN_STREAM_SLOTS_H_ #include +#include #include #include @@ -33,6 +34,15 @@ * than hand it a fresh stream beside a live one, and teardown needs the handle * to retry. Stream creation and destruction are injected so this state machine * is exercisable without a device. + * + * Threading: a `Slot` is touched only by the thread that owns that slot's run, + * and admission gives at most one owner per slot, so the per-slot handles need + * no synchronization. Two slots are therefore serviced concurrently — a native + * prepare acquires the successor's slot while the executor retires the + * predecessor's. `created_count_` is the one field shared across those owners + * and is additionally readable from an unrelated thread through + * `get_run_stream_set_create_count`, so it is atomic. `destroy_all()` walks + * every slot and requires all runs to be quiesced. */ class RunStreamSlots { public: @@ -64,7 +74,7 @@ class RunStreamSlots { s.aicore = nullptr; return rc; } - ++created_count_; + created_count_.fetch_add(1, std::memory_order_relaxed); return 0; } @@ -99,7 +109,7 @@ class RunStreamSlots { void *aicpu(unsigned slot) const { return slot < slots_.size() ? slots_[slot].aicpu : nullptr; } void *aicore(unsigned slot) const { return slot < slots_.size() ? slots_[slot].aicore : nullptr; } bool ready(unsigned slot) const { return aicpu(slot) != nullptr && aicore(slot) != nullptr; } - size_t created_count() const { return created_count_; } + size_t created_count() const { return created_count_.load(std::memory_order_relaxed); } static constexpr size_t capacity() { return PTO_PIPELINE_MAX_DEPTH; } private: @@ -111,7 +121,7 @@ class RunStreamSlots { CreateFn create_; DestroyFn destroy_; std::array slots_{}; - size_t created_count_{0}; + std::atomic created_count_{0}; }; #endif // SRC_COMMON_PLATFORM_INCLUDE_HOST_RUN_STREAM_SLOTS_H_ diff --git a/src/common/platform/onboard/host/device_runner_base.cpp b/src/common/platform/onboard/host/device_runner_base.cpp index 16f8a6961a..a8f9b4c3de 100644 --- a/src/common/platform/onboard/host/device_runner_base.cpp +++ b/src/common/platform/onboard/host/device_runner_base.cpp @@ -86,23 +86,30 @@ void create_run_selection_key() { g_run_selection_key_error = pthread_key_create(&g_run_selection_key, std::free); } -DeviceRunnerBase::NativeRunThreadSelection &run_selection() { +/** This thread's selection storage, or nullptr when it cannot be installed. */ +NativeRunThreadSelection *try_run_selection() noexcept { int once_rc = pthread_once(&g_run_selection_once, create_run_selection_key); - if (once_rc != 0 || g_run_selection_key_error != 0) { - throw std::runtime_error("failed to create native-run pthread TLS key"); - } + if (once_rc != 0 || g_run_selection_key_error != 0) return nullptr; auto *selection = static_cast(pthread_getspecific(g_run_selection_key)); if (selection == nullptr) { void *storage = std::malloc(sizeof(NativeRunThreadSelection)); - if (storage == nullptr) throw std::bad_alloc(); + if (storage == nullptr) return nullptr; selection = new (storage) NativeRunThreadSelection{}; int set_rc = pthread_setspecific(g_run_selection_key, selection); if (set_rc != 0) { selection->~NativeRunThreadSelection(); std::free(storage); - throw std::runtime_error("failed to install native-run pthread TLS state"); + return nullptr; } } + return selection; +} + +DeviceRunnerBase::NativeRunThreadSelection &run_selection() { + NativeRunThreadSelection *selection = try_run_selection(); + if (selection == nullptr) { + throw std::runtime_error("failed to install native-run pthread TLS state"); + } return *selection; } @@ -196,7 +203,15 @@ DeviceRunnerBase::NativeRunThreadSelection DeviceRunnerBase::capture_native_run_ } void DeviceRunnerBase::restore_native_run_thread_selection(const NativeRunThreadSelection &selection) noexcept { - run_selection() = selection; + NativeRunThreadSelection *target = try_run_selection(); + if (target == nullptr) { + // Returning would leave this thread on the default slot and bank, so a + // run would address storage another run's lease owns. There is no + // caller-visible channel for the failure on a freshly started thread. + LOG_ERROR("native-run thread selection storage could not be installed"); + std::abort(); + } + *target = selection; } uint64_t DeviceRunnerBase::arena_bank_gm_heap_base(uint32_t bank_id) const { diff --git a/src/common/platform/onboard/host/device_runner_base.h b/src/common/platform/onboard/host/device_runner_base.h index e5634d7dfd..acbf910e2b 100644 --- a/src/common/platform/onboard/host/device_runner_base.h +++ b/src/common/platform/onboard/host/device_runner_base.h @@ -137,6 +137,12 @@ class DeviceRunnerBase { uint32_t pipeline_slot() const; uint32_t selected_arena_bank() const; NativeRunThreadSelection capture_native_run_thread_selection() const; + /** + * Install `selection` on the calling thread. Aborts if the per-thread + * storage cannot be created: every caller either runs inside a scope guard + * or on a thread that has not started its run yet, so proceeding on the + * default slot and bank would silently address another lease's storage. + */ void restore_native_run_thread_selection(const NativeRunThreadSelection &selection) noexcept; /** diff --git a/src/common/platform/sim/host/c_api_shared.cpp b/src/common/platform/sim/host/c_api_shared.cpp index d9e510b135..65112473d4 100644 --- a/src/common/platform/sim/host/c_api_shared.cpp +++ b/src/common/platform/sim/host/c_api_shared.cpp @@ -785,6 +785,11 @@ int set_task_accepted_state_ctx(DeviceContextHandle ctx, volatile int32_t *state } } +/** + * Simulation keeps no per-thread run selection, so the identity is carried only + * by the onboard runner's trace attributes and is discarded here. Accepting it + * keeps the pipeline symbol set uniform across every host runtime. + */ int set_native_run_identity_ctx(DeviceContextHandle ctx, uint64_t, uint64_t, uint64_t, uint64_t) { return ctx == NULL ? -1 : 0; } diff --git a/src/common/worker/chip_worker.cpp b/src/common/worker/chip_worker.cpp index 1e2866ed3b..42add01d73 100644 --- a/src/common/worker/chip_worker.cpp +++ b/src/common/worker/chip_worker.cpp @@ -38,6 +38,7 @@ T load_symbol(void *handle, const char *name) { msg += name; msg += "': "; msg += err; + msg += "; every host runtime built from this source tree exports it, so rebuild the runtime"; throw std::runtime_error(msg); } return reinterpret_cast(sym); @@ -668,7 +669,9 @@ ChipWorkerNativeRun ChipWorker::prepare_native_run_on_slot( ); } } - if (occupied != 0 && occupied != 1) { + // The loop above already rejected every predecessor that may not carry a + // successor, so more than one survivor means a successor is staged. + if (occupied > 1) { throw std::runtime_error( "prepare_native_run already owns a prepared successor " + format_native_run_identity(run_identity) );