Skip to content

enable gfx1250 generic target in CLR/ROCr - #9741

Open
iassiour wants to merge 2 commits into
developfrom
users/iassiour/gfx12-5-generic-gfx1250
Open

enable gfx1250 generic target in CLR/ROCr#9741
iassiour wants to merge 2 commits into
developfrom
users/iassiour/gfx12-5-generic-gfx1250

Conversation

@iassiour

@iassiour iassiour commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Motivation

The compiler side already supports it — LLVM has had the target since llvm/llvm-project#183381, and device-libs/comgr carry it on amd-staging — but the ROCm runtime and hip-tests in rocm-systems do not. As a result, a gfx12-5-generic code object cannot load on gfx1250: ROCr's loader can't determine its ISA, and CLR/hip-tests don't recognize the target.

This PR adds the missing runtime support (CLR + ROCr) and wires up the generic-target compiler tests for gfx1250.

Technical Details

ROCr (hsa-runtime)

  • Add EF_AMDGPU_MACH_AMDGCN_GFX12_5_GENERIC (0x5b) to inc/amd_hsa_elf.h.
  • Add a GetMachInfo case in libamdhsacode/amd_hsa_code.cpp so AmdHsaCode::GetIsa() can resolve the code object's ISA name (this was the first hard failure: HSA_STATUS_ERROR_INVALID_CODE_OBJECT).
  • Register gfx12-5-generic in IsaRegistry::GetSupportedGenericVersions() and set gfx1250's generic to gfx12-5-generic in the ISA registry (core/runtime/isa.cpp), so the loader's generic-vs-agent compatibility check accepts it.

CLR (rocclr / hipamd)

  • Add gfx12-5-generic to Isa::supportedIsas() (rocclr/device/device.cpp) so findIsa() recognizes the ISA.
  • Map gfx1250 -> gfx12-5-generic in the fatbin TargetGenericMap (hipamd/src/hip_fatbin.cpp) so bundle selection picks the generic code object.

hip-tests

  • Map gfx1250 -> gfx12-5-generic in getGenericTarget() (hipTestMain/hip_test_features.cc) so isGenericTargetSupported() returns true on gfx1250.
  • Build the generic-target tests with --offload-arch=gfx12-5-generic (unit/compiler/CMakeLists.txt).
  • Re-enable Unit_test_generic_target_only_in_{compressed,regular}_fatbin (drop the stale Rock_Linux_Failures_on_gfx94X disable in config/configs/unit/compiler.yaml).

