Arm the resident DFX collectors under the execution claim - #2163
Merged
Conversation
The collectors are resident objects that serve every run, so opening a run's collection window is destructive: begin_run() drops the previous run's records and counters and republishes the level the device reads. That ran during prepare_execution, which on onboard can run for a successor while a predecessor is still executing — it would have reset the collectors out from under the live run. Move it to launch, where the run holds the execution claim and is therefore the only run touching those collectors. begin_run() now sits beside the start() it belongs with, in start_shared_collectors_for_run() on both bases, which collapses five per-runner copies into one per platform. The level cannot move with it: chip-swimlane sizes its orch phase pool from the level and args-dump writes it into DumpDataHeader, and both happen in initialize(), which must stay in prepare because its device pointers go into kernel_args before it uploads. So initialize() takes the level as an argument instead of reading what begin_run() latched, which also removes an ordering constraint between two calls that no longer share state. Launch and drain read the run's configuration from its own PreparedExecution rather than from the runner, which needs no new plumbing: PreparedExecution already carried the run's CallConfig by value, so it now resolves DfxRunConfig from it at construction. The two shared-collector bracket helpers and the host-phase-records writer take it as an argument. Six runner members are dead as a result — enable_chip_swimlane_, enable_dump_args_, enable_pmu_, enable_scope_stats_, pmu_event_type_, capture_clock_anchors_ — with every remaining occurrence a declaration or a setter assignment. They are removed with their setters. Leaving a write-only enable_pmu_ on the runner is the exact shape of the defect this change is about: it reads as the source of truth and is not one. chip_swimlane_level_ stays because a device-context query answers from it, and output_prefix_ because host_phase_pool_arm() reads it. a5's PMU degradation becomes a per-run value. When init_pmu fails, a5 disables PMU for that run rather than failing it; that flag was the one DFX value prepare still wrote runner-wide, and it now lands on the run's own dfx, which its arming and its teardown both read. a2a3 fails the whole run on the same error — an undocumented arch divergence, filed as part of hw-native-sys#2162 and left alone here. This does not remove the diagnostics depth-one gate. What remains behind it is pool construction, not per-run arming: a prepare whose collector shape differs from the resident one calls finalize_collectors(), which frees device memory the predecessor is using. That is a narrower condition than "any diagnostic is on" and is the next change on this line. Sim takes the same shape but not the same fix: it acquires the execution claim at the top of simpler_prepare_run, so it never prepares a successor against a live predecessor. Keeping the two runner shapes identical is the point — see hw-native-sys#2162. Verified per DFX channel in the shapes _st-sim-{a2a3,a5}.yml uses; a bare sweep enables no channel and cannot fail on a collector defect. 12 channel runs, 23 cases, green on both sim platforms. The negative control fires on the relocated code: with begin_run() omitted from start_shared_collectors_for_run(), all 5 a5sim swimlane cases fail. Full sim sweeps green on both platforms (33 and 29 cases), pyut 2216 passed / 7 skipped, cpput 135/135 from a cleared build dir — which is what caught the three test call sites of the changed initialize() — and a2a3 onboard smokes green under task-submit across PMU, chip_swimlane + dep_gen, hbg dep_gen and args_dump: 11 cases.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (18)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughDFX configuration now lives on each ChangesPer-run DFX lifecycle
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to Runs that change chip-swimlane diagnostic level may continue using the prior run’s collector level, producing incomplete or incorrect diagnostics. This should be corrected before merge. Sequence Diagram(s)sequenceDiagram
participant CallConfig
participant PreparedExecution
participant DeviceRunner
participant Collectors
CallConfig->>PreparedExecution: create DfxRunConfig
PreparedExecution->>DeviceRunner: provide prepared.dfx
DeviceRunner->>Collectors: begin_run and start
DeviceRunner->>Collectors: teardown with dfx
Collectors-->>DeviceRunner: write configured artifacts
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. A rabbit reads each line, Comment |
ChaoWao
added a commit
that referenced
this pull request
Sep 12, 2026
) The collector pools are resident and shared by every run on a runner, and a run whose core / AICPU-thread counts differ from theirs releases and rebuilds them. Releasing frees device memory the collectors hold, so it is only safe while no other run is executing against them — and it ran during prepare_execution, which on onboard can run for a successor while a predecessor is still executing. #2163 moved the per-run arming (begin_run) into the launch path for this reason and left the pool construction behind, because the pools produce the device pointers that go into KernelArgs, which prepare uploads at its end. That constraint turns out not to be one: KernelArgsHelper::init_device_kernel_args allocates once and memcpys on every call, so the copy can be refreshed after launch-time arming. The two device readers differ, and only one needs it — AICore's KERNEL_ENTRY reads the profiling flag and the swimlane / PMU ring tables out of that device copy, while the AICPU receives the host-side struct as its launch argument blob. So the whole block moves: the shape check and rebuild, the five init_*, and the profiling-flag bitfield, into a per-runner arm_collectors_for_run() called from the launch arming, followed by a KernelArgs refresh when the run has any diagnostic on. Preparation now touches no collector state at all. Moving the flag with the pools is not incidental. It is built from the same host-orch check init_dep_gen makes, and a5 degrades PMU for a run whose init_pmu fails — with the flag built in prepare, that run shipped a flag still advertising a channel it had just switched off. The cost lands only where diagnostics are already on: the first run with a channel enabled pays its pool allocation on the launch path, and every run with a channel on pays one sizeof(KernelArgs) H2D. initialize() is idempotent, so later runs with an unchanged shape pay only the memcpy. This removes the last reason the diagnostics depth-one gate exists. The gate itself stays here and comes off in its own change, where the evidence that it is gone has to be a pipeline depth measurement rather than a passing suite. Verified per DFX channel in the shapes _st-sim-{a2a3,a5}.yml uses; a bare sweep enables no channel and cannot fail on a collector defect. 12 channel runs, 23 cases, green on both sim platforms. The negative control targets the one genuinely new mechanism, the KernelArgs refresh, and is therefore an onboard run — sim has no device copy. With the refresh suppressed, 2 of 4 a2a3 onboard swimlane cases fail and the other 2 pass, which is the expected shape: the AICPU half still receives the host struct at launch, so only the AICore-side records go missing. Full sim sweeps green on both platforms (39 and 35 cases), pyut 2242 passed / 7 skipped, cpput 140/140 from a cleared build dir, and a2a3 onboard smokes green under task-submit across PMU, chip_swimlane + dep_gen, hbg dep_gen and args_dump: 11 cases.
ChaoWao
added a commit
to ChaoWao/simpler-fork
that referenced
this pull request
Sep 12, 2026
`allow_prepared_successor` folded in `CallConfig::diagnostics_any()` — the OR of all five diagnostic flags — so enabling any one of them dropped the pipeline back to depth one. The reason was that a collector's setup wrote runner-global state during preparation, which a prepared successor would have done while its predecessor was still executing against it. That is no longer true. hw-native-sys#2163 moved the per-run arming and hw-native-sys#2200 moved the pool construction, the profiling flag and the shape-driven rebuild into `arm_collectors_for_run()`, which runs from the launch arming under the execution claim. A successor's preparation now touches no collector state at all, so the term comes out of all four places that carried it: the c_api's prepare admission, the L3 scheduler's prepared-lane dispatch, the chip run lane's staging predicate, and ChipWorker's slot admission. Removing it also dissolves an asymmetry rather than needing one fixed: the lane checked both the predecessor's and the successor's config while the other two checked only the successor's. With no config term left there is nothing to disagree about, and `permits_native_successor` loses its now-unused parameter and its forwarding overload with it. The three tests that asserted the serialization are converted, not deleted. Each now requires the opposite verdict, which is what makes the removal enforced rather than claimed: - the `concurrent_prepare_stress` diagnostics arm (a2a3 onboard) requires the successor's `bind` to overlap the predecessor's `runner_run` - `ChipRunLaneTest.DiagnosticSuccessorPreparesLikeAnyOther` requires `NATIVE_PREPARED` and the staged event order - `ProgressSchedulerFixture.DiagnosticSuccessorStagesLikeAnyOther` requires `prepare_only` on the successor's dispatch The scene-test arm's own docstring said to delete it when the fallback retired. Converting it is worth more: the lane declines to stage silently rather than raising, so a reinstated special case would show up as nothing but lost pipeline depth, with goldens and artifacts unchanged. Its serial-submission sibling stays as the negative control that keeps the overlap verdict a detector rather than a formality. Verified on a2a3 onboard, where the property is real: the stress passes 4/4, and restoring the lane's `diagnostics_any()` term alone turns exactly one arm red — the diagnostics one — with the other three still green. Also green: 12 DFX channel runs over both sim platforms, full sim sweeps (39 and 35 cases), pyut 2242 passed / 7 skipped, and cpput 140/140 from a cleared build dir, which is what caught the two unit tests above. One unrelated intermittent failure was seen during the sweep and is recorded in KNOWN_ISSUES.md: the a5sim `chip_swimlane` suite segfaulted once in six runs inside the L2 native wait. It is not reachable from this change — three of the four hunks sit behind `supports_concurrent_native_prepare()`, which sim hardcodes to 0, and the fourth is on the L3 orchestrator path while the crash is on the L2 `chip_run.wait` path.
ChaoWao
added a commit
to ChaoWao/simpler-fork
that referenced
this pull request
Sep 12, 2026
`allow_prepared_successor` folded in `CallConfig::diagnostics_any()` — the OR of all five diagnostic flags — so enabling any one of them dropped the pipeline back to depth one. The reason was that a collector's setup wrote runner-global state during preparation, which a prepared successor would have done while its predecessor was still executing against it. That is no longer true. hw-native-sys#2163 moved the per-run arming and hw-native-sys#2200 moved the pool construction, the profiling flag and the shape-driven rebuild into `arm_collectors_for_run()`, which runs from the launch arming under the execution claim. A successor's preparation now touches no collector state at all, so the term comes out of all four places that carried it: the c_api's prepare admission, the L3 scheduler's prepared-lane dispatch, the chip run lane's staging predicate, and ChipWorker's slot admission. Removing it also dissolves an asymmetry rather than needing one fixed: the lane checked both the predecessor's and the successor's config while the other two checked only the successor's. With no config term left there is nothing to disagree about, and `permits_native_successor` loses its now-unused parameter and its forwarding overload with it. One piece of per-run setup did still ride on `apply_call_config`, which a prepare skips when it overlaps an active predecessor: `set_dep_gen_enabled`, whose only surviving effect is `dep_gen_host_graph_set_enabled`. A host-orchestrating runtime holds the captured graph in thread-local state between orchestration and emit, so an overlapping prepare that never armed it bound with no capture and emitted no deps.json. It is now armed unconditionally from the run's own config, on the thread that is about to bind, by `arm_host_dep_gen_capture` — renamed because setting an enable flag was the half of it that had already become dead: `enable_dep_gen_` was write-only on all four runners and is removed with the setter. The four tests that asserted the serialization are converted, not deleted. Each now requires the opposite verdict, which is what makes the removal enforced rather than claimed: - the `concurrent_prepare_stress` diagnostics arm (a2a3 onboard) requires the successor's `bind` to overlap the predecessor's `runner_run` - `native_run_lifecycle` runs both diagnostic pairs to completion instead of expecting the successor to be refused, and its two span counters move with the two runs that now launch - `ChipRunLaneTest.DiagnosticSuccessorPreparesLikeAnyOther` requires `NATIVE_PREPARED` and the staged event order - `ProgressSchedulerFixture.DiagnosticSuccessorStagesLikeAnyOther` requires `prepare_only` on the successor's dispatch The scene-test arm's own docstring said to delete it when the fallback retired. Converting it is worth more: the lane declines to stage silently rather than raising, so a reinstated special case would show up as nothing but lost pipeline depth, with goldens and artifacts unchanged. Its serial-submission sibling stays as the negative control that keeps the overlap verdict a detector rather than a formality. Verified on a2a3 onboard, where the property is real: the stress passes 4/4, and restoring the lane's `diagnostics_any()` term alone turns exactly one arm red — the diagnostics one — with the other three still green. The whole a2a3 onboard host_build_graph suite is green in the per-PR CI shape; running only the arm that asserts the property is what let the dep_gen arming defect and the lifecycle assertions reach CI. Also green: 12 DFX channel runs over both sim platforms, full sim sweeps (39 and 35 cases), pyut 2242 passed / 7 skipped, and cpput 140/140 from a cleared build dir, which is what caught the two unit tests above. One unrelated intermittent failure was seen during the sweep and is recorded in KNOWN_ISSUES.md: the a5sim `chip_swimlane` suite segfaulted once in six runs inside the L2 native wait. It is not reachable from this change — three of the four hunks sit behind `supports_concurrent_native_prepare()`, which sim hardcodes to 0, and the fourth is on the L3 orchestrator path while the crash is on the L2 `chip_run.wait` path.
ChaoWao
added a commit
to ChaoWao/simpler-fork
that referenced
this pull request
Sep 12, 2026
`allow_prepared_successor` folded in `CallConfig::diagnostics_any()` — the OR of all five diagnostic flags — so enabling any one of them dropped the pipeline back to depth one. The reason was that a collector's setup wrote runner-global state during preparation, which a prepared successor would have done while its predecessor was still executing against it. hw-native-sys#2163 moved the per-run arming and hw-native-sys#2200 moved the pool construction, the profiling flag and the shape-driven rebuild into `arm_collectors_for_run()`, which runs from the launch arming under the execution claim. So preparation no longer touches collector state, and the broad term comes out of all four places that carried it: the c_api's prepare admission, the L3 scheduler's prepared-lane dispatch, the chip run lane's staging predicate, and ChipWorker's slot admission. One configuration keeps its serialization, under a narrow term rather than the OR. A host-orchestrating bind calls `host_phase_pool_arm`, which opens a clock-correlation session on the *resident* swimlane collector and samples its `HostOrchestrationBegin` anchor into it, and which arms the runner's single host-phase record store. Neither is per-run, and neither can move under the claim the way the pools did: the anchor's meaning is when host orchestration began, and host orchestration is the bind. So a run at chip-swimlane level 4 — or under the `SIMPLER_HBG_HOST_PHASE_RECORDS_ENABLE` opt-in, which arms the same store — neither carries a prepared successor nor is one. `CallConfig::captures_host_orchestration_phases()` names that condition once; `dfx_run_config.h` is the only header that sees both it and `ChipSwimlaneLevel`, so the static_assert tying the two lives there. The opt-in is read through a weak `host_phase_records_enabled()` whose strong definition is in the host_build_graph runtime, matching the existing dep_gen stubs beside it. It is declared with C++ linkage on purpose: giving it C linkage would mangle to a different symbol that the weak definition satisfies unconditionally, so the opt-in would read `false` even where the strong one is linked and nothing would say so. One piece of per-run setup was still riding on `apply_call_config`, which a prepare skips when it overlaps an active predecessor: `set_dep_gen_enabled`, whose only surviving effect is `dep_gen_host_graph_set_enabled`. A host-orchestrating runtime holds the captured graph in thread-local state between orchestration and emit, so an overlapping prepare that never armed it bound with no capture and emitted no deps.json. It is now armed unconditionally from the run's own config, on the thread that is about to bind, by `arm_host_dep_gen_capture` — renamed because setting an enable flag was the half of it that had already become dead: `enable_dep_gen_` was write-only on all four runners and is removed with the setter. The tests that asserted the old serialization are converted, not deleted, and a new arm holds the boundary that remains: - the `concurrent_prepare_stress` diagnostics arm (a2a3 onboard) requires the successor's `bind` to overlap the predecessor's `runner_run` - a fourth arm at chip-swimlane level 4 requires the opposite, and retires when that state becomes per-run - `native_run_lifecycle` runs both diagnostic pairs to completion instead of expecting the successor to be refused, and its two span counters move with the two runs that now launch - `ChipRunLaneTest.DiagnosticSuccessorPreparesLikeAnyOther` requires `NATIVE_PREPARED` and the staged event order - `ProgressSchedulerFixture.DiagnosticSuccessorStagesLikeAnyOther` requires `prepare_only` on the successor's dispatch The scene-test arm's own docstring said to delete it when the fallback retired. Converting it is worth more: the lane declines to stage silently rather than raising, so a run that lost its overlap and a run that kept an overlap it should not have look identical from outside — goldens pass and artifacts land either way. Verified on a2a3 onboard, where the property is real. Restoring the lane's old `diagnostics_any()` term turns exactly one arm red, the diagnostics one. Removing the new level-4 term turns three red — that arm plus two of its siblings, because the arms share a class-scoped worker and an overlapping level-4 bind corrupts it rather than merely flipping a verdict. The whole a2a3 onboard host_build_graph suite is green in the per-PR CI shape; running only the arm that asserts the property is what let the dep_gen arming defect and the lifecycle assertions reach CI. Also green: 12 DFX channel runs over both sim platforms, full sim sweeps (39 and 35 cases), pyut 2242 passed / 7 skipped, and cpput 140/140 from a cleared build dir, which is what caught the two unit tests above. One unrelated intermittent failure was seen during the sweep and is recorded in KNOWN_ISSUES.md: the a5sim `chip_swimlane` suite segfaulted once in six runs inside the L2 native wait. It is not reachable from this change — the gate sits behind `supports_concurrent_native_prepare()`, which sim hardcodes to 0, and the scheduler hunk is on the L3 orchestrator path while the crash is on the L2 `chip_run.wait` path.
ChaoWao
added a commit
that referenced
this pull request
Sep 12, 2026
`allow_prepared_successor` folded in `CallConfig::diagnostics_any()` — the OR of all five diagnostic flags — so enabling any one of them dropped the pipeline back to depth one. The reason was that a collector's setup wrote runner-global state during preparation, which a prepared successor would have done while its predecessor was still executing against it. #2163 moved the per-run arming and #2200 moved the pool construction, the profiling flag and the shape-driven rebuild into `arm_collectors_for_run()`, which runs from the launch arming under the execution claim. So preparation no longer touches collector state, and the broad term comes out of all four places that carried it: the c_api's prepare admission, the L3 scheduler's prepared-lane dispatch, the chip run lane's staging predicate, and ChipWorker's slot admission. One configuration keeps its serialization, under a narrow term rather than the OR. A host-orchestrating bind calls `host_phase_pool_arm`, which opens a clock-correlation session on the *resident* swimlane collector and samples its `HostOrchestrationBegin` anchor into it, and which arms the runner's single host-phase record store. Neither is per-run, and neither can move under the claim the way the pools did: the anchor's meaning is when host orchestration began, and host orchestration is the bind. So a run at chip-swimlane level 4 — or under the `SIMPLER_HBG_HOST_PHASE_RECORDS_ENABLE` opt-in, which arms the same store — neither carries a prepared successor nor is one. `CallConfig::captures_host_orchestration_phases()` names that condition once; `dfx_run_config.h` is the only header that sees both it and `ChipSwimlaneLevel`, so the static_assert tying the two lives there. The opt-in is read through a weak `host_phase_records_enabled()` whose strong definition is in the host_build_graph runtime, matching the existing dep_gen stubs beside it. It is declared with C++ linkage on purpose: giving it C linkage would mangle to a different symbol that the weak definition satisfies unconditionally, so the opt-in would read `false` even where the strong one is linked and nothing would say so. One piece of per-run setup was still riding on `apply_call_config`, which a prepare skips when it overlaps an active predecessor: `set_dep_gen_enabled`, whose only surviving effect is `dep_gen_host_graph_set_enabled`. A host-orchestrating runtime holds the captured graph in thread-local state between orchestration and emit, so an overlapping prepare that never armed it bound with no capture and emitted no deps.json. It is now armed unconditionally from the run's own config, on the thread that is about to bind, by `arm_host_dep_gen_capture` — renamed because setting an enable flag was the half of it that had already become dead: `enable_dep_gen_` was write-only on all four runners and is removed with the setter. The tests that asserted the old serialization are converted, not deleted, and a new arm holds the boundary that remains: - the `concurrent_prepare_stress` diagnostics arm (a2a3 onboard) requires the successor's `bind` to overlap the predecessor's `runner_run` - a fourth arm at chip-swimlane level 4 requires the opposite, and retires when that state becomes per-run - `native_run_lifecycle` runs both diagnostic pairs to completion instead of expecting the successor to be refused, and its two span counters move with the two runs that now launch - `ChipRunLaneTest.DiagnosticSuccessorPreparesLikeAnyOther` requires `NATIVE_PREPARED` and the staged event order - `ProgressSchedulerFixture.DiagnosticSuccessorStagesLikeAnyOther` requires `prepare_only` on the successor's dispatch The scene-test arm's own docstring said to delete it when the fallback retired. Converting it is worth more: the lane declines to stage silently rather than raising, so a run that lost its overlap and a run that kept an overlap it should not have look identical from outside — goldens pass and artifacts land either way. Verified on a2a3 onboard, where the property is real. Restoring the lane's old `diagnostics_any()` term turns exactly one arm red, the diagnostics one. Removing the new level-4 term turns three red — that arm plus two of its siblings, because the arms share a class-scoped worker and an overlapping level-4 bind corrupts it rather than merely flipping a verdict. The whole a2a3 onboard host_build_graph suite is green in the per-PR CI shape; running only the arm that asserts the property is what let the dep_gen arming defect and the lifecycle assertions reach CI. Also green: 12 DFX channel runs over both sim platforms, full sim sweeps (39 and 35 cases), pyut 2242 passed / 7 skipped, and cpput 140/140 from a cleared build dir, which is what caught the two unit tests above. One unrelated intermittent failure was seen during the sweep and is recorded in KNOWN_ISSUES.md: the a5sim `chip_swimlane` suite segfaulted once in six runs inside the L2 native wait. It is not reachable from this change — the gate sits behind `supports_concurrent_native_prepare()`, which sim hardcodes to 0, and the scheduler hunk is on the L3 orchestrator path while the crash is on the L2 `chip_run.wait` path.
This was referenced Sep 12, 2026
ChaoWao
added a commit
to ChaoWao/simpler-fork
that referenced
this pull request
Sep 12, 2026
The host-phase record store and the clock-correlation provider were one per runner, but their lifetime is one per run: armed during a host-orchestrating bind, read at that run's teardown. A bind is preparation, and a prepared successor prepares while its predecessor is still executing — so the successor's bind reset the store its predecessor had finished and was waiting to publish, and released the provider whose session that predecessor was still running under. Unlike the collector pools, this could not simply move under the execution claim the way hw-native-sys#2163 and hw-native-sys#2200 moved theirs. The records describe the bind, and the `HostOrchestrationBegin` anchor means the instant host orchestration began; deferring either to launch would record something else. hw-native-sys#2201 therefore excluded the affected configurations from pipeline overlap rather than fixing them, which is what this removes. The state is now `std::array<HostPhaseRunState, PTO_PIPELINE_MAX_DEPTH>`, and what reaches the resident swimlane collector is split from what is sampled: - `begin_host_phase_run()` stamps the run's own level and prefix into its slot before its bind, because the runner's members describe whichever run last held the claim - `host_phase_pool_arm()` and `capture_clock_correlation_begin()` write only that slot — no collector writes at all during bind - `publish_host_phase_run_to_collector()` hands the session, the anchors and `set_host_orchestrated` to the collector from the launch arming, under the claim — after the stale-shape `finalize_collectors()`, which resets both, and before the `initialize()` that reads `host_orchestrated_` when it decides whether to size a device orch phase pool The two `HostApiOps` host-phase hooks take a `uint32_t pipeline_slot`, matching the five sibling hooks that already do. No new plumbing was needed to supply it: `HostApi` is already constructed per run and already carries `pipeline_slot_`, and the teardown readers have it on `PreparedExecution`. `HostPhaseRecordStore` deletes copy and move on purpose — `pool_` holds raw pointers into `buffers_` — so the array holds it in place rather than moving it onto `PreparedExecution`. The providers are per slot but the collector's clock-correlation session is not, so the runner records which slot opened it and only that slot may end it. A prepared successor that fails while its predecessor is executing reaches `finish_clock_correlation_session` for its own slot; without the owner check it would end the predecessor's session, costing that run its `DeviceExecutionComplete` anchor and exporting a correlation with no closing edge. With that, the exclusion and everything that supported it goes: the `captures_host_orchestration_phases()` predicate, its four gate sites, the weak `host_phase_records_enabled()` read, and the static_assert pinning the level literal. Any diagnostics configuration now overlaps its predecessor. The scene test's level-4 arm was written for this and flips from requiring `did not overlap` to requiring the overlap. Overlap alone would not have detected a regression, though — collapsing the state back to one store still overlaps, it just loses a run's records — so the arm now also gives each iteration its own output directory and requires every one of them to carry `orchestrator_source: host` in its swimlane artifact. That marker is written only when a run's own host-phase records reached the collector. Verified on a2a3 onboard. The negative control indexes the array at 0 instead of the run's slot: the overlap assertion alone stays green, and the artifact assertion fails with `run 0: its host-orchestration phase records never reached the collector`. The whole a2a3 onboard host_build_graph suite is green in the per-PR CI shape. Also green: 12 DFX channel runs over both sim platforms, full sim sweeps (39 and 35 cases), pyut 2242 passed / 7 skipped, and cpput 140/140 from a cleared build dir.
ChaoWao
added a commit
that referenced
this pull request
Sep 12, 2026
The host-phase record store and the clock-correlation provider were one per runner, but their lifetime is one per run: armed during a host-orchestrating bind, read at that run's teardown. A bind is preparation, and a prepared successor prepares while its predecessor is still executing — so the successor's bind reset the store its predecessor had finished and was waiting to publish, and released the provider whose session that predecessor was still running under. Unlike the collector pools, this could not simply move under the execution claim the way #2163 and #2200 moved theirs. The records describe the bind, and the `HostOrchestrationBegin` anchor means the instant host orchestration began; deferring either to launch would record something else. #2201 therefore excluded the affected configurations from pipeline overlap rather than fixing them, which is what this removes. The state is now `std::array<HostPhaseRunState, PTO_PIPELINE_MAX_DEPTH>`, and what reaches the resident swimlane collector is split from what is sampled: - `begin_host_phase_run()` stamps the run's own level and prefix into its slot before its bind, because the runner's members describe whichever run last held the claim - `host_phase_pool_arm()` and `capture_clock_correlation_begin()` write only that slot — no collector writes at all during bind - `publish_host_phase_run_to_collector()` hands the session, the anchors and `set_host_orchestrated` to the collector from the launch arming, under the claim — after the stale-shape `finalize_collectors()`, which resets both, and before the `initialize()` that reads `host_orchestrated_` when it decides whether to size a device orch phase pool The two `HostApiOps` host-phase hooks take a `uint32_t pipeline_slot`, matching the five sibling hooks that already do. No new plumbing was needed to supply it: `HostApi` is already constructed per run and already carries `pipeline_slot_`, and the teardown readers have it on `PreparedExecution`. `HostPhaseRecordStore` deletes copy and move on purpose — `pool_` holds raw pointers into `buffers_` — so the array holds it in place rather than moving it onto `PreparedExecution`. The providers are per slot but the collector's clock-correlation session is not, so the runner records which slot opened it and only that slot may end it. A prepared successor that fails while its predecessor is executing reaches `finish_clock_correlation_session` for its own slot; without the owner check it would end the predecessor's session, costing that run its `DeviceExecutionComplete` anchor and exporting a correlation with no closing edge. With that, the exclusion and everything that supported it goes: the `captures_host_orchestration_phases()` predicate, its four gate sites, the weak `host_phase_records_enabled()` read, and the static_assert pinning the level literal. Any diagnostics configuration now overlaps its predecessor. The scene test's level-4 arm was written for this and flips from requiring `did not overlap` to requiring the overlap. Overlap alone would not have detected a regression, though — collapsing the state back to one store still overlaps, it just loses a run's records — so the arm now also gives each iteration its own output directory and requires every one of them to carry `orchestrator_source: host` in its swimlane artifact. That marker is written only when a run's own host-phase records reached the collector. Verified on a2a3 onboard. The negative control indexes the array at 0 instead of the run's slot: the overlap assertion alone stays green, and the artifact assertion fails with `run 0: its host-orchestration phase records never reached the collector`. The whole a2a3 onboard host_build_graph suite is green in the per-PR CI shape. Also green: 12 DFX channel runs over both sim platforms, full sim sweeps (39 and 35 cases), pyut 2242 passed / 7 skipped, and cpput 140/140 from a cleared build dir.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The DFX collectors are resident objects that serve every run, so opening a run's collection window is destructive:
begin_run()drops the previous run's records and counters and republishes the level the device reads. That ran duringprepare_execution, which on onboard can run for a successor while a predecessor is still executing.This moves it to launch, where the run holds the execution claim and is the only run touching those collectors — and carries the run's own DFX configuration through launch and drain instead of reading the runner's members.
Part of #2078.
What changed
begin_run()× 5 collectors × 4 runnersstart_shared_collectors_for_run()on each base, beside thestart()it belongs withinitialize()for chip-swimlane / args-dumpbegin_run()latchedPreparedExecution::dfx, not runner membersdfx.pmu_enabled = false— a per-run value, no longer a runner-wide writeinitialize()could not move withbegin_run(): chip-swimlane sizes its orch phase pool from the level and args-dump writes it intoDumpDataHeader, and its device pointers must be inkernel_argsbefore that uploads at the end of prepare. Passing the level in removes the ordering constraint between two calls that no longer share state.PreparedExecutionalready carried the run'sCallConfigby value, so carryingDfxRunConfigneeded no new plumbing.Why the dead members go in this commit
enable_chip_swimlane_,enable_dump_args_,enable_pmu_,enable_scope_stats_,pmu_event_type_andcapture_clock_anchors_are now write-only — every remaining occurrence is a declaration or a setter assignment, which is what let the compiler confirm the removal. A write-onlyenable_pmu_on the runner is the exact shape of the defect this change is about: it reads as the source of truth and is not one.chip_swimlane_level_stays (a device-context query answers from it) andoutput_prefix_stays (host_phase_pool_arm()reads it).What this does not do
The diagnostics depth-one gate stays. What remains behind it is pool construction, not per-run arming: a prepare whose collector shape differs from the resident one calls
finalize_collectors(), which frees device memory the predecessor is using. That is a much narrower condition than "any diagnostic is on", and turning the gate into it is the next change on this line.Sim takes the same shape but not the same fix. It acquires the execution claim at the top of
simpler_prepare_run, so it never prepares a successor against a live predecessor. Keeping the two runner shapes identical is the point — see #2162.Testing
Verified per DFX channel in the shapes
_st-sim-{a2a3,a5}.ymluses. A bare sweep enables no channel and so cannot fail on a collector defect.begin_run()omitted fromstart_shared_collectors_for_run(), all 5 a5sim swimlane cases failinitialize()task-submitacross PMU, chip_swimlane + dep_gen, hbg dep_gen and args_dump: 11 casesRelated: #2078, #2162