ci(a2a3): run pytest/ctest directly on X64 runners, task-submit on ARM64#1361
Merged
Conversation
The a2a3 self-hosted pool now spans two runner kinds: the existing ARM64
hosts (shared with interactive users) and new X64 hosts whose cards are
exclusive to the runner and which have no `task-submit` installed. A single
`runs-on: [self-hosted, a2a3]` job can land on either, so the per-device
lock must be conditional.
Branch every a2a3 NPU-touching step on `runner.arch`: X64 runs pytest/ctest
directly with `--device ${DEVICE_RANGE}`; ARM64 keeps the
`task-submit --device ... --run` wrapper. a5 is ARM64-only and unchanged.
docs/ci.md's "every step runs through task-submit" claim is updated to match.
Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthrougha2a3 hardware tests now run directly on x86_64 runners and through Changesa2a3 execution routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the CI documentation in docs/ci.md to clarify how self-hosted runners handle device locking, distinguishing between ARM64 a2a3 runners (which use task-submit) and X64 a2a3 runners (which run tests directly). The review feedback suggests improving the phrasing and formatting of the description for X64 a2a3 runners to be more professional and consistent.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
`runner.arch` is not available in the `env` context (only github, inputs,
matrix, needs, secrets, strategy, vars), so `RUNNER_ARCH: ${{ runner.arch }}`
failed GitHub's workflow compile with a "workflow file issue". Detect the
host arch in shell with `uname -m` instead — context-independent and it
matches the arch the host .so is actually compiled for (x86_64 vs aarch64).
Co-Authored-By: Claude <noreply@anthropic.com>
The a2a3 step branches on `uname -m`, not the `runner.arch` context (which isn't available where the original wording implied). Wording only. Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
488-493: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the
.claude/rules/running-onboard.mdreference.As per the relevant code snippets provided,
.claude/rules/running-onboard.mdstates that "CI always wraps pytest intask-submit". Since this PR introduces an exception forx86_64runners, consider updating that document to reflect the new architecture-aware execution model so that AI assistants maintain an accurate understanding of the CI contracts.🤖 Prompt for 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. In @.github/workflows/ci.yml around lines 488 - 493, Update the CI execution guidance in running-onboard.md to document that pytest runs directly on x86_64 runners and uses task-submit on other architectures. Replace the statement that CI always wraps pytest in task-submit while preserving the existing command and architecture-specific behavior.
🤖 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.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 488-493: Update the CI execution guidance in running-onboard.md to
document that pytest runs directly on x86_64 runners and uses task-submit on
other architectures. Replace the statement that CI always wraps pytest in
task-submit while preserving the existing command and architecture-specific
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3eeb5441-4d7c-4a6a-83a9-6be8278f4590
📒 Files selected for processing (2)
.github/workflows/ci.ymldocs/ci.md
doraemonmj
pushed a commit
to doraemonmj/simpler_wc
that referenced
this pull request
Jul 15, 2026
…M64 (hw-native-sys#1361) The a2a3 self-hosted pool now spans two runner kinds: the existing ARM64 hosts (shared with interactive users) and new X64 hosts whose cards are exclusive to the runner and which have no `task-submit` installed. A single `runs-on: [self-hosted, a2a3]` job can land on either, so the per-device lock must be conditional. Branch every a2a3 NPU-touching step on `runner.arch`: X64 runs pytest/ctest directly with `--device ${DEVICE_RANGE}`; ARM64 keeps the `task-submit --device ... --run` wrapper. a5 is ARM64-only and unchanged. docs/ci.md's "every step runs through task-submit" claim is updated to match.
ChaoZheng109
added a commit
to ChaoZheng109/simpler
that referenced
this pull request
Jul 15, 2026
host_build_graph runs its orchestration .so on the host CPU, so compile_orchestration() builds it with the bare host g++. The orch compile unconditionally links the onboard AICPU helpers device_time.cpp and cache_ops.cpp, whose bodies are aarch64-only inline asm (mrs cntvct_el0, dc civac/cvac, dsb, isb). On an ARM64 runner the host g++ targets aarch64, so the asm assembled and the latent host-arch assumption stayed hidden. hw-native-sys#1361 made st-onboard-a2a3 eligible for x86_64 runners; there the host g++ feeds aarch64 asm to the x86 assembler and every host_build_graph L2 test fails with "Orchestration compilation failed ... no such instruction: mrs %rax,cntvct_el0". Gate the asm behind __aarch64__ and add a host-portable else branch: - device_time: derive the sys-cnt tick from std::chrono::steady_clock, scaled to the same PLATFORM_PROF_SYS_CNT_FREQ unit the DFX markers decode against. steady_clock (matching device_time_now_ticks() in device_time.h) is monotonic; the ticks feed unsigned deltas in the orchestrator poll loops (get_sys_cnt_aicpu() - t0), where a backward clock jump would underflow into a false timeout. - cache_ops: no-op. The host orchestrator reaches device memory only through driver H2D DMA (cache-coherent on x86), so the AICPU-side manual cache maintenance has no host-side referent. aarch64 (device AICPU and ARM64-host orchestration) is unchanged; only the x86 host build gains a compilable path.
ChaoZheng109
added a commit
to ChaoZheng109/simpler
that referenced
this pull request
Jul 15, 2026
host_build_graph runs its orchestration .so on the host CPU, so compile_orchestration() builds it with the bare host g++. The orch compile unconditionally links the onboard AICPU helpers device_time.cpp and cache_ops.cpp, whose bodies are aarch64-only inline asm (mrs cntvct_el0, dc civac/cvac, dsb, isb). On an ARM64 runner the host g++ targets aarch64, so the asm assembled and the latent host-arch assumption stayed hidden. hw-native-sys#1361 made st-onboard-a2a3 eligible for x86_64 runners; there the host g++ feeds aarch64 asm to the x86 assembler and every host_build_graph L2 test fails with "Orchestration compilation failed ... no such instruction: mrs %rax,cntvct_el0". Gate the asm behind __aarch64__ and add a host-portable else branch: - device_time: get_sys_cnt_aicpu() now delegates the clock read to device_time_now_ticks() (monotonic steady_clock off aarch64), then rescales ns to the PLATFORM_PROF_SYS_CNT_FREQ unit it reports in, so the orchestrator's cycle-based timeouts (get_sys_cnt_aicpu() - t0 > ..._CYCLES) and DFX decode stay valid. The prior sim variant used the non-monotonic high_resolution_clock; unify both on device_time_now_ticks(). - cache_ops: no-op off aarch64. The host orchestrator reaches device memory only through driver H2D DMA (cache-coherent on x86), so the AICPU-side manual cache maintenance has no host-side referent. aarch64 (device AICPU and ARM64-host orchestration) is behaviorally unchanged; only the x86 host build gains a compilable path.
ChaoZheng109
added a commit
that referenced
this pull request
Jul 15, 2026
host_build_graph runs its orchestration .so on the host CPU, so compile_orchestration() builds it with the bare host g++. The orch compile unconditionally links the onboard AICPU helpers device_time.cpp and cache_ops.cpp, whose bodies are aarch64-only inline asm (mrs cntvct_el0, dc civac/cvac, dsb, isb). On an ARM64 runner the host g++ targets aarch64, so the asm assembled and the latent host-arch assumption stayed hidden. #1361 made st-onboard-a2a3 eligible for x86_64 runners; there the host g++ feeds aarch64 asm to the x86 assembler and every host_build_graph L2 test fails with "Orchestration compilation failed ... no such instruction: mrs %rax,cntvct_el0". Gate the asm behind __aarch64__ and add a host-portable else branch: - device_time: get_sys_cnt_aicpu() now delegates the clock read to device_time_now_ticks() (monotonic steady_clock off aarch64), then rescales ns to the PLATFORM_PROF_SYS_CNT_FREQ unit it reports in, so the orchestrator's cycle-based timeouts (get_sys_cnt_aicpu() - t0 > ..._CYCLES) and DFX decode stay valid. The prior sim variant used the non-monotonic high_resolution_clock; unify both on device_time_now_ticks(). - cache_ops: no-op off aarch64. The host orchestrator reaches device memory only through driver H2D DMA (cache-coherent on x86), so the AICPU-side manual cache maintenance has no host-side referent. aarch64 (device AICPU and ARM64-host orchestration) is behaviorally unchanged; only the x86 host build gains a compilable path.
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.
Why
The
a2a3self-hosted pool now spans two runner kinds:task-submit?runner-dev4-7,runner-dev8-11infra-gpu-npu-021,infra-gpu-npu-021-2A single
runs-on: [self-hosted, a2a3]job can land on either pool, so the per-device lock can no longer be unconditional —task-submitis not installed on the X64 runners and their cards are already exclusive to the runner.What
RUNNER_ARCH: ${{ runner.arch }}tout-a2a3andst-onboard-a2a3.runner.arch:pytest/ctestdirectly with--device ${DEVICE_RANGE}task-submit --device <list> --run "..."wrapper (shared host)docs/ci.md"Hardware Classification" paragraph updated: the old "every step runs through task-submit" claim no longer holds for X64.No
runs-onchange — both pools stay eligible, which is the point (X64 adds capacity, ARM64 keeps working). Build-only steps (cmake,RuntimeBuilder, thecann-examplessmokes) already take no lock and are unchanged.🤖 Generated with Claude Code