Category
Technical Debt (cleanup, refactor)
Component
AICPU Scheduler
Description
Follow-up to #2095 (do after it merges, as one unified change, with a performance measurement).
The per-task progress byte (progress_flags[], bit0 = COMPLETED, bit1 = PUBLISHED) conceptually encodes a strictly linear progression — pending → published → completed; a task is in exactly one state at a time. The current implementation expresses that progression through bit-containment values (0x0 → 0x2 → 0x3) so that a lock-free fetch_or doubles as a monotone-max: the PUBLISHED bookkeeping write races the COMPLETED store, because the bookkeeping runs after the final MMIO token write while the FIN → completion chain forks off that same token write. A plain store with sequential enum values (0/1/2) would let a late publish write regress an already-completed byte, which livelocks the wake-list sentinel protocol.
The cleaner long-term shape is ordering by construction instead of monotonicity by encoding: hoist the final publisher's bookkeeping (the total-reaching fetch_add + PUBLISHED store, in record_published_blocks) to before its batch's MMIO token writes (after payload prepare, before the token flush). That closes the causal chain — PUBLISHED ≺ token ≺ FIN ≺ COMPLETED — so the byte can become a genuine sequential enum written with plain stores, matching the mental model task_state already follows (whose PENDING → COMPLETED → CONSUMED transitions are all causally serialized; TMR additionally guards COMPLETED → CONSUMED with an expected-value CAS).
Safety argument already worked out in review: cores are claimed at prepare time, before any bookkeeping, so setting PUBLISHED ahead of the token writes cannot let a staged consumer occupy cores the producer's in-flight blocks need; and with concurrent publishers, the total-reaching thread's bit store still precedes its own tokens, whose FINs gate the all-FIN completion, so the chain holds regardless of sibling threads' token timing.
Scope of the unified change:
- Split the publish accounting around the token flush at the three publish sites (
dispatch_shape, stage_consumer_blocks, stage_sync_start_cores), both arches: pre-account before the flush, seal after.
- Re-encode the progress byte as a sequential enum (e.g.
TASK_PROGRESS_NONE/PUBLISHED/COMPLETED), replacing TASK_FLAG_* and the fetch_or with plain stores; update the byte-level readers (is_completion_flag_set / is_publish_flag_set) to ordered comparisons.
- Re-audit the ED publish-list seal placement (it may stay after the flush; only the bit/count ordering must move).
- Measure: dispatch-path cost before/after (the change resequences the hot publish loop), via the qwen device A/B protocol on pinned dies plus the
QPROBE/swimlane probes, and hbg-bind-phases to confirm host side untouched.
Until then the fetch_or encoding is correct and validated; this issue is about expressing the state model directly rather than through an encoding trick.
Location
src/common/host_build_graph/shared_memory.h (progress_flags, TASK_FLAG_*, byte helpers)
src/{a2a3,a5}/runtime/host_build_graph/runtime/scheduler/scheduler.h (record_published_blocks, seal_ed_publish_list)
src/{a2a3,a5}/runtime/host_build_graph/runtime/scheduler/scheduler_dispatch.cpp / scheduler_completion.cpp (publish sites around flush_publish)
Priority
Low (no impact today, good to fix eventually)
Category
Technical Debt (cleanup, refactor)
Component
AICPU Scheduler
Description
Follow-up to #2095 (do after it merges, as one unified change, with a performance measurement).
The per-task progress byte (
progress_flags[], bit0 = COMPLETED, bit1 = PUBLISHED) conceptually encodes a strictly linear progression — pending → published → completed; a task is in exactly one state at a time. The current implementation expresses that progression through bit-containment values (0x0 → 0x2 → 0x3) so that a lock-freefetch_ordoubles as a monotone-max: the PUBLISHED bookkeeping write races the COMPLETED store, because the bookkeeping runs after the final MMIO token write while the FIN → completion chain forks off that same token write. A plain store with sequential enum values (0/1/2) would let a late publish write regress an already-completed byte, which livelocks the wake-list sentinel protocol.The cleaner long-term shape is ordering by construction instead of monotonicity by encoding: hoist the final publisher's bookkeeping (the total-reaching
fetch_add+ PUBLISHED store, inrecord_published_blocks) to before its batch's MMIO token writes (after payload prepare, before the token flush). That closes the causal chain — PUBLISHED ≺ token ≺ FIN ≺ COMPLETED — so the byte can become a genuine sequential enum written with plain stores, matching the mental modeltask_statealready follows (whose PENDING → COMPLETED → CONSUMED transitions are all causally serialized; TMR additionally guards COMPLETED → CONSUMED with an expected-value CAS).Safety argument already worked out in review: cores are claimed at prepare time, before any bookkeeping, so setting PUBLISHED ahead of the token writes cannot let a staged consumer occupy cores the producer's in-flight blocks need; and with concurrent publishers, the total-reaching thread's bit store still precedes its own tokens, whose FINs gate the all-FIN completion, so the chain holds regardless of sibling threads' token timing.
Scope of the unified change:
dispatch_shape,stage_consumer_blocks,stage_sync_start_cores), both arches: pre-account before the flush, seal after.TASK_PROGRESS_NONE/PUBLISHED/COMPLETED), replacingTASK_FLAG_*and thefetch_orwith plain stores; update the byte-level readers (is_completion_flag_set/is_publish_flag_set) to ordered comparisons.QPROBE/swimlane probes, andhbg-bind-phasesto confirm host side untouched.Until then the fetch_or encoding is correct and validated; this issue is about expressing the state model directly rather than through an encoding trick.
Location
src/common/host_build_graph/shared_memory.h(progress_flags,TASK_FLAG_*, byte helpers)src/{a2a3,a5}/runtime/host_build_graph/runtime/scheduler/scheduler.h(record_published_blocks,seal_ed_publish_list)src/{a2a3,a5}/runtime/host_build_graph/runtime/scheduler/scheduler_dispatch.cpp/scheduler_completion.cpp(publish sites aroundflush_publish)Priority
Low (no impact today, good to fix eventually)