From 45a3408e532cf6d5f183115a8f96b985d8885ed3 Mon Sep 17 00:00:00 2001 From: Chao Wang <26245345+ChaoWao@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:54:13 -0700 Subject: [PATCH] Fix: initialise dep_gen on the orchestrator, not behind the handshake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dep_gen_aicpu_init() resolves the collector's buffer state and pops its first buffer, and it ran from the scheduler cold path after the AICore handshake, next to pmu_aicpu_init(). pmu belongs there — it needs the physical_core_ids_ the handshake produces. dep_gen needs nothing from it: init only reads dep_gen_data_base, which kernel.cpp publishes before aicpu_execute() starts. The orchestrator, meanwhile, deliberately skips that handshake: if (decouple_orch && is_orchestrator) { return 0; // do NOT touch the handshake or hs_arrived_ barrier } so it goes straight to building the graph while the schedulers are still handshaking cores. Every submit_task it issues in that window reaches dep_gen_aicpu_record_submit() with s_dep_gen_state still null and is dropped. Whether any records survive is a race against handshake wall time, which scales with core count: at three clusters the schedulers usually win, at a full device they usually do not, and in between runs capture a partial graph missing only its first tasks. Initialising dep_gen on the thread that feeds it removes the race — there is nothing left to order it against. Both cold-path call sites go, not just one: the free_queue is SPSC and the orchestrator is now its only device-side consumer. The failure was silent because the early return skipped the accounting one line below it, the line whose comment reads "Account every attempted record so total == collected + dropped on host". The host's reconcile_counters() saw 0/0/0, called that consistent, and wrote an empty deps.json instead of refusing. A pre-init record cannot be accounted — the counter lives in the BufferState that is still null — so it now reports itself once instead. Once, because record_submit is a per-task path and the AICPU log is not free. a5 carried the identical arrangement and is fixed with it. --- .../aicpu/aicpu_executor.cpp | 15 +++++++++++++-- .../runtime/scheduler/scheduler_cold_path.cpp | 8 ++------ .../aicpu/aicpu_executor.cpp | 15 +++++++++++++-- .../runtime/scheduler/scheduler_cold_path.cpp | 8 ++------ .../shared/aicpu/dep_gen_collector_aicpu.cpp | 19 ++++++++++++++++++- 5 files changed, 48 insertions(+), 17 deletions(-) diff --git a/src/a2a3/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp b/src/a2a3/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp index 52163b981c..e4f4e79ed8 100644 --- a/src/a2a3/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp +++ b/src/a2a3/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp @@ -695,9 +695,20 @@ int32_t AicpuExecutor::run(Runtime *runtime) { #if SIMPLER_DFX // dep_gen plugs into the orchestrator thread (single-instance subsystem): - // record the per-thread ready_queue index before any submit_task fires - // inside orch_func_. + // resolve its buffer state and record the per-thread ready_queue index + // before any submit_task fires inside orch_func_. The init belongs to + // this thread, not to the scheduler cold path: dep_gen needs nothing + // from the AICore handshake, while the orchestrator skips that + // handshake entirely on the decoupled path and starts submitting + // immediately. Initialising it behind the handshake makes the first + // submits race a barrier they never joined — and the wider the device, + // the longer that handshake, so the orchestrator wins more of the race + // the more clusters there are. + // + // The free_queue is SPSC: this must remain the only site that pops + // from it on the device side. if (is_dep_gen_enabled()) { + dep_gen_aicpu_init(); dep_gen_aicpu_set_orch_thread_idx(thread_idx); } diff --git a/src/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cpp b/src/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cpp index 04537c65a7..5986d63438 100644 --- a/src/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cpp +++ b/src/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cpp @@ -981,9 +981,7 @@ void SchedulerContext::post_handshake_profiling_init() { pmu_aicpu_init(physical_core_ids_, cores_total_num_); LOG_INFO_V0("PMU profiling started on %d cores", cores_total_num_); } - if (is_dep_gen_enabled()) { - dep_gen_aicpu_init(); - } + if (is_dep_gen_enabled()) {} #endif } @@ -1224,9 +1222,7 @@ int32_t SchedulerContext::post_handshake_init(Runtime *runtime) { // init() only pops the initial buffer from instance 0's free_queue; the // orchestrator thread still records its idx via // dep_gen_aicpu_set_orch_thread_idx() before the first record_submit. - if (is_dep_gen_enabled()) { - dep_gen_aicpu_init(); - } + if (is_dep_gen_enabled()) {} #endif // total_tasks_ is read in pre_handshake_init (before the orchestrator's early diff --git a/src/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp b/src/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp index 1f6950583a..4d52c1d217 100644 --- a/src/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp +++ b/src/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cpp @@ -696,9 +696,20 @@ int32_t AicpuExecutor::run(Runtime *runtime) { scope_stats_aicpu_set_orch_thread_idx(thread_idx); // dep_gen plugs into the orchestrator thread (single-instance subsystem): - // record the per-thread ready_queue index before any submit_task fires - // inside orch_func_. + // resolve its buffer state and record the per-thread ready_queue index + // before any submit_task fires inside orch_func_. The init belongs to + // this thread, not to the scheduler cold path: dep_gen needs nothing + // from the AICore handshake, while the orchestrator skips that + // handshake entirely on the decoupled path and starts submitting + // immediately. Initialising it behind the handshake makes the first + // submits race a barrier they never joined — and the wider the device, + // the longer that handshake, so the orchestrator wins more of the race + // the more clusters there are. + // + // The free_queue is SPSC: this must remain the only site that pops + // from it on the device side. if (is_dep_gen_enabled()) { + dep_gen_aicpu_init(); dep_gen_aicpu_set_orch_thread_idx(thread_idx); } #endif diff --git a/src/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cpp b/src/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cpp index 30ff2fdcc5..76953dca69 100644 --- a/src/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cpp +++ b/src/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cpp @@ -973,9 +973,7 @@ void SchedulerContext::post_handshake_profiling_init() { pmu_aicpu_init(physical_core_ids_, cores_total_num_); LOG_INFO_V0("PMU profiling started on %d cores", cores_total_num_); } - if (is_dep_gen_enabled()) { - dep_gen_aicpu_init(); - } + if (is_dep_gen_enabled()) {} #endif } @@ -1220,9 +1218,7 @@ int32_t SchedulerContext::post_handshake_init(Runtime *runtime) { // init() only pops the initial buffer from instance 0's free_queue; the // orchestrator thread still records its idx via // dep_gen_aicpu_set_orch_thread_idx() before the first record_submit. - if (is_dep_gen_enabled()) { - dep_gen_aicpu_init(); - } + if (is_dep_gen_enabled()) {} #endif // total_tasks_ is read in pre_handshake_init (before the orchestrator's early diff --git a/src/common/platform/shared/aicpu/dep_gen_collector_aicpu.cpp b/src/common/platform/shared/aicpu/dep_gen_collector_aicpu.cpp index 72191b8506..15c7db4afc 100644 --- a/src/common/platform/shared/aicpu/dep_gen_collector_aicpu.cpp +++ b/src/common/platform/shared/aicpu/dep_gen_collector_aicpu.cpp @@ -42,6 +42,12 @@ static bool g_enable_dep_gen = false; static DepGenDataHeader *s_dep_gen_header = nullptr; static DepGenBufferState *s_dep_gen_state = nullptr; static int s_orch_thread_idx = -1; // set via dep_gen_aicpu_set_orch_thread_idx +// A record arriving before dep_gen_aicpu_init() cannot be accounted: the +// counter reconcile_counters() reads lives in the very BufferState that is +// still null, so the host would see total == collected == dropped == 0 and +// call an empty deps.json clean. One-shot so a per-task path never floods the +// AICPU log (see .claude/rules/codestyle.md). +static bool s_pre_init_drop_reported = false; static constexpr uint64_t kDepGenQueueBackpressureWaitCycles = PLATFORM_DFX_BACKPRESSURE_TIMEOUT_CYCLES; @@ -133,6 +139,7 @@ void dep_gen_aicpu_init() { LOG_ERROR("dep_gen_aicpu_init: dep_gen_data_base is NULL"); return; } + s_pre_init_drop_reported = false; s_dep_gen_header = get_dep_gen_header(base); s_dep_gen_state = get_dep_gen_buffer_state(base, /*instance_index=*/0); @@ -156,7 +163,17 @@ void dep_gen_aicpu_record_submit( const uint8_t *arg_types, int explicit_dep_count, const uint64_t *explicit_deps_raw, int block_num, const int32_t kernel_ids[3] ) { - if (!g_enable_dep_gen || s_dep_gen_state == nullptr) { + if (!g_enable_dep_gen) { + return; + } + if (s_dep_gen_state == nullptr) { + if (!s_pre_init_drop_reported) { + s_pre_init_drop_reported = true; + LOG_ERROR( + "dep_gen: submit recorded before dep_gen_aicpu_init(); this and any further " + "pre-init records are lost and cannot be reconciled on the host" + ); + } return; }