Skip to content

HRX: pin hrx-system to the held-pin backport carrying #409, #410 and #401 - #423

Merged
bong-water-water-bong merged 2 commits into
mainfrom
1bit/r1-hrx-held-pin
Oct 16, 2026
Merged

bong-water-water-bong merged 2 commits into
mainfrom
1bit/r1-hrx-held-pin

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Moves third_party/hrx-system to 1bit/held-98d05d94-3fixes so three verified fixes reach the engine without moving the held AMD core.

Why a backport line

config/hrx-system-hold.config keeps the engine on 98d05d94 because AMD's hrx-system after it miscompiles long-context attention (ROCm/hrx-system#1535). Three open issues have fixes that are entirely independent of that AMD-core question:

issue fix verified by
engine#409 libhrx bounds-checks a length-less ELF extent instead of skipping the check (hipModuleLoadData on a truncated code object no longer segfaults) before/after probe, e_shoff sweep, CTS hip_module_execution_test 3/3
engine#410 libhip_intercept covers every symbol the backend exports (331 → 544, comm -23 == 0), the documented HIP_PASSTHROUGH_INTERCEPTOR works, init is idempotent rebuilt proxy under LD_PRELOAD against stock ROCm 7.16
engine#401 libhsa no longer opens /dev/accel/accel0 in GPU-only processes, with HRX_ENABLE_XDNA=1 as the escape hatch fuser//proc/<pid>/fd/maps before and after on strixhalo

Waiting for the hold lift would block all three on a 1688-commit core jump and an unresolved long-context question. So the hold now names a branch off the same held commit, 1bit/held-98d05d94-3fixes, which carries exactly those fixes plus the +61/−0 api.h forward-port the regenerated passthrough stubs need (that block is the measured drift cost; without it stubs_generated.c fails with unknown type name 'HIP_LAUNCH_CONFIG').

The held AMD core is unchanged, so #1535 stays exactly where it was. When these fixes are rebased onto 1bit/main — or the hold is lifted — this line is dropped.

What moves

  • third_party/hrx-system: 98d05d94a9f9 → 3319e536196b99119f30f78b60f39a340aee6a5d. GitHub's compare API says ahead 4, behind 0, which is what tools/check_pins.py requires.
  • config/hrx-system-hold.config: names the new commit and records why the line exists and when it goes away.
  • config/hrx-long-prompt-validated-pin.config: updated to the new pin only after tests/hrx_long_prompt.py passed against a build at it (see below).
  • docs/hrx.md: documents the backport line beside the hold.

third_party/llama.cpp is untouched (ad18320a).

Verification

Built with -DONEBIT_HRX=ON at exactly this pair (engine main 31091dc, third_party/llama.cpp ad18320a2955736d62b6e2b39908aeed6a2b5cd9, third_party/hrx-system 3319e536196b99119f30f78b60f39a340aee6a5d), toolchain /home/bcloud/sdk-gfx1201-devel, on gfx1151/Strix Halo. Build exit 0. Numbers are contended (load ~10, other agents resident) and labelled as such.

gate result
Build exit 0; hrx/llama/bin/{llama-server,llama-bench,test-backend-ops} and 1bit produced
Pin gate (tests/hrx_long_prompt.py) PASS — 7082-token prompt: expected 7341, got '7341', exit 0, Tctl 49→55 C. Server argv is exactly what 1bit serve --device hrx builds (llama-server -m Qwen3-0.6B-Q4_K_M.gguf -ngl 99 -dev HRX0 -c 8192 --jinja, fingerprint b1-ad18320)
Op gate (test-backend-ops -b HRX0) 1141/1141 passed, Backend HRX0: OK, 0 failures
llama-bench pp512 18642.37, tg128 316.71 tok/s (contended; a second arm 18221/308.16)
engine#409 full code object ret=0; 64-byte truncation ret=200, process exit 0, no SIGSEGV (was SIGSEGV/139)
engine#410 comm -23 between the R1-built backend's 540 HIP exports and libhip_intercept's 544 defined symbols = 0 (was 331 of 540 covered)
engine#401 GPU-only llama-server and test-backend-ops hold 0 /dev/accel/accel0 fds; fuser -v /dev/accel/accel0 never lists them

