Refactor: share host_build_graph's a2a3/a5 host logic through src/common - #2060
Merged
poursoul merged 2 commits intoAug 29, 2026
Merged
Conversation
The two architectures' host_build_graph trees were near-duplicates. Every source moved here was byte-identical between them; what stays in the arch trees is what genuinely differs. - src/common/host_build_graph gains host/, device/ and shared/ subdirectories naming the targets that compile each .cpp. The split is required because every build_config source_dirs entry is collected by a recursive glob, and host-only code uses the STL containers the AICPU target forbids. - Host-only: dep_gen_host_graph, host_phase_trace, host_tensor_access and ready_queue_sizing, plus their four headers. Host and AICPU both: orchestrator, runtime_core, runtime_init, tensormap, shared_memory and runtime. - RUNTIME_MAX_WORKER resolves through PLATFORM_MAX_CORES instead of a literal 72 / 108 spelled twice per architecture, which leaves runtime.h and scheduler_context.h identical across the two. The value is still per-arch, and an absent definition is a compile error rather than a silently wrong struct layout. - Remaining non-substantive drift collapsed: three include guards to #pragma once (their names claimed TENSORMAP_AND_RINGBUFFER and two carried a PTO_ prefix), two blank lines, and comment text that had diverged between the copies. - cpput's twelve repeated five-file source lists become one HBG_ORCH_SHARED_SOURCES, now that both architectures compile the same translation units. - graph_recorder_pool.h's single std::lock_guard becomes std::scoped_lock, matching the five other lock sites in that file. Four files still differ by architecture and stay put: runtime_compile_info.cpp, runtime_maker.cpp (a5 exports simpler_aicpu_query_topology), sdma_completion_scheduler.h (a2a3 invalidates the record's cache line) and scheduler_completion.cpp (the a5 PMU record carries the AICore register token).
|
Important Review skippedToo many files! This PR contains 123 files, which is 23 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (123)
You can disable this status message by setting the 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 |
poursoul
force-pushed
the
refactor/hbg-host-to-common
branch
from
August 28, 2026 09:21
b73ba0e to
5c04231
Compare
Every header the two architectures held byte-identical copies of now lives
in src/common/host_build_graph, with no forwarding shim left behind. The
arch trees keep only what genuinely differs.
- src/common/host_build_graph sits on every hbg target's include path.
That is what lets the runtime-agnostic platform and worker sources reach
this runtime's runtime.h and types.h by bare name: they resolve to the
shared copies when compiled for host_build_graph, and to
tensormap_and_ringbuffer's own copies when compiled for that runtime.
"runtime" precedes it on the path, so an arch-local header of the same
name still wins.
- Sources under src/common name the shared headers with their
host_build_graph/ prefix instead of relying on that include path,
because cpput compiles them with its own include configuration. Only the
runtime-agnostic files keep bare names, which is what keeps them usable
from either runtime.
- cpput's hbg targets gain the same include directory, for the same
reason.
Moved: runtime.h, types.h, runtime_types.h, tensormap.h, shared_memory.h,
runtime_core.h, submit_types.h, task_allocator.h, dep_compute.h,
orchestrator.h, tensor_create_info.h, completion_token.h, common.h,
constants.h, runtime_status.h and graph_recorder_pool.{h,cpp}.
The scheduler, AICPU and AICore headers stay per-arch, as do
orchestration_api.h and arg_with_deps.h.
ChaoWao
approved these changes
Aug 28, 2026
This was referenced Sep 8, 2026
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.
What
host_build_graph's a2a3 and a5 trees were near-duplicates. This moves thehost-side duplication into
src/common/host_build_graphand leaves the archtrees holding the rest. Net -10,700 lines across the two commits, no
behavioral change.
a2ae12a3— host and dual-target sources:dep_gen_host_graph,host_phase_trace,host_tensor_access,ready_queue_sizing(host-only),plus
orchestrator,runtime_core,runtime_init,tensormap,shared_memory,runtime(host + aicpu). Also collapses non-substantivedrift between the copies: three include guards to
#pragma once, two blanklines, and comment text that had diverged.
b73ba0ea— 16 shared headers plusgraph_recorder_pool.cpp, with noforwarding shims.
Scope: host logic only
Deliberately not merged: the scheduler's own implementation, the AICPU and
AICore sources, and
orchestration_api.h/arg_with_deps.h. Several of thoseare byte-identical between the arches today and still stay per-arch — that is
the intended boundary for this change, not an oversight. What did move is host
logic plus the basics the scheduler and host both depend on (types, task/SM
layout, the Runtime core).
How the include path works
src/common/host_build_graphis on every hbg target's include path. That is whatlets the runtime-agnostic
platform/andcommon/worker/sources reach thisruntime's
runtime.handtypes.hby bare name — they resolve to the sharedcopies when compiled for
host_build_graph, and totensormap_and_ringbuffer'sown copies when compiled for that runtime.
"runtime"precedes the shareddirectory on the path, so an arch-local header of the same name still wins.
Sources under
src/commonname the shared headers with theirhost_build_graph/prefix rather than relying on that include path, becausecpput compiles them with its own include configuration. Only the
runtime-agnostic files keep bare names, which is what keeps them usable from
either runtime.
.cppfiles split intohost/,device/,shared/subdirectories naming thetargets that compile them, because every
source_dirsentry is collected by arecursive glob and host-only code uses the STL containers the AICPU target
forbids.
RUNTIME_MAX_WORKER
The one semantic change here. It now resolves through
PLATFORM_MAX_CORESinstead of a literal 72 / 108 spelled twice per architecture, which leaves
runtime.handscheduler_context.hidentical across the two. The value isstill per-arch — it comes from the platform's own core count, and
tensormap_and_ringbufferhas spelled it this way all along — and an absentdefinition is a compile error rather than a silently wrong struct layout.
Files that still differ by architecture (10)
scheduler/scheduler_types.hruntime/dispatch_payload.hcommon/intrinsic.hruntime/async_wait.haicore/aicore_executor.cppscheduler/scheduler_dispatch.cpphost/runtime_maker.cppsimpler_aicpu_query_topologyscheduler/scheduler_completion.cppbackend/sdma/sdma_completion_scheduler.hhost/runtime_compile_info.cppThe first six are #2056's a5-side changes, which landed while this branch was in
flight. Three of them (
intrinsic.h,async_wait.h,dispatch_payload.h) aresmall enough that they could be shared once that divergence is reconciled;
sharing them here would have meant reverting part of #2056, so they stay put.
Verified locally
tensormap_and_ringbuffercold rebuild + 25 scene tests pass — the includesplit does not touch it
examples+tests/st): a2a3sim and a5sim both exit 0pip installcannot complete on an a2a3 box right now because a5 onboard'saicore target fails to build there (
use of undeclared identifier 'aicore'inscheduler_types.h, from #2056). That reproduces on cleanmainand CI's a5jobs are green, so it is a local toolchain gap rather than a regression — but it
does block local scene tests until worked around with
ASCEND_HOME_PATH=/nonexistent pip install --no-build-isolation -e ..🤖 Generated with Claude Code