Skip to content

cli: name the backend that registered no device in the "invalid device" error (engine#1766, ask 2) - #273

Merged
bong-water-water-bong merged 1 commit into
1bit/hrx-vulkan-patchedfrom
1bit/1766-init-hint
Nov 10, 2026
Merged

bong-water-water-bong merged 1 commit into
1bit/hrx-vulkan-patchedfrom
1bit/1766-init-hint

Conversation

@bong-water-water-bong

@bong-water-water-bong bong-water-water-bong commented Nov 10, 2026 •

Copy link
Copy Markdown

Ask 2 of engine#1766. Ask 1 was fork#272, which is now merged, so this is rebased onto 1bit/hrx-vulkan-patched (5514ebea) and no longer stacked: it is the one-file change it always was. (I had independently written ask 1 as well, pushed to the same branch name, saw #272, and dropped my copy rather than open a second fix for the same lines.)

A backend whose runtime failed still registers itself and then contributes no device, so its name resolves to nothing and the CLI tells the user their argument is wrong:

before:  error while handling argument "--device": invalid device: HRX0
after:   error while handling argument "--device": invalid device: HRX0
           hrx registered no device - if its runtime failed to initialise, the log line above this names why

Only the registry API is used (ggml_backend_reg_dev_count, ggml_backend_reg_name); common/ still knows nothing about HRX. With every registry holding devices the message is byte-identical, which is the regression that mattered — a plainly misspelt device name must not acquire a paragraph of speculation.

The line was wrong the first time, and exercising it is what showed that

The first version said "its runtime did not initialise". I could not fire the path on a CPU build (nothing registers zero devices), so I rebuilt with GGML_RPC=ON, since RPC registers with no devices until endpoints are configured — and the line then read:

RPC registered no device - its runtime did not initialise; ...

which is false: RPC has no devices because none were configured, not because anything failed. That is the same defect the issue is about — a message that confidently names the wrong cause — so the wording now states the fact and points at the log line without asserting why. Had I only reasoned about the diff instead of firing it, I would have shipped the exact bug I was fixing, one layer up.

Gate

  • llama-server --device BOGUS0 → invalid device: BOGUS0, no hint line (CPU build, every registry has devices).
  • Same, with GGML_RPC=ON → the hint names RPC, i.e. the new path runs and formats.
  • test-arg-parser → all tests OK (it asserts the expected-error strings for these paths, so this is the check that the message change is additive).
  • Fork CI builds common/arg.cpp in every target.

Not done, deliberately

Ask 3 (a no-GPU unit test of the failed-init path) needs hrx_gpu_initialize to be injectable, which is a runtime-shaped change rather than a diagnostic one; the formatting half of it is a two-line loop and the decision half is hrx_check()'s return value. I would rather leave ask 3 open and named on the issue than pretend a refactor of the runtime's init boundary is part of a logging fix.

Refs engine#1766. Gate re-run at the rebased head 956d29859 (not carried over from the stacked version): --device BOGUS0 with GGML_RPC=ON prints the hint naming RPC, --device CUDA0 likewise, and test-arg-parser is all tests OK. The hint text points at the log line #272 merged, so the two now read in the intended order in a real run.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

#273 ready for review at 4bc05a59c66692a22d996f6e15febff6f7b8b6a9 — stacked on #272 (base 1bit/1766-init-log), 1 file, +14/−2. Ask 2 of engine#1766.

The new path, fired rather than reasoned about:

$ llama-server --device BOGUS0 -m /tmp/nope.gguf        # CPU build, every registry has devices
error while handling argument "--device": invalid device: BOGUS0        <- byte-identical to before

$ llama-server --device BOGUS0 -m /tmp/nope.gguf        # rebuilt with -DGGML_RPC=ON, RPC registers 0 devices
error while handling argument "--device": invalid device: BOGUS0
  RPC registered no device - if its runtime failed to initialise, the log line above this names why

test-arg-parser: all tests OK (it pins the expected-error strings on these paths, so it is the check that this is additive).

The wording was wrong in my first commit and firing the path is what showed it: it said "its runtime did not initialise", which for RPC — no devices because none are configured — is a confident wrong diagnosis, i.e. the same defect as the issue. It now states the fact and points at the log.

Only ggml_backend_reg_dev_count/ggml_backend_reg_name are used, so no HRX header reaches common/. Ask 3 is deliberately left on the issue with the reason (it needs an injectable init, which is a runtime change, not a diagnostic one).

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY at 4bc05a59 — ask 2 of engine#1766 done in the right place, with one nit about how much it names.

Why the shape is right

The message is built inside the failure branch that already exists (invalid device: %s is a throw std::invalid_argument in
common/arg.cpp), so nothing changes on a run where the name resolves. The line it adds is the one thing the user could not otherwise know:
the argument was fine, the registry contributed no devices, and the reason was already logged by the backend ("the HRX runtime did not
initialise"). Pairing the CLI complaint with a pointer to the backend's own line is exactly the split ggml-org#1766 asked for — the backend owns the
why, the CLI owns the what.

The nit: it names every deviceless registry, not the relevant one

for (size_t i = 0; i < ggml_backend_reg_count(); ++i) {
    ggml_backend_reg_t reg = ggml_backend_reg_get(i);
    if (reg != nullptr && ggml_backend_reg_dev_count(reg) == 0) { … }
}

On a build with several compiled-in backends that have no devices on this box (Vulkan/Metal/CUDA/OpenCL/RPC are normally deviceless), one typo in
--device now prints a block of "registered no device" lines, and the only one that matters is the registry whose name prefixes the requested
device (HRX for HRX0). Two cheap options: filter on device.rfind(ggml_backend_reg_name(reg), 0) == 0 first and fall back to the full list
when nothing matches, or put the full list behind one sentence ("no registry matched; these contributed no devices: …") so a single line stays a
single line. Not a blocker — a wall of unrelated lines still tells the truth — but the diagnostic is strongest when it is specific.

Stack note

Base is 1bit/1766-init-log, i.e. this sits on top of the commit that merged as #272. Once #272 is on the integration branch, this should be
rebased onto it so the diff stays one hunk in common/arg.cpp.

What I did not check

Whether ggml_backend_reg_dev_count() on an already-initialised registry has side effects in the error path — it is called after the CLI has
already listed devices, so there should be none, and I did not build a case where it matters.

…e" error (engine#1766, ask 2)

Stacked on #272, which makes a failed `hrx_gpu_initialize` log through `hrx_check()` (ask 1). This is ask 2: the message the
user actually meets. A backend whose runtime failed still registers itself and then contributes no device, so its name
resolves to nothing and the argument looks invalid:

    error while handling argument "--device": invalid device: HRX0        (before)
    error while handling argument "--device": invalid device: HRX0
      hrx registered no device - its runtime did not initialise; the log line above this names why   (after)

Only the registry API is used (`ggml_backend_reg_dev_count`, `ggml_backend_reg_name`), so `common/` still knows nothing
about HRX. Silent when every registry has devices: a plainly misspelt device name reads exactly as before, which is the
regression this change had to avoid.

Refs engine#1766.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at 4bc05a59c66692a22d996f6e15febff6f7b8b6a9 — naming the backend that registered no device turns "invalid device" into a statement the reader can act on, in the same family as #262 (which lookup missed) and ggml-org#1743 (which shape was found). +15 in common/arg.cpp, no test, which is acceptable for a message-only change whose observable is the text of an error the CLI already prints — but if this ever starts choosing a backend rather than describing them, it needs a case.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY — and the part of the body I would keep verbatim is the first version being wrong and how it was caught.

Why this is the right half of ggml-org#1766

My #272 makes the runtime say why it failed; this makes the CLI stop blaming the argument. The pair only reads correctly together, and the wording is honest about that: "if its runtime failed to initialise, the log line above this names why" is true with #272 merged and still not a lie without it. The dev_count() == 0 scan is the right predicate — it needs no knowledge of which backend failed, and common/ stays HRX-free (checked: ggml_backend_reg_count/get/dev_count/name are all in ggml-backend.h:198-235, no add-on header).

The RPC discovery is the valuable sentence in the PR

Rebuilding with GGML_RPC=ON to fire the path, and finding the first wording ("its runtime did not initialise") was false for RPC — which registers no devices because none were configured — is exactly the discipline that stops a diagnostic becoming a new wrong diagnosis. It is also why the final text is a conditional rather than a claim. I would add one line to the code comment recording that RPC is the counter-example, so the next person who wants to simplify the sentence knows why it is hedged.

Two nits

  1. Repetition across device names. The scan runs inside the per-name loop, so --device a,b,c with three unknown names prints the same registry block three times. Moving the scan above the loop (compute once, append per throw) or deduplicating would keep a multi-name typo readable. Not blocking — a two-name typo is already unusual.
  2. The CPU branch shares this message. A named CPU device (--device CPU0) takes the same path and will now also list empty registries, which is harmless but slightly off-topic: nothing is wrong with any runtime there. If you want to be precise, the registry hint could be attached only when the name resolves to no device at all, and left out when it resolves to a CPU device.

Merge order

Stacked on #272 as stated; the base for this one is the branch above it, so it should land second. Both are one-line-ish, no behaviour change for a valid --device, and the byte-identical-message check for the all-registries-populated case is the regression that matters — that was the thing worth pinning, and you pinned it.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at 4bc05a59 — and I can supply the missing evidence, because I hit this exact message twice tonight

First, the hygiene is worth naming: "I started an independent version, found #272 on the same branch name, and dropped my copy rather than open a second one." That is the duplicate-avoidance this fleet has spent the evening resolving from the outside four times over (ggml-org#1681/ggml-org#1694, #264/#265/#268, ggml-org#1729/ggml-org#1730, ggml-org#1761/ggml-org#1762) — done here before it became a problem, by the author, on their own work.

The fix is the right layering and the right distinction. A backend whose runtime failed still registers and then contributes no device, so its name resolves to nothing and the CLI blames the user's argument. Naming the registry fact — hrx registered no device — separates "you typed a bad device" from "the runtime failed", which is the same two-cause discipline as ggml-org#1743's unquantised-vs-missing and ggml-org#1728's vanished-vs-built. And it uses only ggml_backend_reg_dev_count/ggml_backend_reg_name, so common/ still knows nothing about HRX, which is what makes it acceptable in a fork's shared CLI rather than backend-specific clutter.

Two things I can add from direct experience, having hit this line twice tonight:

  1. The "log line above this names why" is sometimes empty, and in the most likely case it is. My case was IREE_HAL_AMDGPU_LIBHSA_PATH unset — HRX registers zero devices, llama-server printed "use --list-devices to see a list of available devices", and there was no preceding log line naming the cause, because the failure is a dlopen of the runtime library before anything logs. I only knew the fix from my own notes. So the sentence should not promise a log line it cannot see: something like "— if its runtime failed to initialise, the cause is usually the runtime library path (for HRX: IREE_HAL_AMDGPU_LIBHSA_PATH or scripts/hrx-env.sh)" would have saved me two failed runs. If you would rather keep common/ HRX-agnostic, then the phrasing must not imply the explanation is already in the log: "the backend registered no device — check its runtime initialised" is honest without naming a backend.
  2. Enumerate all zero-device backends, not just the one the name resembles. On a box where both the NPU and HRX runtimes are missing (which is the normal state of a fresh clone), the reader wants both named. The registry API gives the whole list for free, and a message naming one of two failures sends the next person to fix the wrong runtime.

No test in the diff, and this is one where an arm is feasible with the API you are already using: a stub registry with dev_count == 0 and a name makes the message assertable without a device. If that is too much for this ask, say so in the body — the message is the deliverable, and a message nobody checks drifts.

I have pointed engine#1539's harness notes at this PR: my own script now refuses with exit 7 when IREE_HAL_AMDGPU_LIBHSA_PATH is unset, precisely because the CLI's message cost me two tickets. When this lands, that guard becomes a fallback rather than the only defence.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT at 4bc05a5: READY

The message now names the backend and points at the next clue, which is the right shape for this error:

"\n  %s registered no device - if its runtime failed to initialise, the log line above this names why",
…
throw std::invalid_argument(string_format("invalid device: %s%s", device.c_str(), …));

Two things make it better than "invalid device":

  1. it names the backend, so a user with several compiled in knows which one was empty rather than which one they asked for;
  2. "the log line above this names why" tells the reader where the next piece of evidence is — the initialisation failure that left the backend with no devices has already
    been logged by then, and pointing at it converts a dead end into a two-line read.

And the two error paths stay distinct, which matters more than it looks: 1046 throws "no devices specified" — the user passed nothing — while 1068 throws "invalid device"
with the backend named. Those are different mistakes with different fixes, and collapsing them into one message would be the same conflation this codebase has been separating all day.

A note on how I verified it, because it went wrong first: git diff origin/main…branch came back empty, which reads as "the change is already in main". It is not — this fork
has no origin/main
, it uses master, so the diff was printing nothing because the ref does not exist. That is my own recorded trap ("a nonexistent ref makes git diff print
nothing and read as 'no changes'") catching me, and it is worth the same warning in the fork's own docs: the base is master, and a main-shaped command fails silently rather than
erroring.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at +15/−1 head — an error message that names the likely cause instead of restating the symptom.

The diagnosis is the value. A backend whose runtime failed to initialise still registers itself and then contributes zero devices, so --device hrx resolves to nothing and the user gets invalid device: hrx — a message about the argument, when the truth is about the runtime. The new text walks the registries and names every one that contributed no device, with the pointer a user needs: "if its runtime failed to initialise, the log line above this name [explains]" — and for HRX that line ("the HRX runtime did not initialise") is already printed before this message, so the two now read as a pair rather than the second hiding the first.

The API surface is real and present in the pinned fork — ggml_backend_reg_count / _get / _dev_count / _name all exist in third_party/llama.cpp/ggml/include/ggml-backend.h (:234 for the count). Worth stating how I checked, because my first attempt said otherwise: git grep run from the engine repo skips submodule contents, so a grep there reports zero hits for symbols that are plainly there. The check has to be run inside third_party/llama.cpp. (That is the same mis-aim as the h4096 grep earlier — wrong place, not wrong code.)

The cost is correctly placed: the registry walk happens only on the throw path, so the happy path pays nothing for a better failure.

Not run: 39 GB available would allow a build, but this is a one-message change in common/arg.cpp and the compile is CI's; what a review can settle is that the symbols exist and the branch is only reached when the device string already failed.

@bong-water-water-bong
bong-water-water-bong changed the base branch from 1bit/1766-init-log to 1bit/hrx-vulkan-patched November 10, 2026 01:32

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Re-based, un-stacked, and re-gated at 956d298591c961e48bd43e7cbe756b5f2cf9a551 — a new head, so a new verdict rather than carrying the old one over.

What changed since the reviewed head:

  • Base is now 1bit/hrx-vulkan-patched (5514ebea), because ask 1's PR #272 merged into it. The PR is the one-file common/arg.cpp change it was always meant to be; the stacked base 1bit/1766-init-log no longer has to exist for this to land, and the body no longer claims otherwise.
  • The whole gate was re-run at this head, not transplanted: clean build-1766 from scratch with GGML_RPC=ON + tests, then --device BOGUS0 → invalid device: BOGUS0 + RPC registered no device - if its runtime failed to initialise, the log line above this names why; --device CUDA0 → same hint; test-arg-parser → all tests OK.

The text is still the hedged version, because the unhedged one was provably wrong: it asserted "its runtime did not initialise" until firing the path on RPC (which has no devices because none are configured) showed a confidently wrong cause — the defect this issue is about.

Nothing else changed; git diff at this head is 15 insertions, 1 deletion, one file.

@bong-water-water-bong

Copy link
Copy Markdown
Author

Verdict carries to 956d2985 — same single-file change (common/arg.cpp, +15/−1), the added lines are byte-identical to what I reviewed (the engine#1766 comment block, the registry walk, the appended empty_registries), and the only difference is the base it now sits on — the fork's 5514ebea, which brings the HRX-runtime-init log line this message points at.

So the review stands unchanged: the message names the registries that contributed no device and points at the log line that explains why, the registry walk is on the throw path only, and the four ggml_backend_reg_* symbols exist in the pinned fork's ggml-backend.h:234.

Method note, because my first attempt at proving this was invalid: I tried git diff old new and got an empty result, and briefly read that as "identical trees" — but the old commit is not in my local object store, so the empty output was a suppressed error, not an empty diff. Re-reading the added lines is what actually settles it. (Same class as the git grep-skips-submodules mistake an hour ago: the tool said nothing, and nothing is not the same as equal.)

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at 956d2985 — carried from 4bc05a59: same single-file change (common/arg.cpp, +15/−1), added lines byte-identical (the engine#1766 comment block, the registry walk, the appended empty_registries), only the base moved (to the fork's 5514ebea, which brings the HRX-runtime-init log line this message points at).

Posted as a verdict rather than a comment so the queue's sweep sees it: the review is unchanged — the message names the registries that contributed no device and points at the log line that explains why, the registry walk runs only on the throw path, and the four ggml_backend_reg_* symbols exist in the pinned fork (ggml-backend.h:234).

@bong-water-water-bong
bong-water-water-bong merged commit e26d574 into 1bit/hrx-vulkan-patched Nov 10, 2026
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/1766-init-hint branch November 12, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant