From 0ca34d4892b2e3e681d69261255c05dcfa45fe45 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 10:02:41 -0400 Subject: [PATCH 1/3] submodule_pin: a submodule at its pin with a dirty work tree passes as "at their pinned commit" engine#2129 is already fixed on main by tools/ci/submodule_pin_check.py -- another lane's, registered in cmake/tests-submodule-pin.cmake as a ctest row -- and it reads `git submodule status`, which reports four states. This adds the fifth, which that command cannot report, and the wiring that runs the check before a build instead of only in CI. **The missing state: at the pin, not clean.** `git submodule status` prints ' ' for "the checkout IS the pinned commit" whatever the submodule's own work tree holds, and the build compiles that work tree. Measured on the box that motivated #2129: on top of the checkouts being off-pin, `third_party/llama.cpp` had 301 uncommitted paths, `xdna-driver` 32, `tokenizers` 20, and four others 4 each. Every one of those would read as "at their pinned commit" to the existing check. Reported always, with its count, and a failure only under the new `--require-clean`: a deliberate local patch in a submodule is a real thing a box does, and moving the pin is not always the caller's to do, so the default is to say it rather than to stop. Five self-test arms, including that a dirty '+' row is not double-counted (the commit is the failure there). **The wiring: nothing ran it before a build.** A ctest row runs in CI; the drift was measured on the box, where the release and the weekly build actually run. `scripts/weekly.sh`'s preflight now runs the check for the `bumps` and `build` stages through `preflight_submodules()` (a function of its own, testable against stub checkers like `preflight_runner_units`): exit 1 **blocks** the run -- the build would not be of the pinned sources -- and exit 2 (no submodule initialised, i.e. a fresh clone) only **warns**, saying that the check inspected nothing rather than implying a pass. tools/ci/submodule_pin_check.py --self-test 19 arms, 5 of them new tests/weekly_submodule_preflight_test.sh 6/6, the function cut out of weekly.sh, stub checkers tools/copyright.py --check clean tools/ci/tests_referenced_check.py 83/83 referenced `--require-clean` is deliberately not passed in the preflight: a dirty submodule is reported there and does not stop a build, while a wrong commit does. This supersedes #2177, which grew a parallel `scripts/check-submodules.sh` before main's tool landed. Two checks for one property is the failure mode the standing rules name, so that script is withdrawn rather than maintained beside it. --- cmake/tests-submodule-pin.cmake | 5 ++ scripts/weekly.sh | 28 ++++++++- tests/weekly_submodule_preflight_test.sh | 76 ++++++++++++++++++++++++ tools/ci/submodule_pin_check.py | 74 +++++++++++++++++++++-- 4 files changed, 177 insertions(+), 6 deletions(-) create mode 100755 tests/weekly_submodule_preflight_test.sh diff --git a/cmake/tests-submodule-pin.cmake b/cmake/tests-submodule-pin.cmake index 3a00b8433..f2c4cdbf0 100644 --- a/cmake/tests-submodule-pin.cmake +++ b/cmake/tests-submodule-pin.cmake @@ -25,3 +25,8 @@ # check's business); `--require-initialised` is the release-gate form, where a missing checkout is a failure too. add_python_test(submodule_pin_check ${CMAKE_SOURCE_DIR}/tools/ci/submodule_pin_check.py --check --root ${CMAKE_SOURCE_DIR}) add_python_test(submodule_pin_check_self ${CMAKE_SOURCE_DIR}/tools/ci/submodule_pin_check.py --self-test) + +# engine#2129: the release preflight's use of the check -- a build tree whose submodules are not the pinned commits +# BLOCKS the run, and "no checkout to compare" only WARNs so a fresh clone can still build. The function is cut out of +# scripts/weekly.sh and run against stub checkers: no submodules, no network, no build. +add_test(NAME weekly_submodule_preflight COMMAND ${CMAKE_SOURCE_DIR}/tests/weekly_submodule_preflight_test.sh) diff --git a/scripts/weekly.sh b/scripts/weekly.sh index d8ce116fc..4821a8336 100755 --- a/scripts/weekly.sh +++ b/scripts/weekly.sh @@ -658,6 +658,30 @@ preflight_runner_units() { return 0 } +# The submodule row of the preflight (engine#2129). The bumps and build stages build the tree in front of them, and a +# build compiles whatever the submodules' work trees hold: tools/ci/submodule_pin_check.py is the check for whether those +# checkouts ARE the commits this tree pins. It already exists as a ctest row, and a ctest row runs in CI -- nothing ran it +# before a build on the box, which is where the drift was measured (7 of 7 submodules off-pin). Exit 1 stops the run: the +# build would not be of the pinned sources. Exit 2 (nothing to compare -- no submodules initialised) only WARNS, because a +# fresh clone must still be able to build and the warning says the check inspected nothing rather than implying a pass. +# --require-clean is deliberately NOT passed here: it fails on a dirty submodule work tree, and a deliberate local patch +# is a real thing a box does; the check reports those with their counts either way. +# A function of its own so tests/weekly_submodule_preflight_test.sh can run it against a stub. +preflight_submodules() { + local check=$1 src=$2 out rc=0 + [ -f "$check" ] || return 0 + out=$(python3 "$check" --check --root "$src" 2>&1) || rc=$? + case "$rc" in + 0) printf '%s\n' "$out" | grep -v 'at their pinned commit' | sed 's/^/preflight: /' ;; + 2) printf '%s\n' "$out" | sed 's/^/preflight: /' + echo "preflight: WARNING, not blocking: no submodule checkout to compare against the pin, so this build follows whatever the work trees hold (tools/ci/submodule_pin_check.py exit 2; git submodule update --init fills it)" ;; + *) printf '%s\n' "$out" | sed 's/^/preflight: /' + echo "preflight: a submodule is not at the commit this tree pins, so the build would not be of the pinned sources (tools/ci/submodule_pin_check.py exit $rc; the line above names the submodule and what to do)" + return 1 ;; + esac + return 0 +} + # --- preflight ---------------------------------------------------------------------------- # What the stages need from the machine, checked before the first one runs, so that a missing # tool stops the run at the start rather than after an hour of build (2026-10-19 rehearsal). @@ -677,7 +701,9 @@ preflight() { 2> "$LOGS/hrx-toolchain.log"); then :; else echo "preflight: cannot fetch the pinned HRX toolchain (config/hrx-toolchain.config): see $LOGS/hrx-toolchain.log"; bad=1 fi ;; - esac ;; + esac + # engine#2129: this stage builds the tree in front of it, so the submodules have to BE the pinned tree + preflight_submodules "$SRC/tools/ci/submodule_pin_check.py" "$SRC" || bad=1 ;; package) # scripts/package-linux.sh rewrites RPATHs with patchelf command -v patchelf > /dev/null || diff --git a/tests/weekly_submodule_preflight_test.sh b/tests/weekly_submodule_preflight_test.sh new file mode 100755 index 000000000..35878dc28 --- /dev/null +++ b/tests/weekly_submodule_preflight_test.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# Copyright 2026 bong-water-water-bong +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# weekly_submodule_preflight_test.sh: the release preflight BLOCKS when a submodule is not the commit the tree pins and +# only WARNS when there is no checkout to compare against (engine#2129). The function is cut out of scripts/weekly.sh and +# run against stub checkers, so no submodules, no network and no build. +set -u +root=$(cd "$(dirname "$0")/.." && pwd) +tmp=$(mktemp -d); trap 'rm -rf "$tmp"' EXIT +fail=0 +ok() { printf 'ok %s\n' "$1"; } +bad() { printf 'FAIL %s\n' "$1"; fail=1; } + +sed -n '/^preflight_submodules() {$/,/^}$/p' "$root/scripts/weekly.sh" > "$tmp/fn.sh" +grep -q 'submodule_pin_check.py exit' "$tmp/fn.sh" || { bad "could not cut preflight_submodules() out of scripts/weekly.sh"; echo "weekly_submodule_preflight: FAIL"; exit 1; } +grep -q 'preflight_submodules "' "$root/scripts/weekly.sh" || bad "scripts/weekly.sh's preflight no longer calls preflight_submodules" +say() { echo "$*"; } +# shellcheck disable=SC1091 +. "$tmp/fn.sh" + +# A stub standing in for tools/ci/submodule_pin_check.py. It is a Python file because the function runs the real tool as +# `python3 `: printing what the tool prints and exiting with the code under test is the whole stub. +stub() { # + { + printf 'import sys\nprint(' + printf '%s' "$2" | sed 's/.*/"&"/' + printf ')\nsys.exit(%s)\n' "$1" + } > "$tmp/stub.py" + echo "$tmp/stub.py" +} +row() { # + local name=$1 want=$2 text=$3 rc=$4 line=$5 + local check out got=0 + check=$(stub "$rc" "$line") + out=$(preflight_submodules "$check" "$tmp" 2>&1) || got=$? + if [ "$got" = "$want" ] && printf '%s\n' "$out" | grep -q -- "$text"; then + ok "preflight: $name -> return $got" + else + bad "preflight: $name: want return $want with '$text', got $got: $out" + fi +} + +# A passing run prints nothing at all: the tool's summary line ("N submodule(s) at their pinned commit") is the one +# thing the function drops, because a preflight that always prints a line trains its reader to skip the lines. +check=$(stub 0 "submodule_pin: 7 submodule(s) at their pinned commit") +out=$(preflight_submodules "$check" "$tmp" 2>&1) +if [ -z "$out" ]; then ok "a tree whose submodules are their pins passes silently"; else bad "a passing run should print nothing, got: $out"; fi +row "a submodule at a different commit BLOCKS the run" 1 "not at the commit this tree pins" 1 "submodule_pin: third_party/x is checked out at abc, which is NOT the commit this tree pins" +row "no checkout to compare only WARNS" 0 "WARNING, not blocking" 2 "submodule_pin: no submodules reported - cannot tell, and that is not a pass" + +# A dirty submodule at its pin is reported and does NOT block: a deliberate local patch is a real thing a box does. +row "a dirty submodule at its pin is reported, not blocking" 0 "changed path" 0 "submodule_pin: third_party/x is at the pinned abc but its work tree has 301 changed path(s)" + +# A checker that is not there at all (an older tree): the row is skipped, not failed. +preflight_submodules "$tmp/no-such-check.py" "$tmp" >/dev/null 2>&1 && ok "a missing checker does not stop the run" \ + || bad "a missing checker should not stop the run" + +# The asymmetry, in one assertion: exit 2 must not block, exit 1 must. +check=$(stub 2 "submodule_pin: cannot tell") +if preflight_submodules "$check" "$tmp" >/dev/null 2>&1; then ok "exit 2 is not a block"; else bad "exit 2 should not block the release"; fi + +if [ "$fail" != 0 ]; then echo "weekly_submodule_preflight: FAIL"; exit 1; fi +echo "weekly_submodule_preflight: ok" diff --git a/tools/ci/submodule_pin_check.py b/tools/ci/submodule_pin_check.py index 9e3e45600..e36be0cdf 100644 --- a/tools/ci/submodule_pin_check.py +++ b/tools/ci/submodule_pin_check.py @@ -33,8 +33,13 @@ missing the source or using a system copy) -> fail with --require-initialised 'U' merge conflicts in the submodule -> fail +`git submodule status` cannot report one more state a build cares about: a submodule that IS at the pinned commit and +has uncommitted changes in its own work tree. It prints ' ' for that, and a build compiles the uncommitted files. On +2026-11-11 the same box that motivated this check had 301 such files in `third_party/llama.cpp`. Those are reported +always, and --require-clean turns them into a failure. + Usage: - tools/ci/submodule_pin_check.py --check [--root .] [--require-initialised] + tools/ci/submodule_pin_check.py --check [--root .] [--require-initialised] [--require-clean] tools/ci/submodule_pin_check.py --self-test """ from __future__ import annotations @@ -65,14 +70,30 @@ def parse_status(text: str) -> list[tuple[str, str, str, str]]: return rows -def verdict(rows: list[tuple[str, str, str, str]], require_initialised: bool) -> tuple[int, list[str]]: - """(rc, messages). rc 0 pass, 1 a submodule is not at its pin, 2 nothing readable.""" +def verdict(rows: list[tuple[str, str, str, str]], require_initialised: bool, + dirty: dict[str, int] | None = None, require_clean: bool = False) -> tuple[int, list[str]]: + """(rc, messages). rc 0 pass, 1 a submodule is not at its pin, 2 nothing readable. + + `dirty` maps a path to the number of paths its work tree has changed (git status --porcelain). A submodule at its + pin with a dirty work tree compiles code the pin does not name; it is always reported and fails only under + --require-clean, because a deliberate local patch is a real thing a box does and the fix for it (move the pin) is + not always the caller's to make.""" if not rows: return 2, ["submodule_pin: no submodules reported - cannot tell, and that is not a pass"] out: list[str] = [] - bad = unknown = uninit = 0 + bad = unknown = uninit = dirty_at_pin = 0 for flag, sha, path, describe in rows: if flag == " ": + n = (dirty or {}).get(path, 0) + if n: + dirty_at_pin += 1 + out.append( + f"submodule_pin: {path} is at the pinned {sha[:12]} but its work tree has {n} changed " + f"path(s), so the build compiles code the pin does not name. Repair: git -C {path} stash, or " + f"move the pin if the change is meant to stay (git add {path})" + ) + if require_clean: + bad += 1 continue if flag == "?": unknown += 1 @@ -102,6 +123,8 @@ def verdict(rows: list[tuple[str, str, str, str]], require_initialised: bool) -> return 1, out if unknown and not (len(rows) - unknown): return 2, out + if dirty_at_pin and not require_clean: + out.append(f"submodule_pin: WARNING, not a failure: {dirty_at_pin} submodule(s) at their pin with a dirty work tree (--require-clean makes this an error)") out.append( f"submodule_pin: {len(rows) - bad - unknown - uninit} submodule(s) at their pinned commit" + (f", {uninit} not initialised" if uninit else "") @@ -123,6 +146,28 @@ def run_git(root: pathlib.Path) -> tuple[str, int]: return p.stdout, 0 +def run_dirty(root: pathlib.Path, rows: list[tuple[str, str, str, str]]) -> dict[str, int]: + """{path: changed-path count} for the submodules that ARE at their pin. + + Only those: a '+' or 'U' row already fails and its dirty count would be noise. `git status --porcelain` inside the + submodule is the only thing that reports this state - `git submodule status` prints ' ' for it. + """ + out: dict[str, int] = {} + for flag, _sha, path, _describe in rows: + if flag != " ": + continue + try: + p = subprocess.run(["git", "-C", str(root / path), "status", "--porcelain"], + capture_output=True, text=True, check=False) + except OSError: + continue + if p.returncode == 0: + n = len([l for l in p.stdout.splitlines() if l.strip()]) + if n: + out[path] = n + return out + + def main(argv: list[str]) -> int: ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) g = ap.add_mutually_exclusive_group(required=True) @@ -134,6 +179,11 @@ def main(argv: list[str]) -> int: action="store_true", help="treat a submodule that is not checked out as a failure (a release build should use this)", ) + ap.add_argument( + "--require-clean", + action="store_true", + help="treat a submodule that is at its pin with a dirty work tree as a failure (CI uses this)", + ) args = ap.parse_args(argv) if args.self_test: @@ -143,7 +193,9 @@ def main(argv: list[str]) -> int: if rc != 0: print(f"submodule_pin: cannot tell: {text}") return 2 - rc, messages = verdict(parse_status(text), args.require_initialised) + rows = parse_status(text) + rc, messages = verdict(rows, args.require_initialised, run_dirty(pathlib.Path(args.root).resolve(), rows), + args.require_clean) for m in messages: print(m) return rc @@ -189,6 +241,18 @@ def check(ok: bool, what: str) -> None: any("not initialised" in m for m in verdict(parse_status(uninit), True)[1]), "... and so does the uninitialised one", ) + # engine#2129, the half `git submodule status` cannot report: at the pin, dirty work tree + check(verdict(parse_status(at_pin), False, {"third_party/llama.cpp": 301})[0] == 0, + "at the pin with 301 dirty paths -> pass by default (a deliberate local patch is a real thing)") + check(any("301 changed" in m for m in verdict(parse_status(at_pin), False, {"third_party/llama.cpp": 301})[1]), + "... but it IS reported, with its count") + check(verdict(parse_status(at_pin), False, {"third_party/llama.cpp": 301}, True)[0] == 1, + "... and FAILS with --require-clean") + check(verdict(parse_status(at_pin), False, {}, True)[0] == 0, + "a clean submodule at its pin still passes with --require-clean") + check(not any("changed" in m for m in verdict(parse_status(ahead), False, {"third_party/llama.cpp": 5})[1]), + "a dirty '+' row is not double-counted (the commit is the failure there)") + # one bad among several still fails: the check is not 'all lines' or 'the first line' check(verdict(parse_status(at_pin + ahead + at_pin), False)[0] == 1, "one '+' among three lines still fails") From fd40148027245d10c39b767cd4a9295eeb6143ed Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 10:31:48 -0400 Subject: [PATCH 2/3] weekly: the submodule gate runs in build(), not preflight() -- where it was a no-op that reported success Found by checking my own wiring against where the variables are set rather than against whether the tests pass. `SRC=$W/src` is created by `checkout`, which the `bumps` and `build` stages call, and `preflight()` runs before any stage -- so `[ -f "$SRC/tools/ci/submodule_pin_check.py" ]` was false on every real run, the function returned 0, and the gate inspected nothing while reporting success. A preflight row that cannot see the tree it is meant to inspect is worse than no row: it reads as verified. The gate now runs at the top of `build()`, immediately after `checkout` has updated `$SRC`'s submodules and before the first compile -- the tree that is about to be compiled, stopped before the compile rather than after it. `build()` returning non-zero fails the stage. And the missing-checker path now says so. An older ref does not carry the tool, so that build's submodule drift is not checked; it prints a WARNING naming the ref's reason instead of returning 0 silently. That was the same defect one level up: the function's first version treated "no file to run" as "fine", which is the reporting shape this whole check exists to remove. tests/weekly_submodule_preflight_test.sh 6/6, including that a missing checker warns rather than passing quietly tools/ci/submodule_pin_check.py --self-test 19 arms tools/copyright.py --check clean --- scripts/weekly.sh | 16 +++++++++++----- tests/weekly_submodule_preflight_test.sh | 6 +++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/scripts/weekly.sh b/scripts/weekly.sh index 4821a8336..2a0045f70 100755 --- a/scripts/weekly.sh +++ b/scripts/weekly.sh @@ -118,6 +118,11 @@ host_desc() { # --- build / test ------------------------------------------------------------------------- build() { + # engine#2129: what this builds is $SRC, whose submodules `checkout` has just updated, and a build compiles whatever + # those work trees hold. This cannot live in preflight(): preflight runs before $SRC exists, so the check would find + # no file to run, skip, and report success -- a check that inspects nothing and says it passed. Here it is the tree + # that is about to be compiled, and the run stops before the first compile instead of after it. + preflight_submodules "$SRC/tools/ci/submodule_pin_check.py" "$SRC" || return 1 # the private components compiled in (docs/releases.md, "Private components"): the NPU runtime # (into 1bit) and the HIP kernels (into the HRX llama.cpp), each a checkout at the commit its # config/*.config pins; the build stops here when one cannot be fetched at that commit @@ -658,7 +663,8 @@ preflight_runner_units() { return 0 } -# The submodule row of the preflight (engine#2129). The bumps and build stages build the tree in front of them, and a +# The submodule row of the build (engine#2129). It runs at the top of build(), not in preflight(): the tree to inspect is +# $SRC, which `checkout` creates, and preflight runs before that. A # build compiles whatever the submodules' work trees hold: tools/ci/submodule_pin_check.py is the check for whether those # checkouts ARE the commits this tree pins. It already exists as a ctest row, and a ctest row runs in CI -- nothing ran it # before a build on the box, which is where the drift was measured (7 of 7 submodules off-pin). Exit 1 stops the run: the @@ -669,7 +675,9 @@ preflight_runner_units() { # A function of its own so tests/weekly_submodule_preflight_test.sh can run it against a stub. preflight_submodules() { local check=$1 src=$2 out rc=0 - [ -f "$check" ] || return 0 + # An older ref does not carry the tool. That is not a failure, but it is not a pass either: say that the tree was not + # inspected, because a silent 0 here is a check that reports success for doing nothing. + [ -f "$check" ] || { echo "preflight: WARNING, not blocking: $src has no tools/ci/submodule_pin_check.py (a ref older than engine#2129), so submodule drift is not checked for this build"; return 0; } out=$(python3 "$check" --check --root "$src" 2>&1) || rc=$? case "$rc" in 0) printf '%s\n' "$out" | grep -v 'at their pinned commit' | sed 's/^/preflight: /' ;; @@ -701,9 +709,7 @@ preflight() { 2> "$LOGS/hrx-toolchain.log"); then :; else echo "preflight: cannot fetch the pinned HRX toolchain (config/hrx-toolchain.config): see $LOGS/hrx-toolchain.log"; bad=1 fi ;; - esac - # engine#2129: this stage builds the tree in front of it, so the submodules have to BE the pinned tree - preflight_submodules "$SRC/tools/ci/submodule_pin_check.py" "$SRC" || bad=1 ;; + esac ;; package) # scripts/package-linux.sh rewrites RPATHs with patchelf command -v patchelf > /dev/null || diff --git a/tests/weekly_submodule_preflight_test.sh b/tests/weekly_submodule_preflight_test.sh index 35878dc28..0e28554c6 100755 --- a/tests/weekly_submodule_preflight_test.sh +++ b/tests/weekly_submodule_preflight_test.sh @@ -64,9 +64,9 @@ row "no checkout to compare only WARNS" 0 "WARNING, not blocking" 2 "submodule_p # A dirty submodule at its pin is reported and does NOT block: a deliberate local patch is a real thing a box does. row "a dirty submodule at its pin is reported, not blocking" 0 "changed path" 0 "submodule_pin: third_party/x is at the pinned abc but its work tree has 301 changed path(s)" -# A checker that is not there at all (an older tree): the row is skipped, not failed. -preflight_submodules "$tmp/no-such-check.py" "$tmp" >/dev/null 2>&1 && ok "a missing checker does not stop the run" \ - || bad "a missing checker should not stop the run" +# A checker that is not there at all (an older ref): not a failure, but it must SAY that nothing was inspected -- +# a silent pass here is a check that reports success for doing nothing, which is what its first version did. +out=$(preflight_submodules "$tmp/no-such-check.py" "$tmp" 2>&1) && printf '%s' "$out" | grep -q 'is not checked for this build' && ok "a missing checker warns and does not stop the run" || bad "a missing checker should warn (and not stop the run): $out" # The asymmetry, in one assertion: exit 2 must not block, exit 1 must. check=$(stub 2 "submodule_pin: cannot tell") From c6ab8d81f896d005140119e72e8b780fefd4de77 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 12:22:34 -0400 Subject: [PATCH 3/3] hrx_f16_narrowing: match the type pair, so the vector form is visible and an f32 truncation stops being listed (engine#2219) The ratchet matched the substrings `f32 to f16` or `truncf`, which sees the scalar narrowing and misses the vector one: %x = scalar.fptrunc %y : f32 to f16 matched %h = vector.fptrunc %v : vector<4xf32> to vector<4xf16> not matched Measured on the corpus at the pin 2b7960bae (git archive of the submodule commit, then --root): 233 fptrunc lines = 101 scalar + 132 vector bodies, old matcher = 25 bodies, type-pair = 51 (27 more, the gap the issue describes) The `truncf` alternative is dropped, and that is a correction rather than a tidy-up: it was the only thing matching loom-libs/motifs/unary_f32_apply.loom, whose narrowing is spelled %a = scalar.truncf %value : f32 %b = vector.truncf %values : vector<4xf32> -- a truncation that stays inside f32 and names no f16 type at all, listed as a narrowing to f16 for as long as this ratchet has existed. No body in this corpus spells an f16 narrowing as a `truncf` the two type patterns miss (checked), so dropping the alternative removes exactly that line and nothing else. That is why the count goes 25 -> 51 and not 25 -> 52: the issue's 52 is the old matcher's output, one of which was never narrowing to f16. Both arms are intact and the baseline was written by --write against the pin, not by hand: `--check` reports "51 narrowing body(ies), none new, none stale; 10 of them carry a guard marker". Against the old matcher the same baseline fails with "FAIL (1 new, 27 stale of 51)" -- the 27 the issue found, and the false positive as the 1 new. Three self-test arms pin the distinction: the vector form is a narrowing, a truncf naming no f16 type is not, and the two are decided by the type pair rather than by which opcode is spelled. The header comment in tools/ci/hrx_f16_narrowing.txt is unchanged; the entries are regenerated. --- tools/ci/hrx_f16_narrowing.py | 27 +++++++++++++++++++++++++-- tools/ci/hrx_f16_narrowing.txt | 28 +++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/tools/ci/hrx_f16_narrowing.py b/tools/ci/hrx_f16_narrowing.py index 4d4034a0f..7a39659fd 100755 --- a/tools/ci/hrx_f16_narrowing.py +++ b/tools/ci/hrx_f16_narrowing.py @@ -41,11 +41,24 @@ """ import argparse import pathlib +import re import sys CORPUS = pathlib.Path("third_party/llama.cpp/ggml/src/ggml-hrx/kernel-corpus/kernels") BASELINE = pathlib.Path("tools/ci/hrx_f16_narrowing.txt") -NARROW = ("f32 to f16", "truncf") +# The narrowing this lists is the *type pair*, not the opcode spelling: the corpus writes the scalar form as +# `%x = scalar.fptrunc %y : f32 to f16` and the vector form as +# `%h = vector.fptrunc %v : vector<4xf32> to vector<4xf16>`, and a substring test on `f32 to f16` saw only the first +# (engine#2219). Measured at the pin 2b7960bae: 233 `fptrunc` lines, 101 scalar and 132 vector, over 27 more bodies than +# the old rule reached. +# +# The bare `truncf` alternative it used to carry is GONE, and that is a correction rather than a simplification: it +# matched `loom-libs/motifs/unary_f32_apply.loom`, whose narrowing spells out `scalar.truncf %value : f32` and +# `vector.truncf %values : vector<4xf32>` and names no f16 type at all -- a truncation *within* f32, listed as a +# narrowing to f16 for as long as the ratchet has existed. No body in this corpus spells an f16 narrowing as a +# `truncf` that these two patterns miss (checked), so dropping it removes exactly that line and nothing else: 25 -> 51, +# with the 52nd body in the old count being the one that was never narrowing to f16. +NARROW = (re.compile(r"f32 to f16"), re.compile(r"vector<\d+xf32> to vector<\d+xf16>")) GUARD = ("scale_log2", "prescale") @@ -59,7 +72,7 @@ def narrowing_bodies(corpus): found = set() for path in corpus.rglob("*.loom"): body = path.read_text(encoding="utf-8", errors="replace") - if any(marker in body for marker in NARROW): + if any(m.search(body) if hasattr(m, "search") else m in body for m in NARROW): found.add(path.relative_to(corpus).as_posix()) return found @@ -144,6 +157,16 @@ def check_that(ok, what): new, stale = drift(baseline_paths((root / BASELINE).read_text()), narrowing_bodies(corpus)) check_that(stale == ["a.loom"], "a rename leaves a stale baseline line (the engine#1728 lesson)") + # engine#2219: the vector form narrows too, and a truncation that stays inside f32 does not. + (corpus / "v.loom").write_text(" %h = vector.fptrunc %v : vector<4xf32> to vector<4xf16>\n") + check_that(narrowing_bodies(corpus) >= {"v.loom"}, "the vector form is a narrowing (it was invisible before #2219)") + (corpus / "t.loom").write_text( + " %a = scalar.truncf %value : f32\n %b = vector.truncf %values : vector<4xf32>\n") + check_that("t.loom" not in narrowing_bodies(corpus), + "a truncf that names no f16 type is NOT a narrowing (the body the old `truncf` alternative listed)") + check_that("v.loom" in narrowing_bodies(corpus) and "t.loom" not in narrowing_bodies(corpus), + "... and the two are decided by the type pair, not by which opcode is spelled") + (corpus / "v.loom").unlink(); (corpus / "t.loom").unlink() (corpus / "renamed.loom").write_text(" %x = scalar.fptrunc %y : f32 to f16 # scale_log2\n") check_that(guarded({"renamed.loom"}, corpus) == {"renamed.loom"}, "a guard marker is reported, not denied") diff --git a/tools/ci/hrx_f16_narrowing.txt b/tools/ci/hrx_f16_narrowing.txt index b7e67e613..d52512a5c 100644 --- a/tools/ci/hrx_f16_narrowing.txt +++ b/tools/ci/hrx_f16_narrowing.txt @@ -21,21 +21,46 @@ # perfectly correct. What the list buys is that a NEW narrowing body cannot appear without someone # writing it down and saying what feeds it -- and that is the open question on engine#1962: which of # these are protected by a producer-side prescale cannot be answered by reading them. +loom-libs/motifs/bf16_f16.loom +loom-libs/motifs/dequant.loom +loom-libs/motifs/dequant_1bit.loom +loom-libs/motifs/dequant_prism.loom +loom-libs/motifs/f32_f16.loom +loom-libs/motifs/llm_attention_qkv_matmul_postops.loom +loom-libs/motifs/mul_mat_f32_f32_wmma_core.loom loom-libs/motifs/mul_mat_id_f16_f16_wmma_core.loom +loom-libs/motifs/mul_mat_id_f32_f32_wmma_core.loom loom-libs/motifs/mul_mat_id_q4k_f16_wmma_projection.loom loom-libs/motifs/mul_mat_id_q5k_f16_wmma_projection.loom +loom-libs/motifs/mul_mat_id_q6k_f16_wmma_projection.loom +loom-libs/motifs/mul_mat_id_swiglu_f16_f16_accumulate.loom +loom-libs/motifs/mul_mat_id_swiglu_f16_f16_wmma_body.loom +loom-libs/motifs/mul_mat_quantized_f16_prefill.loom +loom-libs/motifs/publish_f32.loom loom-libs/motifs/q4_k_f16.loom +loom-libs/motifs/q6_k_f16.loom +loom-libs/motifs/q8_0_f16.loom +loom-libs/motifs/q8_1_f16.loom loom-libs/motifs/quantize_q8_1_x4.loom -loom-libs/motifs/unary_f32_apply.loom +loom-libs/ops/copy_f32.loom loom-libs/ops/flash_attention_decode_split_f32_f16_wmma.loom loom-libs/ops/flash_attention_f32_f16_wmma.loom loom-libs/ops/gated_delta_net_f32_wmma.loom +loom-libs/ops/llm_attention_k_matmul_rope_set_rows_decode_f32_f32.loom loom-libs/ops/llm_attention_v_matmul_set_rows_decode_f32_f32.loom +loom-libs/ops/mul_mat_id_swiglu_f32_f32_wmma.loom +loom-libs/ops/mul_mat_q6_k_packed_f16_wmma.loom +loom-libs/ops/mul_mat_swiglu_f32_f32_wmma.loom loom-libs/ops/mul_mat_swiglu_symmetric_i4_wmma.loom +loom-libs/ops/mul_mat_symmetric_i4_wmma.loom loom-libs/ops/mul_mat_symmetric_i8_wmma.loom +loom-libs/ops/rmsnorm_binary_f32.loom +loom-libs/ops/rope_set_rows_f32.loom loom-libs/ops/set_rows.loom loom-libs/ops/small_rows_f32.loom +qwen_moe/ggml/linear_q6k_q8_1_x4.loom qwen_moe/ggml/quantize_q8_1_x4.loom +qwen_moe/qwen3_moe/attention_postprocess_f32_f16.loom qwen_moe/qwen3_moe/attention_prepare_quantized.loom qwen_moe/qwen3_moe/dense_linear_quantized_f16_wmma.loom qwen_moe/qwen3_moe/flash_attention_decode_f32_f16_wmma.loom @@ -43,6 +68,7 @@ qwen_moe/qwen3_moe/flash_attention_decode_q128_f32_f16_wmma.loom qwen_moe/qwen3_moe/flash_attention_decode_split_f32_f16_wmma.loom qwen_moe/qwen3_moe/flash_attention_decode_split_next_q8_test.loom qwen_moe/qwen3_moe/flash_attention_f32_f16_wmma.loom +qwen_moe/qwen3_moe/routed_down_quantized_f16_wmma.loom qwen_moe/qwen3_moe/routed_down_weighted_reduce_next_rmsnorm_q8_1_x4.loom qwen_moe/qwen3_moe/routed_gate_up_swiglu_q4k.loom qwen_moe/qwen3_moe/routed_linear_q4k_f16_wmma.loom