Skip to content

Fix: scope A5 CPU topology fallback to x86 standard cards - #1552

Open
sunkaixuan2018 wants to merge 1 commit into
hw-native-sys:mainfrom
sunkaixuan2018:skx/a5-cpu-topo-fallback
Open

Fix: scope A5 CPU topology fallback to x86 standard cards#1552
sunkaixuan2018 wants to merge 1 commit into
hw-native-sys:mainfrom
sunkaixuan2018:skx/a5-cpu-topo-fallback

Conversation

@sunkaixuan2018

@sunkaixuan2018 sunkaixuan2018 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • use the AICPU OCCUPY fallback only when CPU_TOPO is unavailable on the verified x86 Ascend950PR_9579 standard-card signature (OCCUPY=0x3e)
  • preserve the existing failure behavior for unverified hosts, SoCs, and masks while keeping the normal CPU_TOPO path unchanged
  • cover accepted and rejected signatures in the focused C++ regression test and document the compatibility boundary

Testing

  • git diff --check upstream/main...HEAD
  • previous A5 hardware validation on the same Ascend950PR_9579 / OCCUPY=0x3e environment: test_a5_aicpu_topology_fallback passed and two-device onephase AllReduce passed in 15.11s
  • Linux/macOS PR CI (running after this update)

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0db14cee-917b-4a53-a790-966e0aa37d41

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

A5 host topology probing now falls back to deriving non-SMT CPU metadata from the OCCUPY bitmap when CPU_TOPO is unsupported. Documentation and no-hardware unit tests cover topology derivation, affinity ordering, and empty masks.

Changes

A5 CPU topology fallback

Layer / File(s) Summary
Topology fallback implementation
src/a5/platform/onboard/host/aicpu_topology_probe.*, src/a5/docs/hardware.md
Topology probing derives CPU, cluster, and die metadata from set OCCUPY bits when CPU_TOPO is unavailable, with updated API documentation and compatibility details.
Fallback validation and test wiring
tests/ut/cpp/a5/test_aicpu_topology_fallback.cpp, tests/ut/cpp/CMakeLists.txt
No-hardware tests validate derived metadata, allowed-CPU ordering, and empty-mask rejection; CMake registers the test target.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

I’m a rabbit hopping through bits in a row,
Mapping each CPU where the OCCUPY lights glow.
CPU_TOPO may hide, but the fallback is bright,
Clusters and dies now line up just right.
Tests thump their paws: the pool’s in sight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title matches the main change: scoping A5 CPU topology fallback to verified standard cards.
Description check ✅ Passed The description clearly relates to the fallback behavior, test coverage, and documentation changes.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/a5/platform/onboard/host/aicpu_topology_probe.cpp`:
- Around line 179-192: Restrict the OCCUPY fallback in
derive_topology_from_occupy to verified CPU_TOPO-capable drivers, or validate
each set host mask bit against the device-side user pool and SMT topology before
assigning phy_cpu_id and hyperthread_id. Update the corresponding
contract/documentation in src/a5/platform/onboard/host/aicpu_topology_probe.h
(lines 37-50) and src/a5/docs/hardware.md (lines 165-169) to state this
restriction; apply the implementation change at
src/a5/platform/onboard/host/aicpu_topology_probe.cpp (lines 179-192).
🪄 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: 818eba7d-e086-4c4a-8af3-fdf7e676487e

📥 Commits

Reviewing files that changed from the base of the PR and between 96d0291 and 2dfba39.

📒 Files selected for processing (5)
  • src/a5/docs/hardware.md
  • src/a5/platform/onboard/host/aicpu_topology_probe.cpp
  • src/a5/platform/onboard/host/aicpu_topology_probe.h
  • tests/ut/cpp/CMakeLists.txt
  • tests/ut/cpp/a5/test_aicpu_topology_fallback.cpp

Comment thread src/a5/platform/onboard/host/aicpu_topology_probe.cpp Outdated
@sunkaixuan2018
sunkaixuan2018 force-pushed the skx/a5-cpu-topo-fallback branch from 2dfba39 to a7c98f0 Compare July 29, 2026 01:59
@sunkaixuan2018 sunkaixuan2018 changed the title Fix: handle missing A5 CPU topology Fix: scope A5 CPU topology fallback to x86 standard cards Jul 29, 2026
Use OCCUPY-derived topology only for the verified x86 Ascend950PR_9579 signature when CPU_TOPO is unavailable. Preserve the existing failure behavior for other hosts, SoCs, and masks, and cover both accepted and rejected signatures in the C++ regression test.
@sunkaixuan2018
sunkaixuan2018 force-pushed the skx/a5-cpu-topo-fallback branch from a7c98f0 to ec408a3 Compare July 29, 2026 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant