Skip to content

Commit 3115269

Browse files
Isolate upstream #30270: correct fix, no measurable effect here, and a guard proven red
A clean isolation of llama.cpp PR #30270 (commit c28e174141dd, merged 2026-10-11), which was merged with, in the author's own words, a better by-eyeball measurement on a FreeBSD box with mismatched RAM clocks, no numbers and no review comments. One machine, one source tree copied twice, one variable: the seven lines present or removed, with the removal printed into the run log. Numbers from upstream's own llama-bench with 5 repetitions, so each carries a spread, and our JNI layer is not in the picture. The result is negative and that is the point: every difference is inside plus or minus 2 percent with random signs, across four F16C tiers, and the two non-F16C control modules move just as much as the test cases. haswell is plus 1.8 percent at 16 threads and minus 0.8 at 8. The fix is still right -- the F16C branch defined only the COMPUTE macro, so the selected one fell through to a 64K lookup table and the hardware instruction was never used -- but its gain is machine and workload dependent, and on a bandwidth-rich server CPU with a 4-bit quant it is zero. The author's own caveat fits that. It also refutes something this repository wrote an hour earlier. A haswell jump from 221.8 to 422.7 between a b11538 4-vCPU run and a b11558 64-vCPU one had been read as consistent with #30270, because haswell gained far more than the AMX module. The isolation says #30270 contributes nothing, so that jump was the instance size: 4 threads on a 64-vCPU machine have far more memory bandwidth to themselves. The correction is kept in cpu-backend-modules.md rather than quietly removed, because the next person comparing two tables across a pin and a machine size will be tempted the same way. And a guard that makes the omission impossible, proven both ways with clang -fsyntax-only against the real b11574 header: it passes on the fixed tree, fails with the #error on the reverted one, does not fire without __F16C__, and both trees compile when it is absent -- so the guard is the only thing that changes the outcome. The first attempt at that test proved nothing, because it appended the guard after the include and the header's own fallback always defines the macro; a guard that passes in the broken case has to be suspected of being in the wrong place. Both the mistake and the fix are recorded. cpu-backend-modules.md also gains the b11558 tables from a 64-core Granite Rapids as an additional section, never editing the b11538 ones, as that file's house rules require. They answer the open question about high core counts: at 64 threads haswell leads token generation at 192.6 while sapphirerapids is last at 181.8 -- and ggml picks the latter, which sharpens the upstream module-scoring finding rather than adding a new one. TODO.md: five rows of the measurement table are struck through, leaving only the two that need hardware nobody rents, and the upstream section gains a fifth item that is a small PR rather than an issue -- move the two defines into the branch they complete, add the guard, and offer the measurement with it rather than disputing the change. check-natives.py 0 disagreements, 122 buildcheck tests green, and a search over TODO.md and docs/benchmarks for bucket names, account ids, instance ids and presigned URLs finds nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E2h8gXyyE5UeimkL9vQv9G
1 parent ee7dcc1 commit 3115269

4 files changed

Lines changed: 274 additions & 6 deletions

File tree

‎TODO.md‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -274,12 +274,12 @@ Kept as a list because each entry names the machine it needs -- the harness is
274274
| what | needs | why it cannot wait for CI |
275275
|---|---|---|
276276
| ~~the cost of `GGML_OPENMP=OFF`~~ | ~~16-vCPU Linux~~ | **done 2026-10-11: no measurable cost** |
277-
| **re-baseline the module tables at the current pin** | the same instances | the tables are b11538, and **#30270 in the b11556 range fixes F16C on AVX2**, i.e. the `haswell` class every "against haswell" column is computed against -- so nothing measured from b11556 on may be compared with that page (noted there too) |
278-
| Does a self-contained jar load on a **slim/distroless** image? | any Linux instance, a container | CI runs on full runners, so it cannot see this class at all |
279-
| Does **`75-real 80-real 90-real`** make a T4 work? | a rebuilt CUDA module + a `g4dn` instance | the module has to be built first, so this follows a CI run |
280-
| **Windows extraction reuse**: second start copies nothing, a running JVM's locked DLLs survive another build's cleanup | a rented Windows instance, two JVMs | file locking is Windows-only and needs two processes |
281-
| the **`sapphirerapids` vs `haswell`** gap at high core counts | a large Intel instance | one 4-vCPU table exists; generation ranking can reverse with threads |
282-
| **OpenVINO on an Intel GPU/NPU** | hardware nobody rents | the 172x figure is the CPU-plugin fallback only, so nothing is known about the hardware it is for |
277+
| ~~re-baseline the module tables at the current pin~~ | — | **done 2026-10-12 at b11558 on 64 cores**, and the suspicion behind it was **refuted**: #30270 changes nothing measurable ([`docs/benchmarks/upstream-f16c-30270.md`](docs/benchmarks/upstream-f16c-30270.md)), so the baseline moved with the machine size, not with the pin |
278+
| ~~Does a self-contained jar load on a slim/distroless image?~~ | — | **done 2026-10-12**: distroless and `temurin:21-jre` load, `temurin:21-jre-alpine` is refused by name (`os.name=Linux-Musl`) -- see [`docs/benchmarks/native-loading.md`](docs/benchmarks/native-loading.md) |
279+
| ~~Does `75-real 80-real 90-real` make a T4 work?~~ | — | **done 2026-10-12: yes.** Three `exit 134` aborts became three clean runs on the same AMI, driver and GPU |
280+
| ~~Windows extraction reuse~~ | — | **done 2026-10-12 locally**, no rented machine needed: ~810 ms saved per start, and a sweep leaves exactly the five files a running JVM still holds |
281+
| ~~the `sapphirerapids` vs `haswell` gap at high core counts~~ | — | **done 2026-10-12: the ranking reverses.** At 64 threads `haswell` leads token generation at 192.6 while `sapphirerapids` is last at 181.8 -- and ggml picks the latter |
282+
| **OpenVINO on an Intel GPU/NPU** | hardware nobody rents | the 21.7x/336x figures are the CPU-plugin fallback only, so nothing is known about the hardware the jar is actually for |
283283
| the two **SME** aarch64 modules | not rentable anywhere | — |
284284

285285
## Open — jllama-specific
@@ -860,6 +860,45 @@ three different subsystems and a combined report gets triaged as one.
860860
| 3 | `ggml-rpc: the client answers every supports_op with true` | the RPC section of `CLAUDE.md`: a served device that lacks an op **aborts the server**. Reproduced on the macOS CI runners' paravirtual Metal GPU, which has no `MUL_MAT` and took the first run down; it is why every test here serves `CPU` by name | upstream's own TODO comment admits it. Should the client forward the query to the server, or should `rpc-server -d` refuse a device whose op set it cannot report? |
861861
| 4 | `windows-rocm: 83,362 compiler warnings, against 9 on linux-rocm` | the table below in this section (714,345 log lines against 2,455; the per-class counts; all of them in `ggml/src/ggml-cuda/*.cu`) | the cause is the language, not the arch count: HIP sources are compiled as plain C++ on Windows (`CXX_IS_HIPCC` on `WIN32`) and as HIP on Linux. Fix the warnings, or the C++-on-Windows choice? Their own `windows-rocm` job pays the identical cost |
862862

863+
#### A fifth, and this one is a small PR rather than an issue: the #30270 follow-up
864+
865+
Unlike the four above, here there **is** something to offer -- and the measurement is already done
866+
([`docs/benchmarks/upstream-f16c-30270.md`](docs/benchmarks/upstream-f16c-30270.md)).
867+
868+
[#30270](https://github.com/ggml-org/llama.cpp/pull/30270) (commit `c28e174141dd`, merged 2026-10-11)
869+
fixes a real defect: the `#elif defined(__F16C__)` branch of `simd-mappings.h` defined only
870+
`GGML_CPU_COMPUTE_FP16_TO_FP32`, so the selected macro fell through to the 64K lookup table and an
871+
F16C CPU never used the hardware instruction. NEON, POWER9 and RISC-V each define **both** macros in
872+
their branch; only F16C defined half of its own decision.
873+
874+
Two things to offer, both independent of throughput:
875+
876+
1. **Move the two `#define`s into the branch**, where the COMPUTE macros are defined, as every other
877+
branch does. They currently sit **88 lines** further down and outside the chain, which is why the
878+
new code has to explain the ordering in its own comment ("checks below don't test for `__F16C__`").
879+
A future reordering of the file breaks it **silently**, back to the lookup table -- the exact defect
880+
that went unnoticed until now.
881+
2. **Add a guard so the omission cannot recur**, after the branch chain and before the fallback:
882+
883+
```c
884+
#if defined(GGML_CPU_COMPUTE_FP16_TO_FP32) && !defined(GGML_CPU_FP16_TO_FP32)
885+
#error "a branch defined GGML_CPU_COMPUTE_FP16_TO_FP32 but not GGML_CPU_FP16_TO_FP32"
886+
#endif
887+
```
888+
889+
**Proven both ways** with clang `-fsyntax-only` against the real b11574 header: it passes on the
890+
fixed tree, fails with the `#error` on the reverted one, does not fire without `__F16C__`, and both
891+
trees compile when the guard is absent -- so the guard is the only thing that changes the outcome.
892+
893+
And one thing to **say** in the same comment, because it is honest and useful rather than an
894+
objection: a clean isolation on Granite Rapids (one tree built twice, upstream's own `llama-bench`,
895+
5 repetitions, two non-F16C control modules) finds **no measurable effect** for Q4_K_M at 8 and 16
896+
threads -- every difference inside ±2 % with random signs. The author's own description (a FreeBSD box
897+
with "clock-mismatched RAM", "gains like these may be greater than reasoned") fits that: the fix
898+
removes a ~256 KB table from the path, which costs on a bandwidth-starved machine and nothing on a
899+
server CPU. **The change stays right; the number is not general.** Offering the measurement with the
900+
PR is the point, not disputing it.
901+
863902
**Two rules for the day.** State the measurement and ask the question; do not propose the fix for 1-3,
864903
because each is a design decision in code this project does not own. And say plainly what was done
865904
downstream instead (the opt-in, the non-aborting registration, the `-w` on `ggml-hip`), so a

‎docs/benchmarks/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ themselves live in [`../../TODO.md`](../../TODO.md).
1515
|---|---|
1616
| [`cpu-backend-modules.md`](cpu-backend-modules.md) | What each of the 22 shipped CPU backend modules is worth, per microarchitecture — and whether `ggml_backend_load_best` picks the fastest one |
1717
| [`gpu-module-jars.md`](gpu-module-jars.md) | What happens when a GPU module jar is actually *used* -- two findings that contradict the "a module that cannot help is skipped" premise |
18+
| [`upstream-f16c-30270.md`](upstream-f16c-30270.md) | An isolation of one upstream commit: what it does, that it changes nothing measurable here, and a compile-time guard proven red against the defect |
1819
| [`native-loading.md`](native-loading.md) | What extracting the natives costs and what the reuse saves, how a running JVM's locked files behave, and which container base images can load the Linux jar |
1920
| [`ec2-harness.md`](ec2-harness.md) | How a measurement is run on rented hardware, and the invariants that came out of getting it wrong |
2021

‎docs/benchmarks/cpu-backend-modules.md‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,88 @@ issue with these tables (`TODO.md`, "Upstream submissions"), and worth knowing f
153153
about generation latency — `net.ladenthin.llama.backend` cannot select a *CPU* module, so the only
154154
lever today is shipping a directory with one module in it.
155155

156+
## Re-measured at b11558 on 64 cores: the baseline moved, and the ranking reverses
157+
158+
Measured **2026-10-12** against the **published** jars (`5.2.0-20261012.112736-13`, llama.cpp
159+
**b11558**) on an `m8i.16xlarge` -- the **same CPU model** as the table above (Xeon 6975P-C, Granite
160+
Rapids) but **64 vCPU instead of 4**. Same method: one module staged at a time, same model, 35 runs in
161+
one boot.
162+
163+
This exists because the section below predicted it: the b11538 tables are relative to `haswell`, and
164+
**upstream #30270 (merged 2026-10-11, in the b11538..b11556 range) makes AVX2+F16C CPUs use the
165+
hardware conversion instead of a lookup table** -- i.e. it moves exactly that baseline.
166+
167+
### All 14 modules at 16 threads
168+
169+
| module | pp512 | tg128 | against `haswell` (pp) |
170+
|---|---:|---:|---:|
171+
| **`sapphirerapids`** (AMX) ← ggml's choice | **2775.5** | **160.0** | **+96 %** |
172+
| `cooperlake` | 1949.1 | 152.5 | +37 % |
173+
| `skylakex` | 1801.4 | 153.0 | +27 % |
174+
| `cannonlake` | 1787.7 | 151.9 | +26 % |
175+
| `icelake` | 1565.7 | 152.9 | +10 % |
176+
| `cascadelake` | 1563.1 | 152.8 | +10 % |
177+
| `zen4` | 1553.2 | 153.9 | +10 % |
178+
| `haswell` | 1417.6 | 151.8 | — |
179+
| `alderlake` | 1410.0 | 150.4 | −1 % |
180+
| `piledriver` | 892.6 | 140.6 | −37 % |
181+
| `ivybridge` | 861.2 | 138.3 | −39 % |
182+
| `sandybridge` | 841.2 | 76.2 | −41 % |
183+
| `sse42` | 68.9 | 36.4 | −95 % |
184+
| `x64` | 63.4 | 33.2 | −96 % |
185+
186+
Full set on the same machine: ggml chose `sapphirerapids`, 2763.4 / 159.6 -- within noise of the
187+
staged run, so the staging still does not distort anything. **All 14 load and run**, as at b11538.
188+
189+
**AMX is worth +96 % on prompt processing here, against +148 % in the 4-vCPU table.** Two things
190+
changed at once (the pin and the machine size), so see the isolation attempt below before reading that
191+
as the effect of #30270 alone.
192+
193+
### The thread sweep: at 64 threads `haswell` beats the module ggml picks
194+
195+
The open question was whether the generation ranking reverses with thread count, as the Windows
196+
OpenMP measurements showed it can. **It does.** Token generation, four modules, same machine:
197+
198+
| threads | 1st | 2nd | 3rd | 4th |
199+
|---:|---|---|---|---|
200+
| 4 | **`sapphirerapids`** 59.7 | `zen4` 55.2 | `cascadelake` 54.1 | `haswell` 53.8 |
201+
| 8 | **`sapphirerapids`** 105.6 | `cascadelake` 102.0 | `zen4` 101.5 | `haswell` 99.4 |
202+
| 16 | **`sapphirerapids`** 159.1 | `zen4` 153.1 | `cascadelake` 151.7 | `haswell` 150.7 |
203+
| 32 | **`sapphirerapids`** 188.2 | `cascadelake` 186.8 | `haswell` 185.1 | `zen4` 183.8 |
204+
| **64** | **`haswell` 192.6** | `zen4` 190.0 | `cascadelake` 188.6 | **`sapphirerapids` 181.8** |
205+
206+
At 64 threads the richest module is **last**, and the one ggml would never pick on this CPU is first.
207+
Prompt processing turns over there too: between 32 and 64 threads `sapphirerapids` falls 4119.7 → 3880.2
208+
and `zen4` collapses 2069.0 → **1143.3**, while `haswell` keeps rising 1752.8 → 1960.3. The machine has
209+
32 physical cores with SMT, so 64 threads means hyperthreads -- which is a realistic default, since
210+
`std::thread::hardware_concurrency()` and most "use all cores" settings report 64 here.
211+
212+
**This sharpens the upstream finding below rather than adding a new one**: `ggml_backend_load_best`
213+
scores by feature level, which costs 10-11 % of generation at 4 vCPU and **loses outright at 64
214+
threads**.
215+
216+
### #30270 is NOT the reason the baseline moved -- measured, and it refutes what this page first said
217+
218+
Same CPU model and the **same thread count** (4), the only pair comparable across the two tables:
219+
220+
| | b11538, 4 vCPU | b11558, 64 vCPU at t=4 | change |
221+
|---|---:|---:|---:|
222+
| `haswell` pp512 | 221.8 | 422.7 | **+91 %** |
223+
| `sapphirerapids` pp512 | 550.0 | 847.9 | +54 % |
224+
| ratio | 2.48x | **2.01x** | |
225+
226+
This page first read that asymmetry as *consistent with #30270*, since `haswell` gained far more than
227+
the AMX module and #30270 is an AVX2-specific fix. **That was wrong, and the clean isolation says so**
228+
([`upstream-f16c-30270.md`](upstream-f16c-30270.md)): one source tree built twice on one machine with
229+
those eight lines removed shows **no measurable effect at all** -- every difference inside ±2 %, with
230+
two non-F16C control modules moving just as much.
231+
232+
So the +91 % is **the instance size**: 4 threads on a 64-vCPU machine have far more memory bandwidth
233+
to themselves than a 4-vCPU machine does, and `haswell` is the more bandwidth-bound of the two. The
234+
asymmetry between two rows of two different machines is not an isolation, and reading it as one was
235+
the mistake. It is kept here rather than quietly removed, because the next person comparing two tables
236+
across a pin **and** a machine size will be tempted the same way.
237+
156238
## These numbers are bound to llama.cpp b11538, and b11556 already moved one of them
157239

158240
Every table above was measured against the natives jars built at **b11538**. That is not boilerplate:

0 commit comments

Comments
 (0)