From 7aed9ed18e0f65f683c40eb58990baf0bf80eb72 Mon Sep 17 00:00:00 2001 From: Vasilev Dmitrii Date: Tue, 18 Aug 2026 19:40:45 +0700 Subject: [PATCH] fix(tools): print what the compiler said, not a guess at which subsystem broke Root-caused while fixing #2185. The four-day misdiagnosis was the wrappers, not the missing brace. An AST scan over tools/*.py found two patterns: run(..., capture_output=True).returncode 9 sites exit code read, message dropped run(..., capture_output=True).stdout 18 sites NEITHER exit code nor stderr read The second is what cost the days. When t27c gen-c failed to parse the spec, .stdout was the empty string, it flowed downstream, and the failure surfaced as 'FAIL: C backend failed to build/run' -- naming a subsystem that had never been reached. The compiler's message named file, function, line and token, and capture_output collected it so it could be discarded. Two self-contained helpers per script, since these tools are deliberately import-free: _build prints a compiler's message on failure; _gen returns None and prints the reason when t27c gen- exits non-zero. Call sites handle None so the run ends cleanly rather than with a traceback. Verified by planting the original fault. With a brace removed, the output now leads with the compiler's own message before the misleading summary. Self-critical: my first attempt fixed the wrong nine sites -- the .returncode ones -- and changed the output not at all, because the failure was upstream in the .stdout calls my scan had not been written to find. The negative control caught that. The scan alone would have let me report a fix that fixed nothing. Closes #2187 Refs #2185 Co-Authored-By: Claude Opus 5 --- docs/NOW.md | 12 +++++ tools/verify_igla_race.py | 87 +++++++++++++++++++++++++++++-------- tools/verify_multitarget.py | 28 ++++++++++-- tools/verify_trainer_c.py | 25 ++++++++++- 4 files changed, 128 insertions(+), 24 deletions(-) diff --git a/docs/NOW.md b/docs/NOW.md index db529269aa..1e13e5f0e0 100644 --- a/docs/NOW.md +++ b/docs/NOW.md @@ -1,3 +1,15 @@ +# NOW -- the wrappers threw the compiler's message away (2026-08-18) + +Last updated: 2026-08-18 + +## tools: print what the compiler said, instead of guessing which subsystem broke (Closes #2187) + +- **The four-day misdiagnosis in #2185 was the wrappers, not the brace.** An AST scan over `tools/*.py` found two patterns: 9 sites doing `run(..., capture_output=True).returncode` (exit code inspected, message discarded) and **18 doing `run(..., capture_output=True).stdout`** -- taking stdout while checking neither the exit code nor stderr +- **The second is the one that cost the four days.** When `t27c gen-c` failed to parse, `.stdout` was the empty string; it flowed downstream and surfaced as `FAIL: C backend failed to build/run`, naming a subsystem that had never been reached +- Two self-contained helpers per script, because these tools are deliberately import-free: `_build` prints a compiler's message on failure, `_gen` returns `None` and prints the reason when `t27c gen-` exits non-zero. Callers handle `None` so the run ends cleanly instead of with a traceback +- **Verified by planting the original fault.** With a brace removed, the output now leads with `t27c gen-c ...: exited 1` and the compiler's own `parse error in fn '...' near line 1810: unexpected token after expression statement: KwTest`, before the misleading summary +- **My first attempt fixed the wrong nine sites and changed nothing** -- the failure was upstream, in the `.stdout` calls the scan had not been written to find. The negative control caught it; the scan alone would have let me report a fix that fixed nothing + # NOW -- one missing brace, and three layers that named the wrong subsystem (2026-08-18) Last updated: 2026-08-18 diff --git a/tools/verify_igla_race.py b/tools/verify_igla_race.py index db26bae878..f011db5828 100644 --- a/tools/verify_igla_race.py +++ b/tools/verify_igla_race.py @@ -14,6 +14,50 @@ """ import os, re, sys, shutil, subprocess, tempfile, random +def _gen(t27c, mode, spec, root): + """Run `t27c gen-` and return its output, or None with the reason printed. + + Every caller below used to take `.stdout` directly, checking neither the exit + code nor stderr. When the spec failed to PARSE, stdout was empty, the empty + string flowed downstream, and the failure surfaced as "the C backend failed to + build/run" -- pointing at a subsystem that had never been reached. The + compiler's message named the file, the function, the line and the token; it + was collected by capture_output and discarded. Four days were read in the + wrong place. Ask the exit code, and print what the tool said. + """ + r = subprocess.run([t27c, "gen-" + mode, spec], capture_output=True, text=True, cwd=root) + if r.returncode == 0: + return r.stdout + out = (r.stderr or r.stdout or "").strip().splitlines() + print(f" t27c gen-{mode} {spec}: exited {r.returncode}" + + ("" if out else " with no message")) + for line in out[:4]: + print(f" {line}") + return None + + +def _build(cmd, cwd, what): + """Run a compiler and, if it fails, print what IT said before giving up. + + Every caller below used to test `.returncode` on a capture_output=True run and + discard the message. That is how a missing brace in a spec came to be reported + as "the C backend failed to build" for four days: the compiler named the file, + the function, the line and the token, and the wrapper threw it away. A + diagnostic that names the wrong subsystem costs more than none. + """ + r = subprocess.run(cmd, cwd=cwd, capture_output=True, text=True) + if r.returncode == 0: + return True + out = (r.stderr or r.stdout or "").strip().splitlines() + print(f" {what}: {os.path.basename(cmd[0])} exited {r.returncode}" + + ("" if out else " with no message")) + for line in out[:6]: + print(f" {line}") + if len(out) > 6: + print(f" ... {len(out) - 6} more line(s)") + return False + + ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) SPEC = "specs/igla/race/ternary_mac.t27" SYS_SPEC = "specs/igla/race/systolic_ternary.t27" @@ -91,19 +135,24 @@ def full_spec_compiles(t27c, wd): """Diagnostic: does the WHOLE spec emit compilable C and Rust? Returns (c_ok, rust_ok, note). This surfaces gen-backend gaps in the IGLA spec (slice .len(), duplicate test emission, serde deps, non-Copy struct).""" - c = subprocess.run([t27c, "gen-c", SPEC], capture_output=True, text=True, cwd=ROOT).stdout + c = _gen(t27c, "c", SPEC, ROOT) + if c is None: + return False, False open(os.path.join(wd, "full.c"), "w").write('#define assert_eq(x,y) ((void)0)\n' + c + "\nint main(){return 0;}\n") - c_ok = subprocess.run(["cc", "-c", "-o", os.path.join(wd, "f.o"), os.path.join(wd, "full.c")], - cwd=wd, capture_output=True, text=True).returncode == 0 - r = subprocess.run([t27c, "gen-rust", SPEC], capture_output=True, text=True, cwd=ROOT).stdout + c_ok = _build(["cc", "-c", "-o", os.path.join(wd, "f.o"), os.path.join(wd, "full.c")], wd, "full-spec gen-c") + r = _gen(t27c, "rust", SPEC, ROOT) + if r is None: + return False, False open(os.path.join(wd, "full.rs"), "w").write(r + "\nfn main(){}\n") - r_ok = subprocess.run(["rustc", "-A", "warnings", "--emit=metadata", "-o", os.path.join(wd, "f.rmeta"), - os.path.join(wd, "full.rs")], cwd=wd, capture_output=True, text=True).returncode == 0 + r_ok = _build(["rustc", "-A", "warnings", "--emit=metadata", "-o", os.path.join(wd, "f.rmeta"), + os.path.join(wd, "full.rs")], wd, "full-spec gen-rust") return c_ok, r_ok def _core_c(t27c): - src = subprocess.run([t27c, "gen-c", SPEC], capture_output=True, text=True, cwd=ROOT).stdout + src = _gen(t27c, "c", SPEC, ROOT) + if src is None: + return None st = re.search(r"typedef struct\s*\{[^}]*\}\s*TernaryWeight\s*;", src) defs = [_extract_def(src, s) for s in ( "int8_t ternary_decode(TernaryWeight w)", @@ -115,7 +164,9 @@ def _core_c(t27c): def _core_rust(t27c): - src = subprocess.run([t27c, "gen-rust", SPEC], capture_output=True, text=True, cwd=ROOT).stdout + src = _gen(t27c, "rust", SPEC, ROOT) + if src is None: + return None ms = re.search(r"pub struct TernaryWeight\b", src) if not ms: return None @@ -144,8 +195,7 @@ def run_c(t27c, vecs, wd): f'for(int i=0;i supply the # primitive from ternary_mac's core, then add systolic's tuple + PE definition core = _core_c(t27c) - sysc = subprocess.run([t27c, "gen-c", SYS_SPEC], capture_output=True, text=True, cwd=ROOT).stdout + sysc = _gen(t27c, "c", SYS_SPEC, ROOT) + if sysc is None: + return None tup = re.search(r"typedef struct\s*\{[^}]*\}\s*t27_tuple_int8_t_int16_t\s*;", sysc) pe = _extract_def(sysc, "t27_tuple_int8_t_int16_t systolic_ternary_pe(int8_t a_in, TernaryWeight w, int16_t psum_in)") if core is None or not tup or pe is None: @@ -198,8 +249,7 @@ def run_pe_c(t27c, vecs, wd): f'for(int i=0;i 6: + print(f" ... {len(out) - 6} more line(s)") + return False + + ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) SPECS = {"gft_smul": "smul", "gft_sadd": "sadd"} # spec file -> top function N = 600 @@ -78,8 +100,7 @@ def run_c(t27c, spec, fn, pairs, wd): f'uint32_t A[]={{{a}}},B[]={{{b}}};int n={len(pairs)};' f'for(int i=0;i 6: + print(f" ... {len(out) - 6} more line(s)") + return False + + ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) SMUL_SPEC = os.path.join(ROOT, "specs/ternary/gft_smul.t27") ARCHS = [(2, 2, 1), (2, 4, 2), [2, 4, 3, 1]] # 2-layer, multi-output, deep @@ -135,8 +157,7 @@ def run_c(g, reg, steps, init_pairs, n_in, n_out, seq, t27c, wd): """ cf = os.path.join(wd, "trainer.c"); open(cf, "w").write(main) b = os.path.join(wd, "tbin") - r = subprocess.run(["cc", "-O2", "-o", b, cf], cwd=wd, capture_output=True, text=True) - if r.returncode != 0: + if not _build(["cc", "-O2", "-o", b, cf], wd, "trainer C"): return None out = subprocess.run([b], capture_output=True, text=True).stdout return [tuple(map(int, ln.split())) for ln in out.strip().splitlines()]