Out of scope (upstream / llvm-project): comgr (comgr-isa-metadata.def) and device-libs (oclc/CMakeLists.txt) also need gfx12-5-generic; those already carry it on amd-staging (device-libs via #3070). For this PR to work end-to-end, the build's toolchain must include those.

Issue Tracking

JIRA ID : AIRUNTIME-2596

Test Plan

  • Built CLR + ROCr with these changes and verified the full gfx12-5-generic load path end-to-end (ISA-name resolution → fatbin bundle selection → ROCr code-object load → kernel execution) with a --offload-arch=gfx12-5-generic compressed and regular fatbin.
  • Ran the four generic-target compiler tests: Unit_test_generic_target_{in,only_in}_{compressed,regular}_fatbin.
  • For the re-enabled _only_ tests, verified the generic-only child executables load their gfx*-generic code objects and produce correct results on real gfx942 (MI300X) hardware, confirming the prior Rock_Linux_Failures_on_gfx94X disable no longer reproduces.

Test Result

  • All four generic-target test cases pass; gfx12-5-generic compressed/regular fatbins load and execute correctly with the runtime changes.
  • The re-enabled _only_ tests pass on gfx942.

Note on the _only_ tests: this branch intentionally omits the deployment fix for the generic-only child executables. They are installed via the custom-file path (0644, no execute bit) and rely on the test's runtime chmod u+x. If a Linux CI deploy is read-only, these may still fail — in which case the children should be installed as PROGRAMS (0755). Leaving that out here to confirm the current behavior in CI.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/clr/hipamd/src/hip_fatbin.cpp, projects/clr/rocclr/device/device.cpp, projects/hip-tests/catch/hipTestMain/hip_test_features.cc, projects/rocr-runtime/runtime/hsa-runtime/core/runtime/isa.cpp, projects/rocr-runtime/runtime/hsa-runtime/inc/amd_hsa_elf.h (+1 more); no test file found
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled

🎉 All policy checks passed!

📖 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 Aug 6, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@iassiour iassiour changed the title feat(gfx12-5-generic): enable gfx1250 generic target in CLR/ROCr and … enable gfx1250 generic target in CLR/ROCr Aug 6, 2026
@iassiour
iassiour force-pushed the users/iassiour/gfx12-5-generic-gfx1250 branch from 0e477a5 to a54e4b4 Compare August 6, 2026 00:51
@iassiour
iassiour requested a lite review from Copilot August 6, 2026 00:51

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.

Pull request overview

This PR enables end-to-end support for the gfx12-5-generic code object target on gfx1250 across ROCr (loader/ISA registry), CLR (ISA recognition and fatbin bundle selection), and hip-tests (generic-target detection and compiler test coverage).

Changes:

  • ROCr: add ELF mach/ISA decoding and register gfx12-5-generic as a supported generic ISA; map gfx1250gfx12-5-generic.
  • CLR: recognize gfx12-5-generic in the supported ISA list and map gfx1250gfx12-5-generic during fatbin target selection.
  • hip-tests: build compiler tests for gfx12-5-generic, map gfx1250gfx12-5-generic, and re-enable generic-target-only tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
projects/rocr-runtime/runtime/hsa-runtime/libamdhsacode/amd_hsa_code.cpp Adds mach-to-ISA-name resolution for gfx12-5-generic code objects.
projects/rocr-runtime/runtime/hsa-runtime/inc/amd_hsa_elf.h Introduces an ELF e_flags mach constant for gfx12-5-generic.
projects/rocr-runtime/runtime/hsa-runtime/core/runtime/isa.cpp Registers gfx12-5-generic and maps gfx1250 generic compatibility accordingly.
projects/clr/rocclr/device/device.cpp Adds gfx12-5-generic to CLR’s supported ISA table so ISA lookup can succeed.
projects/clr/hipamd/src/hip_fatbin.cpp Maps gfx1250 targets to the gfx12-5-generic bundle during fatbin selection.
projects/hip-tests/catch/unit/compiler/CMakeLists.txt Builds compiler unit tests with --offload-arch=gfx12-5-generic.
projects/hip-tests/catch/hipTestMain/hip_test_features.cc Enables generic-target support checks on gfx1250 by mapping to gfx12-5-generic.
projects/hip-tests/catch/config/configs/unit/compiler.yaml Re-enables previously-disabled generic-target-only compiler tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread projects/rocr-runtime/runtime/hsa-runtime/inc/amd_hsa_elf.h
@iassiour
iassiour marked this pull request as ready for review August 6, 2026 09:05
@iassiour
iassiour force-pushed the users/iassiour/gfx12-5-generic-gfx1250 branch 2 times, most recently from 655d034 to 325059e Compare August 6, 2026 19:05

@dayatsin-amd dayatsin-amd 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.

Thanks. Approved

iassiour and others added 2 commits August 7, 2026 17:51
Rename RESERVED_0X49 to GFX1250 at 0x049 (its correct sorted position) and
remove the out-of-order duplicate GFX1250 = 0x049 that sat after GFX950.
No functional change (gfx1250 stays 0x049); removes the reserved alias /
duplicate flagged in review.

Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
@iassiour
iassiour force-pushed the users/iassiour/gfx12-5-generic-gfx1250 branch from 325059e to 294c960 Compare August 7, 2026 16:51
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.

3 participants