Skip to content

feat(rocke): expose ASIC revision through typed HIP queries - #11639

Merged
tenpercent merged 18 commits into
developfrom
users/mpodkory/rocke/aick-2069-runtime-capabilities
Sep 17, 2026
Merged

tenpercent merged 18 commits into
developfrom
users/mpodkory/rocke/aick-2069-runtime-capabilities

Conversation

@tenpercent

@tenpercent tenpercent commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

rocKE needs ASIC revision information for logic that depends on a device's
silicon revision. get_device_info(device) now returns a snapshot with read-only
asic_revision, target_id, base_arch, and compiler_target properties.
Each call makes one HIP query; reading the snapshot makes no further calls.
There is no query cache. Zero is a valid revision; unavailable values are None.

The HIP binding replaces raw-buffer scanning with a typed
hipDeviceProp_tR0600 structure bound to hipGetDevicePropertiesR0600. ASIC
revision and target ID come from named fields in the same result. Runtimes
without that versioned function return no properties.

Python and C++ share the same target identity contract: preserve the exact
runtime target ID, use the base architecture for catalog lookup and lowering,
and remove runtime profiles while preserving compiler feature suffixes for
compilation. For example, gfx1250-strict maps to compiler/base gfx1250,
while gfx942:sramecc+:xnack- keeps its compiler features and uses base gfx942.
Catalog names such as gfx11-generic remain intact. Explicit compilation
continues to use the caller's target without querying the build host's GPU.

The C++ architecture API now mirrors the three Python target helpers and fixes
rocke_arch_from_isa(), whose last-hyphen parsing previously returned strict,
an empty string, or generic for these inputs. Its caller-owned output buffers,
NUL termination, and truncation convention are preserved. Native tests cover
the target forms and buffer boundaries, and are registered with CTest, installed
test artifacts, and CI test categories. A Python test compares all four native
queries with Python across 312 inputs when ROCKE_ARCH_TARGET_TEST_EXE points
to the built native test executable.

Capability rules will follow separately. The HIP query/snapshot API remains in
the Python runtime; the C++ additions are architecture string helpers.

ISSUE ID : AICK-2222

Harness: Codex; model: gpt-6; reasoning: medium.

Test plan

  • Focused target, compile, runtime, HIP-layout, and multi-architecture tests:
    79 passed, 1 deselected (the LLVM datalayout guard excluded in prior validation).
    Includes the 312-input Python/C++ comparison.
  • All 7 host CTests, including target parsing and output-buffer boundaries.
  • LLVM-IR byte identity for all 70 families under llvm20, llvm22, and llvm23.
  • Full PR pre-commit checks, git diff --check, and the relative-path guard.
  • Current header-based ctypes layout test with the installed ROCm headers.

Previously recorded validation for the HIP-query changes:

  • Header-based layout tests with ROCm 7.1 and 10.0.0 headers, covering all
    ctypes fields and architecture bitfields without a GPU or HIP runtime library.
  • Native C++ / ctypes layout and property comparisons on gfx90a and gfx950
    with ROCm 10.0.0 / HIP 7.15.26333.
  • DeviceInfo on gfx90a: one HIP call per query, no calls from property access,
    and agreement with native HIP.
  • Windows and older HIP runtime validation.

@therock-pr-bot

therock-pr-bot Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Sep 2, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@tenpercent
tenpercent force-pushed the users/mpodkory/rocke/aick-2069-runtime-capabilities branch from 24e5aed to 3c18a84 Compare September 2, 2026 23:09
@tenpercent
tenpercent marked this pull request as ready for review September 2, 2026 23:24
@tenpercent
tenpercent requested review from a team as code owners September 2, 2026 23:24
@yraparti

yraparti commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

does it work correctly when the binary is cross compiled?

@tenpercent tenpercent changed the title feat(rocke): add gfx1250 runtime capability detection feat(rocke): preserve exact runtime device target identity Sep 8, 2026
@tenpercent
tenpercent marked this pull request as draft September 8, 2026 18:45
@tenpercent

tenpercent commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Yes, for the compilation boundary owned by this PR.

Cross-compilation uses the explicit target supplied by the caller and remains independent of the build host's GPU. rocKE separates that input into the runtime target ID, compiler target, and base architecture. For gfx1250-strict, COMGR and hipcc receive gfx1250, while rocKE lowering also uses the gfx1250 base architecture. For a compiler target such as gfx942:sramecc+:xnack-, the feature suffixes remain attached for compiler validation.

I verified those cases with the real ROCm 10 COMGR and hipcc toolchains as well as focused unit tests covering both compilation paths and the LLVM-IR hipcc helper. The destination ROCr stack remains responsible for loading the resulting code object and applying any available compatibility transformation; artifact selection and translation-aware dispatch are tracked in AICK-2069.

@tenpercent tenpercent changed the title feat(rocke): preserve exact runtime device target identity feat(rocke): model runtime and compiler target identity Sep 8, 2026
@tenpercent tenpercent changed the title feat(rocke): model runtime and compiler target identity feat(rocke): expose ASIC revision through typed HIP queries Sep 9, 2026
@tenpercent
tenpercent marked this pull request as ready for review September 9, 2026 02:03
@tenpercent
tenpercent requested a balanced review from Copilot September 9, 2026 02:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The hand-maintained HIP binary layout and unvalidated Windows and older-runtime behavior require final human review.

Pull request overview

Adds typed HIP device-property queries so rocKE can expose ASIC revisions and normalize runtime targets for lowering and compilation.

Changes:

  • Adds immutable DeviceInfo snapshots backed by hipDeviceProp_tR0600.
  • Separates runtime target IDs, base architectures, and compiler targets.
  • Updates compile paths and tests for profile and feature suffix handling.
File summaries
File Description
tests/runtime/test_device_info.py Tests querying, snapshots, failures, and exports.
tests/instances/test_rocke_multiarch.py Updates device-property parsing tests.
tests/core/test_compile_target_selection.py Tests normalized compiler and lowering targets.
tests/core/test_arch_target_identity.py Tests target identity conversions.
python/rocke/runtime/hip_module.py Uses typed R0600 HIP property queries.
python/rocke/runtime/device_info.py Introduces the DeviceInfo API.
python/rocke/runtime/_hip_device_properties.py Defines the ctypes R0600 ABI structure.
python/rocke/runtime/__init__.py Exports the device-info API.
python/rocke/helpers/compile.py Normalizes targets for COMGR and hipcc.
python/rocke/core/arch/target.py Adds target parsing and normalization helpers.
python/rocke/core/arch/__init__.py Exports target helpers.
Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@CongMa13 CongMa13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@tenpercent
tenpercent enabled auto-merge (squash) September 14, 2026 02:09
@tenpercent
tenpercent disabled auto-merge September 14, 2026 18:24
@tenpercent
tenpercent enabled auto-merge (squash) September 15, 2026 18:23
@tenpercent
tenpercent merged commit e133555 into develop Sep 17, 2026
110 of 111 checks passed
@tenpercent
tenpercent deleted the users/mpodkory/rocke/aick-2069-runtime-capabilities branch September 17, 2026 00:11
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants