Fix host-build-graph validation and teardown defects - #1674
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 changes add atomic ownership for executor cleanup, null-task checks in wait and drain paths, safer MIX-cluster bit construction, and zero initialization for ChangesRuntime safety and scheduler state
Estimated code review effort: 2 (Simple) | ~10 minutes 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
`@src/a2a3/runtime/host_build_graph/runtime/scheduler/scheduler_completion.cpp`:
- Around line 661-662: The null pending-task checks in scheduler_completion.cpp
at lines 661-662 and 666-667 must abort the open drain transaction before
returning. Route each null scheduled_task_slot_state path through the
dispatcher-owned abort flow that clears the drained slot_state and restarts the
state machine, preventing follower spins and allowing future drains to reopen.
🪄 Autofix (Beta)
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: 14035f49-6935-4d60-a7a1-a6a51babaee7
📒 Files selected for processing (10)
src/a2a3/runtime/host_build_graph/aicpu/aicpu_executor.cppsrc/a2a3/runtime/host_build_graph/runtime/orchestrator_core/pto_runtime2.cppsrc/a2a3/runtime/host_build_graph/runtime/scheduler/scheduler_completion.cppsrc/a2a3/runtime/host_build_graph/runtime/scheduler/scheduler_dispatch.cppsrc/a2a3/runtime/host_build_graph/runtime/scheduler/scheduler_types.hsrc/a5/runtime/host_build_graph/aicpu/aicpu_executor.cppsrc/a5/runtime/host_build_graph/runtime/orchestrator_core/pto_runtime2.cppsrc/a5/runtime/host_build_graph/runtime/scheduler/scheduler_completion.cppsrc/a5/runtime/host_build_graph/runtime/scheduler/scheduler_dispatch.cppsrc/a5/runtime/host_build_graph/runtime/scheduler/scheduler_types.h
ChaoWao
force-pushed
the
fix-hbg-coderabbit
branch
7 times, most recently
from
August 4, 2026 07:53
91d01af to
0db7c02
Compare
ChaoWao
changed the title
ChaoWao
force-pushed
the
fix-hbg-coderabbit
branch
2 times, most recently
from
August 4, 2026 08:46
a310c2b to
3c9bc61
Compare
- Preserve 128-bit MIX placement across all clusters and initialize tracker state deterministically - Publish cleanup eligibility only after the last thread finishes runtime destruction, with an exclusive cleanup claim - Reject invalid block/subtask totals and forged tensor producer IDs with INVALID_ARGS instead of asserting or dereferencing an unbound slot - Correct scalar-access, dispatch-payload, profiling, and runtime lifecycle documentation for host graph construction - Add architecture-parity unit and simulation coverage for validation, teardown ordering, and wide dispatch
ChaoWao
force-pushed
the
fix-hbg-coderabbit
branch
from
August 4, 2026 09:31
3c9bc61 to
cb7046e
Compare
ChaoWao
added a commit
to YunjiQin/simpler
that referenced
this pull request
Aug 4, 2026
…h sources The host_build_graph_validation and host_build_graph_wide_dispatch scene-test orchestration sources use the pre-rename type names L0TaskArgs, L2TaskArgs and the GM-address-bearing Tensor. These sources landed via hw-native-sys#1674, outside this rename branch's base, so the L0->Core, L2TaskArgs->ChipTaskArgs and Tensor->ChipTensor renames never reached them. Update them to CoreTaskArgs/CoreTaskArgsWithDeps, ChipTaskArgs and ChipTensor so the renamed tree compiles, matching the spelling already used in sibling orchestration sources. Co-Authored-By: Claude <noreply@anthropic.com>
ChaoWao
added a commit
to YunjiQin/simpler
that referenced
this pull request
Aug 4, 2026
…t sources The host_build_graph_validation and host_build_graph_wide_dispatch scene-test sources landed via hw-native-sys#1674, outside this rename branch's base, so the L0->Core, L2TaskArgs->ChipTaskArgs and Tensor->ChipTensor renames never reached them: - orchestration .cpp: L0TaskArgs/L0TaskArgsWithDeps -> CoreTaskArgs/ CoreTaskArgsWithDeps, L2TaskArgs -> ChipTaskArgs, GM-address Tensor -> ChipTensor. - host_build_graph_validation Python test: the device-descriptor import simpler.task_interface.Tensor -> ChipTensor (the Tensor.make arg builder), which otherwise aborts pytest collection with ImportError. host_build_graph_wide_dispatch uses simpler_setup.Tensor (the scene-test arg spec), which is a separate type and is not renamed.
ChaoWao
added a commit
to YunjiQin/simpler
that referenced
this pull request
Aug 4, 2026
…r names Files that landed on main via hw-native-sys#1674 and hw-native-sys#1684 sit outside this rename branch's base and still use the pre-rename names. Bring them onto the renamed types and doc paths so the tree compiles, imports, and links cleanly: - host_build_graph_validation/wide_dispatch orchestration .cpp (hw-native-sys#1674): L0TaskArgs/L0TaskArgsWithDeps -> CoreTaskArgs/CoreTaskArgsWithDeps, L2TaskArgs -> ChipTaskArgs, GM-address Tensor -> ChipTensor. - host_build_graph_validation Python test (hw-native-sys#1674): the device-descriptor import simpler.task_interface.Tensor -> ChipTensor (the .make arg builder), which otherwise aborted pytest collection with ImportError. - SUBMIT_BY_CLUSTER.md example (hw-native-sys#1674) and build_config.py comment (hw-native-sys#1674): L0TaskArgs -> CoreTaskArgs, Tensor -> ChipTensor. - troubleshooting/device-error-codes/README.md index (hw-native-sys#1684): inbound link l0-swimlane-profiling.md -> core-swimlane-profiling.md, the swimlane doc this rename renamed. host_build_graph_wide_dispatch uses simpler_setup.Tensor (the scene-test arg spec), a separate type that is not renamed.
ChaoWao
added a commit
that referenced
this pull request
Aug 4, 2026
* Rename: L3L2 identifiers to WorkerChip
The worker-chip message queue, orchestration comm, endpoint and region types
carried the numeric topology level in their names. Codestyle rule 13 reserves
numeric levels for architecture prose and names software entities by role, so
the L3+ recursive scheduling context is Worker and the chip runtime context is
Chip.
- L3L2Queue* -> WorkerChipQueue*, L3L2Orch* -> WorkerChipOrch*,
L3L2Endpoint* -> WorkerChipEndpoint*, L3L2Region* -> WorkerChipRegion*
- L3L2_* and CTRL_L3_L2_* constants -> WORKER_CHIP_* / CTRL_WORKER_CHIP_*;
their values are unchanged, so the wire layout is untouched
- l3_l2_* functions, Python modules, headers and example directories ->
worker_chip_*
- _L2HostL3L2Region* -> _HostWorkerChipRegion*, collapsing the doubled context
- the four renamed headers use #pragma once, their guard names having
disagreed with the new paths
Device-log text and architecture prose keep the L3-L2 spelling, which the rule
sanctions for topology description. No compatibility aliases are kept.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Rename: L3Host identifiers to WorkerHost
The host-side registry and lease types for a worker-owned mapped region named
the numeric topology level rather than the owning software context. Codestyle
rule 13 puts L3+ recursive scheduling in the Worker context.
- L3HostMappedRegion* -> WorkerHostMappedRegion* (Handle, Entry, Registry,
Lease, CleanupErrors)
- L3HostRegionMapping* -> WorkerHostRegionMapping*
- l3_host_* helpers and attributes -> worker_host_*
No compatibility aliases are kept.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Rename: L2TaskArgs to ChipTaskArgs and L2 queue ends to Chip/Worker
The orchestration entry argument holder is the chip-level container -- its own
comment already said so -- and the message queue named its two ends by numeric
level. Codestyle rule 13 names the chip runtime context Chip and the L3+
recursive scheduling context Worker.
- L2TaskArgs -> ChipTaskArgs
- L2ChildOnboardRegion* -> ChipChildOnboardRegion*, g_l2_child_onboard_regions
-> g_chip_child_onboard_regions
- WORKER_CHIP_QUEUE_L3_ABORT_FLAG_OFFSET -> ..._WORKER_ABORT_FLAG_OFFSET and
WORKER_CHIP_QUEUE_L2_ABORT_FLAG_OFFSET -> ..._CHIP_ABORT_FLAG_OFFSET, with
the l3_/l2_abort_flag_offset layout fields following; offsets are unchanged
- WorkerChipQueue.l2_task_arg_scalars() -> chip_task_arg_scalars()
No compatibility aliases are kept.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Rename: L2Swimlane to ChipSwimlane and L0 swimlane to core swimlane
The swimlane collectors, their records, the DFX flag and the pytest option all
named the numeric topology level. Codestyle rule 13 names the chip runtime
context Chip and the AIC/AIV task-construction context Core.
- L2Swimlane* -> ChipSwimlane*, L2SWIMLANE_* -> CHIP_SWIMLANE_*,
SIMPLER_DFX_FLAG_L2_SWIMLANE -> SIMPLER_DFX_FLAG_CHIP_SWIMLANE
- l2_swimlane_* functions, fields, records and JSON artifact names ->
chip_swimlane_*
- pytest --enable-l2-swimlane -> --enable-chip-swimlane and
--l2-swimlane-records-json -> --chip-swimlane-records-json, with the CI
workflows and scene-test plumbing following
- simpler_setup.tools.l0_swimlane -> core_swimlane, and the intra-core
swimlane is called the core swimlane throughout docs and skills
- tests/st/{a2a3,a5}/tensormap_and_ringbuffer/dfx/l2_swimlane ->
dfx/chip_swimlane
- the four swimlane headers use #pragma once
Prose keeps numeric levels where it describes topology; the feature itself is
named after its owning context. No compatibility aliases are kept, so an
external caller passing --enable-l2-swimlane must move to the new spelling.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Rename: L0 task-construction types to Core
The per-core task argument and predicate types named the numeric topology level.
Codestyle rule 13 names the AIC/AIV task-construction context Core and lists
CoreTaskArgs and CoreTaskPredicate as the intended spellings.
- L0TaskArgs -> CoreTaskArgs, L0TaskArgsWithDeps -> CoreTaskArgsWithDeps
- L0TaskPredicate -> CoreTaskPredicate, L0PredicateOperand ->
CorePredicateOperand
L0A / L0B / L0C, L0TileShape and the other AICore on-chip memory names are
untouched: they name a hardware storage hierarchy, not a topology level.
No compatibility aliases are kept, so orchestration sources outside this repo
must move to the new spelling.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Rename: the GM-address-bearing Tensor to ChipTensor
Two public types were called Tensor: the C++ descriptor in
src/common/task_interface/tensor.h, which embeds a device address and is
materialized by a ChipWorker, and simpler_setup.Tensor, the address-free
scene-test arg spec. Codestyle rule 13 names the first after its owning
context and reserves the unprefixed name for the address-free L3+ form, so the
two no longer collide.
- C++ Tensor -> ChipTensor across the runtimes, the orchestration API, the
AICore kernels that read __gm__ ChipTensor* task args, and the tests
- the nanobind class binds as ChipTensor, so simpler.task_interface.ChipTensor
is the Python spelling; simpler_setup.Tensor is unchanged and its 100 users
are untouched
- docs describing the L2 ABI follow
The struct layout, its 128-byte size assertion and the TensorMap offset
assertions are unchanged, so the wire format is identical.
GlobalTensor, LocalTensor, TensorMap, TensorKey, TensorArgType, RemoteTensorRef
and torch.Tensor are all distinct names and are untouched.
No compatibility alias is kept, so kernel sources outside this repo must move
to the new spelling.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Rename: catch up concurrent main additions to the Core/Chip/ChipTensor names
Files that landed on main via #1674 and #1684 sit outside this rename branch's
base and still use the pre-rename names. Bring them onto the renamed types and
doc paths so the tree compiles, imports, and links cleanly:
- host_build_graph_validation/wide_dispatch orchestration .cpp (#1674):
L0TaskArgs/L0TaskArgsWithDeps -> CoreTaskArgs/CoreTaskArgsWithDeps,
L2TaskArgs -> ChipTaskArgs, GM-address Tensor -> ChipTensor.
- host_build_graph_validation Python test (#1674): the device-descriptor
import simpler.task_interface.Tensor -> ChipTensor (the .make arg builder),
which otherwise aborted pytest collection with ImportError.
- SUBMIT_BY_CLUSTER.md example (#1674) and build_config.py comment (#1674):
L0TaskArgs -> CoreTaskArgs, Tensor -> ChipTensor.
- troubleshooting/device-error-codes/README.md index (#1684): inbound link
l0-swimlane-profiling.md -> core-swimlane-profiling.md, the swimlane doc
this rename renamed.
host_build_graph_wide_dispatch uses simpler_setup.Tensor (the scene-test arg
spec), a separate type that is not renamed.
Co-authored-by: wcwxy <26245345+ChaoWao@users.noreply.github.com>
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
scheduler tracker state deterministically.
per-run runtime, then grant one thread the exclusive
deinit()claim.with
INVALID_ARGSinstead of asserting or dereferencing an unbound slot.lifecycle documentation in both architecture trees.
ordering, invalid inputs, and dispatch above bit 63.
The prior
pending_task == nullptrdrain guard was removed. Its CodeRabbitfinding was withdrawn after confirming that incomplete drain state is
intentionally reset by
deinit()and the guard only made an unreachable statelook recoverable.
Testing
test_thread_completion_gate,test_hbg_core_tracker, andtest_a5_hbg_core_trackerpassed.after formatting.
cpplint, markdownlint, Ruff, and Pyright.
Hardware tests were not run locally because this host has no
npu-smidevice.