Skip to content

fix(kpack): fall through to next matching kpack archive on kernel-not-found - #12518

Merged
nunnikri merged 1 commit into
developfrom
users/nunnikri/fix-kpack-xnack-fallthrough
Oct 1, 2026
Merged

nunnikri merged 1 commit into
developfrom
users/nunnikri/fix-kpack-xnack-fallthrough

Conversation

@nunnikri

Copy link
Copy Markdown
Contributor

Motivation

JIRA ID: ROCM-31644

kpack_load_code_object() committed to the first archive whose architecture tag matched the requesting agent's ISA, and returned KPACK_ERROR_KERNEL_NOT_FOUND immediately if that archive didn't hold the kernel — even when a less-specific but still ISA-compatible archive existed and actually held it. A single architecture's kernels can be split across multiple kpack archives (e.g. an xnack-tagged archive holding a handful of kernels for one binary, plus a bare/generic archive holding the rest for a different binary sharing the same kpack group), so the loader needs to try every compatible candidate before giving up.

This revives #9874, which fixed this exact defect for ROCm/TheRock#7081 (torch 2.12 + ROCm 7.14, hipErrorInvalidImage on gfx90a) but was closed without merging once that specific trigger was worked around in PyTorch's own packaging rather than by fixing the loader.

The same underlying defect is independently hitting rocFFT-test on gfx950 ASAN builds (see Issue Tracking). Root cause there is upstream in rocFFT's own build: projects/rocfft/CMakeLists.txt unconditionally strips all GPU_TARGETS feature flags before compiling (list(TRANSFORM GPU_TARGETS REPLACE ":[^;]+" ""), comment: "rocFFT builds generic code for each target"), so rocFFT-test's kernels always land in the bare, untagged kpack archive — even on ASAN builds where TheRock's own sanitizer configuration deliberately forces gfx950 → gfx950:xnack+. Meanwhile hipFFT-test, a separate project sharing the same fft_test kpack group, has no such stripping logic and is correctly tagged.

Technical Details

  • kpack_load_code_object(): instead of stopping at the first archive whose architecture tag matches, collect every (archive, matched_target) candidate across the full compatible-target power set for an architecture (most specific first), then try kpack_get_kernel() against each in order until one succeeds.
  • Every miss along the way is logged and recorded in last_err; if every candidate is exhausted without success, the original error (KERNEL_NOT_FOUND, or another error type) is still returned — preserving the existing ARCH_NOT_FOUND vs KERNEL_NOT_FOUND distinction for accurate diagnostics, unchanged from fix(kpack): fall through to next matching archive on kernel-not-found #9874's original behavior.
  • Rebased cleanly onto current develop (which already has fix(kpack): Fix kpack loader to fall through to the base kpack  #9953's partial fix) with zero conflicts in the affected file.
  • Added a second regression test alongside fix(kpack): fall through to next matching archive on kernel-not-found #9874's original 1-feature case (gfx90a:xnack- → falls through to bare gfx90a): a 2-feature (sramecc+/xnack+) scenario mirroring the rocFFT-test/gfx950 case exactly, where two of the four power-set candidates have no archive at all — not merely a kernel-not-found miss — to cover that the collection loop correctly skips candidates with no matching archive on the way to the one that has it.

Test Plan

  • Generated all four .kpack test fixtures via generate_test_data.py (ran cleanly, verified output).
  • Traced the fix logic by hand against the real archive layout pulled from a gfx950 ASAN tarball (kpack_get_binary/kpack_get_architecture listing) to confirm it resolves the actual ROCM-31644 failure.
  • Not yet done: a real compiler/CTest pass of test_kpack_runtime (including the pre-existing LoadCodeObject_KernelNotFound test, to confirm no regression there).

Test Result

Pending

kpack_load_code_object() committed to the first archive whose architecture
tag matched the agent's ISA and returned KPACK_ERROR_KERNEL_NOT_FOUND
immediately on a miss, even when a less-specific but still ISA-compatible
archive existed and held the kernel. A single architecture's kernels can be
split across multiple kpack archives (e.g. an xnack-tagged archive holding
a handful of kernels for one binary, plus a bare/generic archive holding
the rest for a different binary in the same kpack group), so the loader
must try every compatible candidate before giving up.

Revives #9874, which fixed this for ROCm/TheRock#7081
(torch 2.12 + ROCm 7.14, gfx90a) but was closed without merging once that
specific trigger was worked around in packaging rather than by fixing the
loader. The same defect independently affects rocFFT-test on gfx950 ASAN
builds (ROCM-31644): rocFFT's own CMakeLists.txt strips all GPU_TARGETS
feature flags before compiling ("rocFFT builds generic code for each
target"), so its kernels always land in the bare, untagged archive even
when a sibling binary sharing the same kpack group (hipFFT-test) is
correctly tagged xnack+. #9874's diff applied cleanly against current
develop with no conflicts.

Changes:
- Collect every (archive, matched_target) candidate across the full
  compatible-target power set for an architecture, instead of stopping at
  the first archive whose tag matches, then try kpack_get_kernel() against
  each in order (most specific first) until one succeeds.
- Every miss is logged and recorded as last_err; if every candidate is
  exhausted, the original KERNEL_NOT_FOUND (or other) error is still
  returned, preserving the existing ARCH_NOT_FOUND vs KERNEL_NOT_FOUND
  distinction for accurate diagnostics.
- Added a second regression test alongside the original 1-feature case:
  a 2-feature (sramecc+xnack) scenario mirroring ROCM-31644 exactly, where
  two of the four power-set candidates have no archive at all (not merely
  a kernel-not-found miss), to cover that this loop correctly skips
  candidates with no matching archive on the way to the one that has it.
@nunnikri nunnikri added the ci:asan Opt-in to building ASAN label Sep 30, 2026
@therock-pr-bot

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🚫 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?

@nunnikri
nunnikri merged commit 694068a into develop Oct 1, 2026
200 of 218 checks passed
@nunnikri
nunnikri deleted the users/nunnikri/fix-kpack-xnack-fallthrough branch October 1, 2026 23:43
@systems-assistant

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

Labels

ci:asan Opt-in to building ASAN organization: ROCm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants