#163 JIRA-2258 review aid: Tsavorite-only diff vs upstream 1f368f354 (side-by-side)

gemma4-validation-logs.zip
posix-validation-logs.zip
tsisim-validation-logs.zip

This PR diffs against upstream-1f368f354-baseline, a branch pointing at the exact
upstream commit (1f368f354d9edcfea9fd6a1e0989b3e7335a050f) that
consolidated-patch.patch is generated against. Since both branches share that same
upstream tree, this diff shows only the ~92 files this sync actually changes: the
Tsavorite backend (ggml/src/ggml-tsavorite/), build config (tsi-pkg-build.sh,
tsavorite-model-deployment.yaml, CMake wiring), new regression tests, and the
JIRA-2258 validation docs/logs. Companion branch jira-2258-llama-cpp-sync (PR #162)
carries the same content against origin/master directly, which is why that PR shows
~3150 changed files instead — almost all of it is upstream's own already-reviewed
commit history landing via the version bump, not new content.

Why this sync, and why now

origin/master last touched upstream at 51abc96bdc (2025-09-16). This sync's target,
1f368f354 (2026-08-13), is ~11 months of upstream ggml-org/llama.cpp history later.
That gap — not a preference for a different branch or approach — is why this lands as
one large, version-bump-style sync consolidating every Tsavorite-specific change
accumulated since 2025-09-16 into one patch file (consolidated-patch.patch), rather
than as a normal incremental PR. The stated core motivation was Gemma 4 architecture
support, which didn't exist at all on the pre-sync fork — see the Gemma 4 findings in
docs/jira-2258/gemma4-validation-logs/GEMMA4-VALIDATION-SUMMARY.md.

cubic-dev-ai review remediation (2026-08-26)

All 54 cubic-dev-ai comments across two review passes replied to and resolved
(40 in the first pass, 14 more in a second pass triggered by pushing the first round's
fixes — cubic re-reviews on every push).

• ~40 fixed directly: CMake unquoted-STREQUAL/$ENV{} bugs (the kind that break
  the whole cmake configure, not just the guarded branch), missing GGML_API on
  ggml_backend_cleanup(), an inverted log-level filter in ggml_log_internal(), a
  NaN/Inf-vs-tolerance check that silently passed bad kernel output, backend-cleanup
  ordering fixed via a BackendCleanupGuard RAII pattern (tools/cli/cli.cpp,
  tools/completion/completion.cpp, tools/server/server.cpp), the .profile
  backend callback missing its backend parameter, api_version never set for
  non-OLLAMA GGML_BACKEND_DL builds, a duplicate perf-summary print in
  common_perf_print(), a regex in model-rerun.py matching the wrong log line,
  LLAMA_OPENSSL silently defaulting OFF, ggml-tsavorite.h shipping a public header
  that depended on an uninstalled private header, plus several doc fixes (missing
  SDK_VERSION exports, a nonexistent TSI-VERSION setting, missing
  UPSTREAM_BASE_COMMIT from the review-scope doc).
• 1 false positive: a .gitmodules/ggml-tsi-kernel gitlink flagged as
  mismatched — confirmed valid via git ls-tree HEAD, no fix needed.
• 3 real bugs found while verifying the fixes above, not flagged by cubic directly:
  1. llama-cli/llama-server were never built at all under GGML_TSAVORITE=ON,
     in posix or FPGA, since this sync landed — a nesting bug in tools/CMakeLists.txt
     (the old fork built main, cli's pre-refactor name, outside the Tsavorite
     exclusion guard; this sync's port put the new cli/server subdirectories
     inside it by accident). This silently broke model-rerun.py (added in this
     same PR), which calls build-posix/bin/llama-cli. Fixed by moving ui/cli/
     server/mtmd out from under the exclusion, and fixing the FPGA .tz packaging
     step (it only knew to copy llama-completion) to include the newly-buildable
     binaries too.
  2. CMAKE_FIND_ROOT_PATH is a 4-entry CMake list (the ARM toolchain's own sysroot
     search path), not a scalar. The first attempt at a cubic-suggested rpath-link fix
     used it as one anyway, which corrupted the FPGA link
     (ld: ... Is a directory) — caught by actually rebuilding, not just trusting a
     green cmake configure. Fixed by moving the rpath-link flags to a single
     add_link_options() call in the top-level CMakeLists.txt, so it applies to
     every target via tsi-pkg-build.sh or the manual cmake -B build-fpga ...
     flow documented in the README/docs/build.md.
  3. llama-cli hung forever on any non-interactive invocation (piped/redirected
     output, no controlling TTY — exactly how model-rerun.py, added in this same PR,
     invokes it via subprocess.check_output()). cli-context.cpp's main loop, once
     the initial -p prompt is consumed, calls read_input() for the next turn and
     treats an empty returned buffer as "blank line, keep waiting" — correct for a real
     interactive TTY session, but on a non-TTY stdin getwchar() hits EOF instantly and
     forever, so the loop spins at ~100% CPU rewriting the > prompt indefinitely,
     never terminating. This is pure upstream code (cli-context.cpp isn't part of the
     Tsavorite patch) that was simply never exercised before tonight, since llama-cli
     was never buildable under Tsavorite until bug 1 above was fixed. Caught by actually
     running the freshly-built binary non-interactively, not just confirming it linked.
     Fixed by checking feof(stdin) before deciding to continue vs. break out of the
     loop — distinguishes genuine end-of-input from an interactively-submitted blank
     line, which were previously indistinguishable from the loop's point of view.
• A separate, more serious bug caught in the second review pass: an earlier fix
  had added a global set -e to tsi-pkg-build.sh to make a narrow error-propagation
  gap more reliable. cubic correctly flagged that as worse than the problem it solved —
  verified empirically: sourcing this script with set -e active and hitting any
  failing command (not just inside die()) terminates the calling shell, not just
  this script, and skips the cleanup trap entirely. Reverted; the narrower gap it was
  trying to close is left as a documented follow-up instead.
• 3 real findings deliberately left unfixed (real, but risky enough that a
  drive-by fix without dedicated regression testing would be reckless — flagged in the
  GitHub replies for a deliberate follow-up decision, not silently dropped):
  • ggml_backend_i gained new callbacks (PR #158, already merged, ollama's
    split-binary boundary depends on it) without bumping GGML_BACKEND_API_VERSION
    (still 2) — a real ABI risk for any old dynamically-loaded backend .so.
  • ggml-backend.cpp's view-op scheduling forcing both a view node and its source
    onto the CPU backend is not a new bug — it's the deliberate fix from #120 (the
    revert of the graph-split optimization that crashed Qwen and hid OPU usage, #118).
  • struct ggml_tensor's size/layout changing under GGML_PERF/GGML_PERF_RELEASE/
    GGML_PERF_DETAIL (also from #158) is safe inside this repo's own build, but a
    real hazard for external consumers of the installed public ggml.h +
    libggml.so that don't define the same flag.

Full rebuild verification after every round: posix + FPGA both clean (0 errors),
llama-cli/llama-server/llama-completion all present and packaged into
tsi-ggml-0.4.24.tz.

Posix re-validation (2026-08-26, post-remediation)

None of the fixes above touch compute kernels, but re-ran the full model set anyway
since real bugs were found and fixed along the way:

Model	Result	Duration	Generated text
tinyllama-vo-5m-para.gguf	✅ 0 warnings	5s	My cat's name is Tim. He
Gemma3-270M-F32.gguf	✅ 0 warnings	33s	My cat's name is "Cat"
Tiny-Llama-v0.3-FP32-1.1B-F32.gguf	✅ 0 warnings	3m55s	My cat's name is Luna.
Llama3.2:1B-1.2B-F32.gguf	✅ 0 warnings	3m59s	My cat's name is "Bella"
qwen2-0_5b-instruct-q5_k_m.gguf	✅ 0 warnings	1m30s	My cat's name is Muffy. She
Qwen2.5-0.5B-Q4_K_M.gguf	✅ 0 warnings	1m25s	My cat's name is Zane. He
Gemma4-e2b-5.1B-Q4_K_M.gguf	⚠️ known upstream bug, not a regression -- fails to load (wrong number of tensors), independently confirmed against ggml-org/llama.cpp#27349	4s	(fails to load)
Gemma4-12b-Q4_K_M.gguf	✅ 0 warnings	29m18s	My cat's name is **"Luna0 (matches the already-documented Tsavorite-vs-CPU formatting divergence, not corruption)

7/8 models pass cleanly with zero warnings; the 8th (Gemma4-e2b) fails identically to before this PR's changes, a pre-existing upstream loading bug unrelated to the Tsavorite backend (see the Gemma 4 investigation this PR already contains for detail).

Raw logs gzipped and attached to this PR description rather than committed to the
repo tree. llama-cli smoke-tested end to end for the first time (it was never
buildable under Tsavorite before this fix): clean run, user_dram_size_gb=16 correctly read, generated "I'm, exited cleanly in 6s (previously spun forever on any non-interactive invocation -- see the cli-context.cpp fix above).

Review this PR's file-by-file diff for the actual code review of the ~92 files that
matter. Once approved here, the same content lands via #162.

---

Summary by cubic

Review-only branch for JIRA-2258 that diffs against the exact upstream baseline consolidated-patch.patch is generated from, exposing only the ~93 files this sync actually changes instead of the ~3150-file diff against master. Do not merge this branch; the real merge is jira-2258-llama-cpp-sync.

What changed

• Carries the full Tsavorite delta as consolidated-patch.patch, with UPSTREAM_BASE_COMMIT pinning its regeneration base.
• Adds the backend (ggml/src/ggml-tsavorite/), build tooling (tsi-pkg-build.sh, tsavorite-model-deployment.yaml, CMake wiring), repro tools under examples/simple/, and a model-rerun.py validation harness.
• Fixes llama-cli and llama-server building under GGML_TSAVORITE (they previously never built).
• Makes FPGA rpath-link a single global add_link_options() call and routes TOOLBOX_DIR overrides (normalized to an absolute path) through one TSAVORITE_TOOLBOX_ROOT for both include and link roots.
• Removes set -e from tsi-pkg-build.sh; sourcing it could kill the caller's shell.
• Fixes llama-cli's 100% CPU spin on piped, non-TTY input by tracking EOF separately from blank lines via a bool* eof out-param; the loop breaks on genuine EOF (getline failure/Ctrl+D/WEOF), and the earlier isatty() heuristic that exited early on embedded blank lines is removed.
• Makes ggml_backend_cleanup() explicit (void) to fix an upstream -Werror=strict-prototypes CI failure.
• Fixes validation findings: non-128-byte-aligned broadcast chunking, 2D scalar broadcast, and unaligned-memory warnings.
• Adds GGML_PERF-guarded op profiling with a dedicated Tsavorite summary, a GGML_LOG_LEVEL_TSAVORITE log level, and backend profile/cleanup hooks.
• Skips upstream examples and tests that don't build on Tsavorite via GGML_TSAVORITE CMake guards.
• Raises the user_dram_size_gb deployment default from 8 to 16.
• Keeps posix, gemma4, and tsisim validation summaries under docs/jira-2258/: 4/4 posix models are byte-identical old vs new and Gemma 4 12b loads; raw per-run logs are attached to this PR instead of committed.