One finding worth reading before you trust the pin gate

tests/hrx_long_prompt.py used to send enable_thinking: false at the top level, which llama-server no longer honours. On this pin the probe therefore spent its whole 16-token budget on reasoning and returned an empty answer — i.e. the gate could only fail, for a reason unrelated to the pin, and the previously recorded 98d05d94 validation cannot have exercised this code path either. PR #399 fixes the probe to send chat_template_kwargs (as every other engine caller already does) and is merged here as 7a7361a; the PASS above is the first honest run of that gate at either pin. This PR's validated-pin record rests on that run.

Notes for the reviewer

  • This is a deliberate branch off a held pin, not a rebase of 1bit/main; the four commits are the three fixes plus the header port, and the only hand-written code among them is the additive api.h block.
  • CI builds without HRX, so the HRX-specific gates below are the ones that matter, and the pin record is what CI enforces.

bong-water-water-bong added 2 commits October 16, 2026 00:49
…401

The hold stays: this is 1bit/held-98d05d94-3fixes, a branch off the held
98d05d94 carrying the three verified fixes plus the +61/-0 api.h port the
regenerated passthrough stubs need. The held AMD core is unchanged, so the
ROCm/hrx-system#1535 long-context question is untouched.

tools/check_pins.py: 98d05d94...3319e5361 is ahead 4, behind 0. The
long-prompt record is updated because tests/hrx_long_prompt.py passed against
a build at this pin (7082-token prompt, expected 7341, got 7341).
@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Hold Commit Update

The hold commit has been updated to 3319e536196b99119f30f78b60f39a340aee6a5d which is a backport branch carrying three verified fixes. Ensure that this change is correctly reflected in all related documentation and that the fixes are indeed compatible with the current engine setup.

# 2026-10-16, R1 backport (owner decision): the hold now names `1bit/held-98d05d94-3fixes`,
# a branch off the held pin carrying three verified fixes - engine#409 (libhrx bounds-checks a
# length-less ELF extent instead of skipping the check), engine#410 (libhip_intercept covers
# every HIP symbol the backend exports; env-var and double-init defects fixed) and engine#401
# (libhsa no longer opens /dev/accel in GPU-only processes) - plus the +61/-0 `api.h`
# forward-port the regenerated stubs need. The held AMD core is unchanged, so the #1535
# question stays exactly where it was. The durable move is still to rebase these fixes onto
# our line (or lift the hold) once the long-prompt check passes there; this line exists so
# three small fixes do not have to wait for that.
3319e536196b99119f30f78b60f39a340aee6a5d
Documentation Update

The documentation has been updated to reflect the new backport line 1bit/held-98d05d94-3fixes. Verify that the description of the fixes and their impact is accurate and complete.

- **The backport line (2026-10-16).** The hold now names `1bit/held-98d05d94-3fixes`, a branch off
  that same held commit carrying three fixes that have nothing to do with the AMD-core move:
  engine#409 (libhrx bounds-checks a length-less ELF extent instead of skipping the check),
  engine#410 (`libhip_intercept` covers every HIP symbol the backend exports; the documented
  interceptor variable works and init is idempotent) and engine#401 (libhsa no longer opens
  `/dev/accel` in GPU-only processes), plus the +61/−0 `api.h` forward-port the regenerated
  passthrough stubs need. Nothing under the held AMD core changes, so the #1535 question is
  untouched. When those fixes are rebased onto `1bit/main` — or the hold is lifted — this line
  is dropped; until then it is what lets three small fixes ship without a 1688-commit core jump.

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