Fix: harden HBG Graph Execution across A2/A3 and A5 - #1767
Merged
ChaoZheng109 merged 1 commit intoAug 11, 2026
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change hardens A2A3 and A5 host-build-graph execution. It validates graph submissions and replay state, centralizes scheduler failure handling, separates resolution from host completion, adds graph-slot rollback, documents error 104, and adds parallel regression tests. ChangesHost-build-graph hardening
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GraphSubmit
participant PTO2SchedulerState
participant SchedulerContext
participant Runtime
GraphSubmit->>PTO2SchedulerState: enqueue graph preparation
PTO2SchedulerState->>PTO2SchedulerState: detect queue or completion error
PTO2SchedulerState->>SchedulerContext: call fail_scheduler
SchedulerContext->>Runtime: trigger one-time emergency shutdown
Possibly related issues
Possibly related PRs
Poem
🚥 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@docs/troubleshooting/device-error-codes.md`:
- Around line 32-35: Update the runtime execution description in the section
containing host_build_graph and tensormap_and_ringbuffer: state that
host_build_graph performs orchestration on the host and transfers the prepared
image to the AICPU scheduler, while preserving the existing description of
tensormap_and_ringbuffer.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 995ae446-d35c-4e0e-9f2e-d4a5ba4c5bf2
📒 Files selected for processing (24)
docs/troubleshooting/device-error-codes.mdsrc/a2a3/runtime/host_build_graph/host/runtime_maker.cppsrc/a2a3/runtime/host_build_graph/runtime/graph_execution.hsrc/a2a3/runtime/host_build_graph/runtime/orchestrator_core/pto_orchestrator.cppsrc/a2a3/runtime/host_build_graph/runtime/pto_async_wait.hsrc/a2a3/runtime/host_build_graph/runtime/scheduler/graph_execution.cppsrc/a2a3/runtime/host_build_graph/runtime/scheduler/pto_scheduler.hsrc/a2a3/runtime/host_build_graph/runtime/scheduler/scheduler_cold_path.cppsrc/a2a3/runtime/host_build_graph/runtime/scheduler/scheduler_context.hsrc/a2a3/runtime/host_build_graph/runtime/scheduler/scheduler_dispatch.cppsrc/a5/runtime/host_build_graph/host/runtime_maker.cppsrc/a5/runtime/host_build_graph/runtime/graph_execution.hsrc/a5/runtime/host_build_graph/runtime/orchestrator_core/pto_orchestrator.cppsrc/a5/runtime/host_build_graph/runtime/pto_async_wait.hsrc/a5/runtime/host_build_graph/runtime/scheduler/graph_execution.cppsrc/a5/runtime/host_build_graph/runtime/scheduler/pto_scheduler.hsrc/a5/runtime/host_build_graph/runtime/scheduler/scheduler_cold_path.cppsrc/a5/runtime/host_build_graph/runtime/scheduler/scheduler_context.hsrc/a5/runtime/host_build_graph/runtime/scheduler/scheduler_dispatch.cppsrc/common/runtime_status/error_names.htests/ut/cpp/CMakeLists.txttests/ut/cpp/a2a3/test_graph_cache.cpptests/ut/cpp/a5/test_graph_cache.cpptests/ut/cpp/common/test_hbg_graph_submit_rollback.cpp
zmnobug
force-pushed
the
fix/issue-1736-harden-hbg-graph-execution
branch
from
August 11, 2026 02:44
a69a75b to
a8fd522
Compare
Collaborator
|
Question on I traced the fanin-failure path and I think the residual outer slot can never reach execution once The failure is host-side, during orchestration:
So as far as I can see, after a fanin failure the half-built GRAPH slot is never seen by the scheduler and never uploaded — it just sits in a fatal orchestrator until teardown. The DUMMY conversion doesn't appear to guard a path that fires. If that holds, I'd lean toward dropping
The invariant genuinely worth pinning is "fanin failure ⇒ fatal + no partial upload," which the test already checks via If I've missed a path that does reach the residual slot — e.g. the AICPU scheduler scanning the ring independently of |
- Validate Graph wire images and affine replay patch bounds - Separate internal node resolution from host task completion - Surface queue overflow and invalid completion via scheduler shutdown - Pin fatal fanin and no-partial-upload invariants on A2/A3 and A5
zmnobug
force-pushed
the
fix/issue-1736-harden-hbg-graph-execution
branch
from
August 11, 2026 09:06
a8fd522 to
b2a205d
Compare
Contributor
Author
|
@ChaoZheng109 Thanks for tracing this path. You are right: append_fanin_or_fail latches fatal before returning false, fatal prevents scope submission, the failed Graph submission never enters pending_uploads, and the device scheduler observes the orchestration error before task classification. The DUMMY conversion therefore did not protect a reachable execution path and could mask a future non-fatal early return. Updated in b2a205d:
Validation on the rebased result: 4 relevant C++ tests passed; A2/A3 HBG simulation passed with 28 passed / 4 skipped; A5 HBG simulation passed with 18 passed. |
ChaoZheng109
approved these changes
Aug 11, 2026
This was referenced Aug 11, 2026
SergioMartin86
added a commit
to huawei-csl/simpler
that referenced
this pull request
Aug 13, 2026
The on-device Graph scheduler expanded a GRAPH task into all its nodes before dispatching any of them: graph_execution_materialize_slice reached PREPARED only after the last node, and activate_prepared_graph routed the roots only at that point. For a large graph the AICores idle through the whole expansion — on the qwen3-14B 3-layer decode that front-loaded ramp is ~530 us per layer during which every core waits. Route roots (and any node whose producers have all completed) as soon as they materialize, once the outer Graph task's external-dependency gate has opened, so compute overlaps the remaining expansion. This is possible only because of the polling-completion scheduler: the per-slot completion_flags byte + task_state completion mirror introduced for tensormap_and_ringbuffer in hw-native-sys#1137 and adapted to host_build_graph in hw-native-sys#1435. That flag lets a consumer discover a producer that completed mid-expansion by reading its state instead of relying solely on the producer's wake list. Under the older pure wake-list model, dispatching before full materialization would have lost wakes if a producer finished before its consumer registered — which is exactly why the whole-graph PREPARED gate existed. - Materialization-time registration moves from register_initial_graph_waiter (a raw wake-list store that assumed no producer could complete during expansion) to register_graph_wake, which resolves a producer that completed mid-expansion through its task_state completion flag (the hw-native-sys#1137 / hw-native-sys#1435 mechanism) instead of losing the wake. The graph's topological node order guarantees every producer a node references is already constructed. - execution.nodes is published at MATERIALIZING (under materialize_busy) so the wake path can read producer slots while the graph is still expanding. - Roots reach the ready queue through route_cursor, a monotonic per-execution cursor that makes routing idempotent across the per-slice calls and the final call at the activation meet, so each root is pushed exactly once. Non-roots reach the queue only through their producers' wake list, unchanged. Because a node now dispatches before the graph reaches ACTIVE, it can complete while the graph is still MATERIALIZING or PREPARED. hw-native-sys#1767's complete_task rejects a node completion unless the graph is ACTIVE; that guard now accepts MATERIALIZING and PREPARED as well, rejecting only SUBMITTED (execution not yet localized) and COMPLETED (already retired). Without this, the largest graphs (longest materialization tail) fault with sched_error_code=5 INVALID_ARGS while the small graph_execution scene tests, which reach ACTIVE before any node completes, pass. Correctness: the three graph_execution scene tests (2D replay, AIC+AIV, MIX-SPMD) and the qwen3-14B 3-layer graph-execution golden pass, and the qwen3-14B 40-layer host-graph decode (the largest graph) passes 6/6 where it faulted 0/6 without the completion-state fix. Three host-side unit tests (tests/ut/cpp/{a2a3,a5}/test_graph_activation.cpp) cover the interleavings deterministically: a consumer registered after its producer completed and drained routes through the task_state re-scan; graph_incremental_publish routes all-producers-complete consumers and wake-chains the rest; and complete_task accepts a node completion in MATERIALIZING/PREPARED/ACTIVE and rejects it in SUBMITTED/COMPLETED. Applies the identical change to the a5 host_build_graph sibling (same graph scheduler; a5 is compile-verified, not perf-measured on this a2a3 box).
ChaoWao
pushed a commit
that referenced
this pull request
Aug 14, 2026
…ize (#1804) The on-device Graph scheduler expanded a GRAPH task into all its nodes before dispatching any of them: graph_execution_materialize_slice reached PREPARED only after the last node, and activate_prepared_graph routed the roots only at that point. For a large graph the AICores idle through the whole expansion — on the qwen3-14B 3-layer decode that front-loaded ramp is ~530 us per layer during which every core waits. Route roots (and any node whose producers have all completed) as soon as they materialize, once the outer Graph task's external-dependency gate has opened, so compute overlaps the remaining expansion. This is possible only because of the polling-completion scheduler: the per-slot completion_flags byte + task_state completion mirror introduced for tensormap_and_ringbuffer in #1137 and adapted to host_build_graph in #1435. That flag lets a consumer discover a producer that completed mid-expansion by reading its state instead of relying solely on the producer's wake list. Under the older pure wake-list model, dispatching before full materialization would have lost wakes if a producer finished before its consumer registered — which is exactly why the whole-graph PREPARED gate existed. - Materialization-time registration moves from register_initial_graph_waiter (a raw wake-list store that assumed no producer could complete during expansion) to register_graph_wake, which resolves a producer that completed mid-expansion through its task_state completion flag (the #1137 / #1435 mechanism) instead of losing the wake. The graph's topological node order guarantees every producer a node references is already constructed. - execution.nodes is published at MATERIALIZING (under materialize_busy) so the wake path can read producer slots while the graph is still expanding. - Roots reach the ready queue through route_cursor, a monotonic per-execution cursor that makes routing idempotent across the per-slice calls and the final call at the activation meet, so each root is pushed exactly once. Non-roots reach the queue only through their producers' wake list, unchanged. Because a node now dispatches before the graph reaches ACTIVE, it can complete while the graph is still MATERIALIZING or PREPARED. #1767's complete_task rejects a node completion unless the graph is ACTIVE; that guard now accepts MATERIALIZING and PREPARED as well, rejecting only SUBMITTED (execution not yet localized) and COMPLETED (already retired). Without this, the largest graphs (longest materialization tail) fault with sched_error_code=5 INVALID_ARGS while the small graph_execution scene tests, which reach ACTIVE before any node completes, pass. Correctness: the three graph_execution scene tests (2D replay, AIC+AIV, MIX-SPMD) and the qwen3-14B 3-layer graph-execution golden pass, and the qwen3-14B 40-layer host-graph decode (the largest graph) passes 6/6 where it faulted 0/6 without the completion-state fix. Three host-side unit tests (tests/ut/cpp/{a2a3,a5}/test_graph_activation.cpp) cover the interleavings deterministically: a consumer registered after its producer completed and drained routes through the task_state re-scan; graph_incremental_publish routes all-producers-complete consumers and wake-chains the rest; and complete_task accepts a node completion in MATERIALIZING/PREPARED/ACTIVE and rejects it in SUBMITTED/COMPLETED. Applies the identical change to the a5 host_build_graph sibling (same graph scheduler; a5 is compile-verified, not perf-measured on this a2a3 box).
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
2 participants
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
Testing
Fixes #1736