Skip to content

hrx: pin llama.cpp to the hybrid (AMD core + our ggml-hrx) - fixes MUL_MAT failures and #315 NaN - #339

Closed
bong-water-water-bong wants to merge 1 commit into
mainfrom
1bit/hrx-hybrid-pin
Closed

bong-water-water-bong wants to merge 1 commit into
mainfrom
1bit/hrx-hybrid-pin

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Pins third_party/llama.cpp to AMD's core sync with our HRX backend kept whole (f2099e9b), which is the configuration measured to fix both open blockers. third_party/hrx-system is unchanged at 98d05d94 (our loom — AMD's does not export loomc_amdgpu_runtime_global_flags_t, which our loom-jit.cpp needs), so this moves one gitlink.

Measured — fresh build directories, the engine's own ExternalProject arguments (cmake/hrx.cmake: fresh -B, Release, amdclang, -DHRX_SOURCE_DIR=…), never through an incrementally rebuilt nested directory (round 77's lesson):

build full gate test-backend-ops -b HRX0 GLM-4.7-Flash, 269-token, -ub 512
pre-sync core + our ggml-hrx 1073/1073 0 NaN, ' Paris.'
AMD core + our ggml-hrx (this PR) 1073/1073 — 0 FAIL 0 NaN, ' Paris.'
AMD core + AMD's ggml-hrx 1019/1071 — 54 FAIL 18 NaN, ??????

The 54 failures are 40 MUL_MAT (in iq1_s/iq1_m/iq3_xxs/mxfp4/pq2_0/ptq1_0/q2_*) plus others; the NaN is engine#315.

Gates: tools/check_pins.py origin/main -> ok third_party/llama.cpp: 522dab478 -> f2099e9b7 is ahead; tools/registry_build.py --check-pins passes and the registry records the new source.

Why our backend and not AMD's: replacing our ggml-hrx with AMD's refactor is what carries both defects — see engine#315 and the evidence table. The hrx-vulkan-patched line already carries our patched ggml-hrx; this is that choice made explicit on the core sync.

Note this is offered as the alternative to #329 (which pins the pure sync 56c3c8a3). Both are verified; the difference is whether AMD's ggml-hrx or ours is in the pinned tree, and that is the call this PR is asking for rather than assuming.

AMD core sync with our HRX backend kept whole — the configuration measured to fix
both the MUL_MAT failures and the all-NaN logits:

  test-backend-ops -b HRX0     AMD core + our ggml-hrx   1073/1073  (0 FAIL)
                               AMD core + AMD ggml-hrx   1019/1071  (54 FAIL)
  GLM-4.7-Flash, 269-token, -ub 512
                               our backend   0 NaN, reply " Paris."
                               AMD backend   18 NaN, reply "??????"

third_party/hrx-system is unchanged at 98d05d94 (our loom: AMD's does not export
loomc_amdgpu_runtime_global_flags_t, which our loom-jit.cpp needs), so this moves
one gitlink: llama.cpp 522dab47 -> f2099e9b.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Superseded by #336, which makes the same change — llama.cpp to f2099e9b, registry source updated — as the corrected pin bump, and supersedes the staged pin in #329. Keeping #336 as the single pin PR; the measurement table from this PR is reproduced there, so nothing is lost by closing this one.

The one thing worth preserving from here, in case it is useful later: the hybrid branch was assembled and verified locally (1bit/amd-core-our-hrx, f2099e9b), and tools/check_pins.py origin/main reports ok … is ahead with registry_build.py --check-pins at rc=0 for it.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

315 - Partially compliant

Compliant requirements:

  • The PR pins llama.cpp to a version that fixes the HSA memory fault and NaN logits issue
  • The PR includes validation results showing 0 NaN and 0 FAIL in the test suite
  • The PR addresses the specific issue described in the ticket

Non-compliant requirements:

  • The PR does not include a detailed analysis of the root cause of the issue

Requires further human verification:

  • The validation gate needs to be manually verified on Strix Halo hardware

329 - Partially compliant

Compliant requirements:

  • The PR pins third_party/llama.cpp to the AMD core with our HRX backend
  • The PR preserves our loom-jit.cpp changes that require loomc_amdgpu_runtime_global_flags_t

Non-compliant requirements:

  • The PR does not fully address the validation gate requirements (e.g., building and running tests on Strix Halo)

Requires further human verification:

  • The validation gate needs to be manually verified on Strix Halo hardware
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Outdated Architecture Mapping

The architecture mapping in registry/architectures.json still refers to the old llama.cpp commit, which may cause confusion or incorrect assumptions about which version of the code is being used for HRX backend support. This should be updated to reflect the new pinned commit.

"llama.cpp (hrx)": "f2099e9b75a58b4489da013fa5dbe267c4034f51"

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.

1 participant