tune: a progress display for --tune, per-ISA [tuned] fallbacks, and arm64 cpu_supports - #3583
Merged
Conversation
Captures the current four-level process shape, the buffered-relay blocker in run_scope_tuner, the event vocabulary, the renderer-vs-forwarder rule, the new daslib/tty binding, and the watchdog.py strings that are a live contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records the four hand-rolled parsers to migrate, get_user_args as the accessor, the shared-argv precedent (ReleaseDepsArgs, same module), and the unknown-flag value-passthrough hazard that keeps the tune spec bool-only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The generator half silently drops the flag the tuner passes it. Records the round-count cut as the analogue of tune_kernels' screening cut, and why the confirm pass stays untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Maps the tuning/restart/normal narrative onto the restart policy rail, which is already correct end to end; the two gaps are the up-front promise and the buffered silence. Adds the watchdog-as-renderer consequence and the STATE control-page surface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Everything displayed is observed, never predicted. Records the three honest axes beyond the outer counter, including the screening/narrowing/finalists passes tune_candidate_active already runs and the live count screening_keep already computes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
run_scope_tuner redirected the tuner to a temp file with an empty popen callback and printed the file after the child exited, so a minutes-long tune produced no signal at all until it finished. Relay live via fgets, the same shape tune_auto_reexec and dasllama_tuner already use. Also stops discarding the tuner's exit code: popen returns it, so report a non-zero rc and return it rather than an unconditional true. Both current callers ignore the result, so this changes no behavior today. Tests: llvm_tune_scope.das and llvm_tune_manifest.das, both under -jit (they short-circuit to pass without it). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ind them daslang had no way to ask whether a stream is a terminal — isatty appeared only in vendored doctest/fmt/gtest C++, and no width query existed at all. Sniffing TERM (what ansi_colors does for styling) is not a substitute: it stays set when stdout is a pipe or a log file, which is exactly the case a redrawn progress display must not fire in. fio_core gains is_terminal(fd) and terminal_width(); daslib/tty wraps them as named predicates plus terminal_columns(fallback), and does not re-export fio_core. Width falls back to COLUMNS, then to the caller's fallback. Verified: piped -> false/0; piped with COLUMNS=123 -> 123; attached to a real pty sized 137 cols -> true/137. AOT emission checked via -aot on daslib/tty.das (the daslib glob AOTs it, hence the aot_builtin_fio.h declarations). Tests: tests/daslib/tty_test.das (registered for AOT), plus tests/fio and tests/daslib green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The tuner runs as a chain of child processes, so progress travels as prefixed
lines on stdout ("@tune kind k=v ...") — a pipe is the only channel spanning
them. llvm_tune owns the vocabulary and the display, so any library's tuner
gets it, not just dasLLAMA's.
One rule decides who draws: a process renders when its OWN stdout is a
terminal, and otherwise forwards events verbatim. Forwarding is unconditional;
verbosity gates only what a human sees. That is what lets "silent" mean
literally nothing on a terminal while a supervisor still receives every event.
Raw tuner chatter is muted only while a display is actually live. A tuner that
emits no events keeps printing exactly as before — the existing scope test
caught the earlier rule, which had made adopting the protocol a prerequisite
for being visible at all.
No duration is ever shown, estimated or otherwise; the segments are too uneven
to predict. The counters are outer kernels, inner rounds, the pass name
(screening/narrowing/finalists) and the live-candidate count, plus observed
elapsed.
Adds the up-front promise: an untuned start now says a tune takes a while and
ends in a restart BEFORE the wait, and says whether the process restarts itself
(auto) or must be restarted (restart policy).
tune_progress_line/_feed/_active/_reset are public so another front end can
render the same state — and so the protocol is testable without owning a tty.
Tests: modules/dasLLVM/tests/llvm_tune_progress.das (9 cases, no tty needed);
whole dasLLVM suite green under -jit. Verified end to end against a real pty:
pipe forwards 19 events and draws 0 bars, pty draws 19 and forwards 0, silent
draws nothing on the pty yet still forwards all 19 under a pipe, verbose alone
shows raw chatter.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…emitters The flags now go through daslib/clargs everywhere, replacing three hand-rolled argv scans. llvm_tune parses the APP's argv it does not own, so it uses the two-arg parse_args form (the ReleaseDepsArgs precedent in this same module); its spec is bool-only and never reads positionals, because clargs skips an unknown flag without consuming its value and an app's `--model foo.gguf` thus leaves `foo.gguf` in the positional list. get_user_args replaces the hand-rolled post-`--` walk and also gets the standalone-exe slice right. --tune-quiet / --tune-verbose set DAS_TUNE_VERBOSITY, which inherits down the whole tuner process chain. They are adopted at macro time, so the value has to travel by environment: the runtime guard is a different context and would not see this one's global. gen_tune_probe never parsed --tune-fast although dasllama_tuner passed it to both halves and bannered "FAST". It now honours it, ROUNDS 6 -> 3 — the flat best-of-N has no screening phase to narrow, so the round count is the only dial. The confirm pass is deliberately untouched: it is a correctness gate, and cutting it ships an unconfirmed crown. The banner no longer advertises tune_kernels' budget as though it covered both halves. The three consumers share one flag spec, harness/tuner_cli.das, required by bare name; the producer side (daspkg) was already clargs. Both halves now emit progress events. tune_kernels reports the pass and the live-candidate count that screening_keep already computes; gen_tune_probe gives the end-to-end confirm its own visible step so it is not an invisible stall. Verified by running both tuners for real: tune_kernels --tune-fast -> plan total=25, 25 begin/end, 500 steps (25 x 20 rounds), phases split 100 screening / 100 narrowing / 300 finalists — exactly fast mode's 4/8/20, which also proves the clargs migration took — live counts narrowing 20 -> 12 -> 4, verdicts 18 beats / 7 holds. gen_tune_probe --tune-fast -> plan total=7, per-family total=3 (the 6 -> 3 cut). Docs: skills/tune.md (what a tune shows + the event protocol), skills/environment_variables.md (DAS_TUNE_VERBOSITY), skills/llvm_tune.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The wire format is k=v split on spaces, so gen_tune_probe's "confirm (end-to-end prefill)" step name truncated to "confirm" and left two junk tokens. It degraded rather than corrupted — the display and counters were fine — but it broke the contract, so the name is now confirm_e2e_prefill and the contract is stated where the prefix is defined. Found by reading the real emitted stream rather than by a test, so a test now pins the degradation shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… prose A supervised child never owns a terminal, so llvm_tune forwards its @tune events rather than drawing a bar — which is exactly what makes them the watchdog's to consume. They fold into STATE["tune"] for the control plugin's status route, and log only at plan/end: a real tune is hundreds of steps, and logging each would recreate the per-variant flood these events exist to replace. Measured, that is 26 log records from 551 events, where the whole raw tuner output used to be JSON-wrapped line by line. This is the part of STARTUP_STAGES that no longer has to regex human prose. The rest still does, and both load-bearing strings are untouched, so a deployed old watchdog meeting a new daslang (and the reverse) still works — verified that both stage regexes still match. Every published field is a counter. Nothing is an estimate, and the README says not to synthesize one: the tuner cannot know how long it has left. Verified by replaying both real captured tuner streams through the new parser: done lands exactly on total for each (25/25 kernels, 7/7 generator families), done never exceeds total at any snapshot, malformed input never raises, and a prose line is never mistaken for an event. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running a tuner half straight from a terminal spewed raw @tune lines instead of a bar, because only the relay rendered. Emitting now goes through the same relay, so the rule is uniform: any process owning a terminal draws, any other forwards. Verified under a pty — 0 raw event lines leaked, 551 bar frames. tune_progress_finish() closes the display; both halves call it. llvm_tune.das lint 49 -> 0, since a PR has to be clean and the rest of daslib already is (143 files, 0 issues). 8x LINT016: `errors :=` on a plain `string&` promised a clone it does not perform. Only the two string& functions were touched — the nine das_string ones use := correctly and were left alone. 41x STYLE014/STYLE015: public //! docs trimmed to what they document, with the deep semantics already carried by skills/tune.md and skills/llvm_tune.md; internal // blocks cut to the one WHY a reader cannot get from the code. No nolint markers — the rule's escape exists but the tree essentially does not use it (3 files), so complying is the honest fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The display only replaced output that flowed through the relay. A tuner half run directly has no relay above it, so report()'s per-variant lines and the checksum lines went straight to stdout and trampled the bar, and the run ended by fread-ing the whole sidecar back — the "output mainly duplicates json" complaint, still intact. tune_detail() is the gate for a tuner's own measurement lines: shown under verbose, or whenever no display is live, so a tuner that emits no progress events still prints exactly as before. Both halves route their chatter through it; the summary block, hard failures and the "wrote <path>" line stay unconditional, and the sidecar body is now verbose-only. finish_progress no longer emits a newline when nothing was drawn — under a pipe the state is live but no bar exists, so it was adding a stray blank line. Measured on a real terminal at normal verbosity: per-variant chatter 0 (was ~500), checksum lines 0, sidecar json 0, 551 bar frames, 31 lines of tuner output total. Note the ~100 `loop not vectorized` lines around a JIT tune are LLVM's own diagnostics on stderr, not this rail: the [tuned] grid deliberately requests vectorization widths LLVM cannot always honour. stdout carries none of them, so 2>/dev/null leaves a clean display. Quieting them needs an LLVM diagnostic handler and is a separate change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A real tune printed 34 `dasllama_tune: <kernel> <- <perm>` lines (plus the llvm_tune twin) before anything else — the compile reporting every stamp it made. That is a wall in front of an app's own startup, and it says the same thing log_tune_status() says on request and the closing summary says at the end. Worse, side by side the two disagree by design: the stamp list is what THIS compile used, the summary is what the run just measured. Both are gated to verbose now. The two tests that read those lines as their stamped-truth proof pass DAS_TUNE_VERBOSITY=verbose to their children; in llvm_tune_manifest that also lets the env prefix lead, so the windows sacrificial-quote wrap is no longer needed. On a terminal a full --tune-fast run is now: 4 JIT lines, the tuner banner, one live bar, the summary, and "wrote <path>". Measured: 551 bar frames, 0 raw events, 0 stamp lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
[tuned]'s fallback= was a single string — one generic default for every target — while [tune]'s was already a per-ISA `;`-chain. That asymmetry is why 18 of 25 kernels beat their shipped default on an M1: a single guess cannot be right for both ARM and x64, and an untuned box (CI, AOT, a release with no sidecar) just ate the miss. fallback= is now a `;`-chain of `suffix` or `suffix:requires` entries, first passing entry wins, so ONE annotation ships a different default per ISA. Its perms are generated and have no row to hang requires= on, hence the inline form; [tune]'s own chain is unchanged. A bare name is a one-entry chain, so every existing annotation behaves exactly as before. The requires= evaluator moves to llvm_tune as public tune_requires_ok(expr) + tune_pick_fallback(chain), so both halves share one implementation of the `,`=AND / `|`=OR grammar and the DAS_JIT_*_FORCE_FEATURES overrides. PREREQUISITE, not scope creep: cpu_supports was x86-only and returned false for every name on arm64 — including neon. Built on that, the feature would have been dead on the platform that motivated it, always falling through to the generic entry. It now answers on arm64 via sysctl (macOS), AT_HWCAP (Linux) and IsProcessorFeaturePresent (Windows), using LLVM target-feature spellings so the names match what DAS_JIT_ARM64_FORCE_FEATURES and `llc -mattr` take. Unknown and wrong-architecture names stay fail-closed. Verified on an M1 Max against sysctl ground truth: dotprod/fullfp16/lse/rdm/ crc/sha3 true, i8mm/bf16/sve false, x86 names false. End to end on a real kernel, three ways: dotprod present picks the arm row; gated on i8mm (absent here) falls through to the generic row; DAS_JIT_ARM64_FORCE_FEATURES=i8mm picks the arm row again. resolve_perm now reports every path at verbose (sidecar / fallback / default), not just sidecar hits — the fallback decision was previously unobservable. Tests: modules/dasLLVM/tests/llvm_tune_requires.das (9 cases, host-independent); tests/language/cpu_supports.das gains arm64 coverage and loses a comment that this change made false. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rays The shipped-fallback fact lived in three hand-maintained copies: the [tuned(fallback=)] annotation, report()'s baseline argument at 23 call sites, and the kfall array. Two of them were display-only; report()'s baseline is not — a sub-0.5% win reverts to it, and that decides what lands in the sidecar. report() is called exactly once per kernel and already holds all three values, so it now records (kernel, winner, fallback) in call order and the summary reads that back. kfall, knames and kwin — three parallel 25-entry arrays — are gone, and the summary can no longer disagree with the decision it reports, because it IS that value. All three copies agreed before this change (verified across all 25 kernels), so this is a refactor with no behavioural delta: same 25 lines, same order, same note logic. The winner locals stay — they still feed the sidecar table, which carries extra semantics the summary does not (softmax_sink mirrors softmax, plus explicit entries for kernels this half does not sweep). TUNED_KERNEL_COUNT stays hand-written: it is the progress denominator and is needed before the first kernel runs, so it cannot come from the results. main now compares it against the rows actually collected and says so if they differ, rather than letting the bar quietly misreport. The remaining duplicate is annotation <-> report baseline. That one has to close before any kernel adopts a per-ISA fallback chain, since a chain resolves per box and a literal baseline argument cannot follow it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…terals The last duplicate. report()'s baseline was 23 hand-written literals restating the [tuned(fallback=)] annotations, and it is not cosmetic: a sub-0.5% win reverts to the baseline, so a wrong one changes what ships. With a per-ISA chain the literal could not be right at all — a chain resolves per box. [tuned] now banks each kernel's resolved fallback and [dasllama_fallbacks] emits dasllama_tuned_fallbacks() from that bank; report() looks it up. All 23 literals are gone. Verified the derived values equal the removed literals for all 25 swept kernels, and the registry carries all 34 [tuned] kernels. Probed the module-hop risk before building on it: a bank filled during the math modules' compile IS visible to a macro applied in tune_kernels (34 entries), because both are the same macro module. The per-requiring-module-copy trap llvm_tune documents applies across macro modules, not within one. Proven with a real chain on rmsnorm: gated on dotprod (present here) the summary reports the fallback as vec16; gated on i8mm (absent) it reports plain. Both said "plain" under the literals, and the noise-floor guard would have judged against the wrong default. Also: `explicit` is a reserved word (DAS_EXPLICIT, the generic-parameter modifier) — hit while naming a local. Added to CLAUDE.md's reserved list, which had label/expect/pass but not this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A new daslib module and two new fio externs both trigger doc.yml. Registers the module (require + document_module_tty + the call + the sec_io toctree + the stdlib index) and groups is_terminal/terminal_width under fio's new "Terminal queries" heading, since an ungrouped public function lands in Uncategorized and fails CI. Fills the three handmade stubs das2rst generated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The arm64 detection block put <sys/sysctl.h>, <sys/auxv.h> and <asm/hwcap.h> inside `namespace das` — the namespace opens at the top of the file and the block sits ~1950 lines in. It built here only because macOS takes the __APPLE__ branch and clang tolerated it; the linux branch pulls two more headers into das::, and a mac-only syntax pass never compiles that path at all. Headers and the HWCAP_* fallback defines move to file scope; the helper functions stay where they were. No behavioural change — same declarations, same macros, just not injected into the namespace. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The arc ships with this branch, so per skills/doc_archiving.md the plan doc moves to history/compiler/ rather than sitting stale in a live module dir. Its operative content is now skills/tune.md (user-facing) and skills/llvm_tune.md (internals); what remains is the record of why it is shaped this way. Ledger line added to history/README.md; no inbound references to the old path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the user-facing --tune experience by adding a streaming progress protocol (forwarded across the multi-process tuning chain) and the terminal-capability plumbing needed to render it safely, while also extending cpu_supports to answer on arm64 and enabling per-ISA [tuned] fallback selection.
Changes:
- Add a
@tune ...event protocol plus renderer/forwarder logic (tty-gated) so tuning progress is visible live without flooding logs. - Introduce
fio_core::is_terminal/terminal_widthand a newdaslib/ttymodule (plus docs/tests) to support terminal detection and sizing. - Extend
cpu_supportsto arm64 and add per-ISA[tuned] fallback=...chains, with tests and watchdog consumption of tune events.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| utils/watchdog/watchdog.py | Parses @tune events from child stdout and publishes folded tune progress state for the control page while keeping logs low-volume. |
| utils/watchdog/README.md | Documents tune progress event consumption and the STATE["tune"] contract. |
| tests/language/cpu_supports.das | Adds arm64 baseline/implication assertions and cross-arch fail-closed checks. |
| tests/daslib/tty_test.das | Adds invariant-based tests for terminal detection and column probing. |
| tests/aot/CMakeLists.txt | AOT-registers the new tests/daslib/tty_test.das. |
| src/builtin/module_builtin_runtime.cpp | Implements arm64 CPU feature detection for cpu_supports via sysctl/auxv/Windows feature probes. |
| src/builtin/module_builtin_fio.cpp | Adds is_terminal(fd) and terminal_width() builtins for terminal detection/geometry. |
| skills/tune.md | Documents the new progress display semantics and the @tune event protocol. |
| skills/llvm_tune.md | Updates internal framework documentation (progress events, per-ISA fallback chains, arm64 cpu_supports). |
| skills/environment_variables.md | Documents DAS_TUNE_VERBOSITY and clarifies feature forcing behavior. |
| modules/dasLLVM/tests/llvm_tune_scope.das | Adjusts scope test env to enable verbose stamping output now gated by verbosity. |
| modules/dasLLVM/tests/llvm_tune_requires.das | Adds tests for requires= expression semantics and fallback chain selection. |
| modules/dasLLVM/tests/llvm_tune_progress.das | Adds tests for feeding/rendering progress events without needing a tty. |
| modules/dasLLVM/tests/llvm_tune_manifest.das | Ensures verbose stamping lines are present for manifest tests. |
| modules/dasLLVM/daslib/llvm_tune.das | Implements progress event relay/rendering, verbosity control, clargs-based parsing, and per-ISA fallback picking. |
| modules/dasLLAMA/tune_for_this_box.md | Updates tuning docs to reflect per-ISA fallback chains and new semantics. |
| modules/dasLLAMA/harness/tuner_cli.das | Centralizes --tune-fast parsing via daslib/clargs. |
| modules/dasLLAMA/harness/tune_kernels.das | Emits progress events, switches chatter to tune_detail, and removes duplicated shipped-fallback sources. |
| modules/dasLLAMA/harness/gen_tune_probe.das | Honors --tune-fast, emits progress events, and routes chatter through tune_detail. |
| modules/dasLLAMA/harness/dasllama_tuner.das | Switches to shared clargs-based --tune-fast parsing and updates mode banner. |
| modules/dasLLAMA/dasllama/dasllama_tune.das | Adds per-ISA fallback resolution and generates dasllama_tuned_fallbacks() from a fallback bank. |
| include/daScript/simulate/aot_builtin_fio.h | Exposes new fio builtins for AOT builds. |
| history/README.md | Records the archival move for the tune UX design doc. |
| history/compiler/TUNE_PROGRESS_UX.md | Adds the archived design record for the tune progress UX work. |
| doc/source/stdlib/sec_io.rst | Adds generated tty module docs to stdlib IO section. |
| doc/source/stdlib/introduction.rst | Lists tty in the stdlib introduction. |
| doc/source/stdlib/handmade/module-tty.rst | Adds prose docs for the tty module. |
| doc/source/stdlib/handmade/function-fio-terminal_width-0x911587b28bfa73bb.rst | Adds docs for fio_core::terminal_width. |
| doc/source/stdlib/handmade/function-fio-is_terminal-0x356da75bc2909a8d.rst | Adds docs for fio_core::is_terminal. |
| doc/reflections/das2rst.das | Wires tty into the doc reflection generator and groups terminal queries in fio docs. |
| daslib/tty.das | Introduces the tty module wrapper API (is_*_terminal, terminal_columns). |
| CLAUDE.md | Updates the reserved-words list to include explicit. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The _M_ARM64 path calls IsProcessorFeaturePresent with PF_ARM_* constants, but nothing in this TU's include graph pulls windows.h in — include/daScript/ does not include it anywhere — so an arm64 Windows build would not compile. CI has no windows-arm64 lane, so this would have shipped latent. Guarded to arm64 + _WIN32, with NOMINMAX and WIN32_LEAN_AND_MEAN, so no lane CI actually builds sees any change. Caught by Copilot on #3583. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
stream_child kept `tune` local per child, but STATE["tune"] was only written when an event arrived — so the tune-bootstrap restart, which is the normal first-run path, left the control page showing the finished tune's counters for the rest of the run. The relaunched child is already tuned and emits no events at all, so nothing ever overwrote them. Publish an empty tune state as each child starts. Caught by Copilot on #3583. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
bench_laneq4x4 returns before report() on non-arm64 or without -jit, and report() is the only place that emits the end event and records a result row. kernel_begin had already announced the kernel, so on those hosts the outer counter stopped one short of its plan for the rest of the run — the bar never completed, the summary silently dropped the row, the hand-written-count check misfired, and watchdog's STATE["tune"] never converged. Fixed in kernel_done rather than at the one call site: it brackets every kernel in main and runs even when the bench returns early, so any future skip path is covered by construction. Audited the other 22 bench functions — laneq4x4 is the only one with a non-report return. Skipped kernels now render as "skipped" instead of a blank winner, and `skipped` joins the documented verdict set. Caught by Copilot on #3583. Verified the normal -jit path is unchanged: 25 begin / 25 end against plan total=25, 25 summary rows, no count-drift note. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
consume_event's plan branch set the outer counters but left kernel/phase/inner counters/live from the scope before it. One relay sees BOTH dasllama halves, so after the generator half ends, the kernel half's plan left the display showing "0/25 confirm_e2e_prefill" — and that is not a single frame: tune_kernels has ~30 lines of fixture setup between its plan and its first kernel_begin, so the wrong kernel name is on screen for all of it. plan now rebuilds the whole state, which is what a fresh scope means; begin already did the same for per-kernel fields. Test added for the two-plan sequence. Caught by Copilot on #3583. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
apply_tune_event's "end" cleared rounds/live but left round and phase from the last step, so between kernels STATE published an incoherent in-flight round — round=47 against rounds=0 — which a status page renders verbatim. The das renderer hides those fields when innerTotal is 0, but STATE has no such guard; it is read raw. Same class as the plan-reset fix in llvm_tune, and more visible here for that reason. Caught by Copilot on #3583. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
emit(logger, "tune", pid=..., kind=kind, **fields) splats parsed fields as kwargs, so a line carrying pid= or kind= is a duplicate-keyword TypeError, and ts=/event= would silently shadow the log envelope. The child's stdout is arbitrary bytes — the watchdog also supervises non-daslang programs — so that input is allowed by contract, and parse_tune_event's docstring already promises never to raise on malformed input. The blast radius is bigger than a lost log line: stream_child runs in a daemon thread, so the exception kills it silently and the watchdog keeps running blind — no stage detection, no child log relay, and no tune_restart_seen, which is what tells exit 3 apart from a crash. A garbled line would have turned every subsequent tune bootstrap into a backoff restart. Reserved keys are dropped before the splat. Caught by Copilot on #3583. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
modules/dasLLAMA/harness/tune_kernels.das:2035
- The summary renders any empty winner as "skipped", but report() also records winner="" when a kernel has no passing variant (verdict=rejected). That makes the summary misleading (rejected kernels appear as skipped). Either record the verdict in KernelResult so you can render "rejected" vs "skipped", or change the empty-winner label to something neutral.
for (r in g_results) {
let note = (r.winner == "" || r.winner == r.fallback) ? "" : " (shipped fallback {r.fallback})"
print(" {r.kernel}: {empty(r.winner) ? "skipped" : r.winner}{note}\n")
if (r.winner != "" && r.winner != r.fallback) {
report()'s "no passing variant" printed straight to stdout while the bar was
live, so it landed on the bar's line and the next redraw wrote over it — the one
message you must not lose was the one most likely to be destroyed.
Routing it through tune_detail (the obvious fix) would be worse: that mutes
under normal verbosity, so a hard failure would vanish entirely. Added
tune_progress_note instead — it wipes the bar's line, prints the text, and
leaves the display armed so the bar redraws underneath. Silent still suppresses.
Verified on a pty by replaying the CR/LF stream the way a terminal does:
k0: no passing variant
[######--------------] 1/3 k1 finalists 2/4 4 live 0s
Copilot also flagged the summary block for the same reason; that one is fine —
it runs after tune_progress_finish() has already closed the display.
Caught by Copilot on #3583.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A tune used to print a per-variant firehose that mostly restated the sidecar JSON.
--tuneis user-facing, so it now shows a progress display instead:Outer kernels, the kernel being tuned, the screening pass with its round counter, how many grid rows are still in contention, and observed elapsed. Nothing is predicted — the segments are far too uneven for an ETA to mean anything, so none is offered.
The blocker underneath it
run_scope_tunerredirected the tuner to a temp file with an empty popen callback and printed the file after the child exited. On the default auto policy — the user-facing path — a minutes-long tune produced no output at all until it finished. That window is precisely whatutils/watchdog/watchdog.pydocuments at line 514: "startup is several minutes of work with no single progress signal … the log is onlyhealth ok:falserepeated for the whole window." The relay now streams.How the display travels
The tuner is a chain of four processes, so progress travels as prefixed stdout lines — a pipe is the only channel spanning them:
One rule decides who draws: a process renders when its own stdout is a terminal, otherwise it forwards events verbatim. Forwarding is unconditional; verbosity gates only the human-visible half. That is what lets
silentmean literally nothing on a terminal while a supervisor still receives every event.Raw tuner chatter is muted only while a display is live, so a tuner that emits no events prints exactly as it always did — adopting the protocol is never a prerequisite for being visible.
llvm_tuneowns the vocabulary and the renderer, so any library's tuner gets this, not just dasLLAMA's.daslib/tty— new, with the binding under itdaslang had no terminal detection at all:
isattyappeared only in vendored doctest/fmt/gtest C++, and there was no width query.TERMis not a substitute — it stays set when stdout is a pipe, which is exactly the case a\r-redrawn display must not fire in.fio_coregainsis_terminal(fd)/terminal_width();daslib/ttywraps them as named predicates plusterminal_columns(fallback).cpu_supportsnow answers on arm64A prerequisite, not scope creep.
cpu_supportswas x86-only and returned false for every name on arm64 — includingneon. The per-ISA fallback feature below would have compiled, tested green, and silently never fired on ARM, always falling through to the generic entry.It now answers via sysctl (macOS),
AT_HWCAP(Linux) andIsProcessorFeaturePresent(Windows), using LLVM target-feature spellings so names matchDAS_JIT_ARM64_FORCE_FEATURESandllc -mattr. Unknown and wrong-architecture names stay fail-closed.Verified on an M1 Max against sysctl ground truth:
dotprod/fullfp16/lse/rdm/crc/sha3true,i8mm/bf16/svefalse, x86 names false.Per-ISA
[tuned]fallbacks[tuned]'sfallback=was a single string — one generic default for every target — while[tune]'s was already a per-ISA chain. That asymmetry is why 18 of 25 kernels beat their shipped default on an M1: one guess cannot be right for both ARM and x64, and an untuned box (CI, AOT, a release with no sidecar) just ate the miss.First passing entry wins.
[tuned]perms are generated and have no row to hangrequires=on, hence the inline form. A bare name is a one-entry chain, so every existing annotation behaves exactly as before.One source for the shipped fallback
That fact lived in three hand-maintained copies: the annotation,
report()'s baseline argument at 23 call sites, and akfallarray. Two were cosmetic;report()'s baseline is not — a sub-0.5% win reverts to it, and that decides what lands in the sidecar.[tuned]now banks each kernel's resolved fallback and[dasllama_fallbacks]emitsdasllama_tuned_fallbacks();report()looks it up.kfall,knamesandkwin— three parallel 25-entry arrays — are gone. All three copies agreed beforehand (verified across all 25 kernels), so this is a refactor with no behavioural delta.Also
gen_tune_probenever parsed--tune-fastalthoughdasllama_tunerpassed it to both halves and bannered "FAST". It now honours it,ROUNDS6 → 3. The confirm pass is deliberately untouched — a correctness gate, not a budget.daslib/clargs, replacing three hand-rolled argv scans.llvm_tuneparses an argv it does not own, so it uses the two-arg form (theReleaseDepsArgsprecedent in the same module) with a bool-only spec that never reads positionals.watchdog.pyconsumes the events intoSTATE["tune"]for the control page, logging only atplan/end: 26 records from 551 events, where the whole raw tuner output used to be JSON-wrapped line by line. Both load-bearing legacy strings are untouched, so old-watchdog/new-daslang and the reverse both still work.explicitis a reserved word (DAS_EXPLICIT) — hit while naming a local; added to CLAUDE.md's list, which hadlabel/expect/passbut not it.Verification
Measured on a real terminal at normal verbosity: 551 bar frames, per-variant chatter 0 (was ~500), checksum lines 0, sidecar JSON 0, 31 lines of tuner output total.
Pipe/pty matrix, end to end: pipe forwards 19 events and draws 0 bars; pty draws 19 and forwards 0;
silentdraws nothing on a pty yet still forwards all 19 under a pipe;verbosealone shows raw chatter.Real tuner runs:
tune_kernels --tune-fast→plan total=25, 25 begin/end, 500 steps splitting 100 screening / 100 narrowing / 300 finalists — exactly fast mode's 4/8/20, which also proves the clargs migration took effect.gen_tune_probe --tune-fast→plan total=7, per-familytotal=3(the 6 → 3 cut).Per-ISA chain proven three ways on a real kernel:
dotprodpresent picks the arm row; gated oni8mm(absent on M1) falls through to generic;DAS_JIT_ARM64_FORCE_FEATURES=i8mmpicks the arm row again.New tests:
llvm_tune_progress.das(10 cases, no tty needed),llvm_tune_requires.das(9, host-independent),tests/daslib/tty_test.das(4, AOT-registered).tests/language/cpu_supports.dasgains arm64 coverage and loses a comment this change made false.llvm_tune.daslint went 49 → 0 as part of this (8 realLINT016:errors :=on a plainstring&promised a clone it does not perform).Docs
skills/tune.md(what a tune shows + the event protocol),skills/llvm_tune.md,skills/environment_variables.md(DAS_TUNE_VERBOSITY),modules/dasLLAMA/tune_for_this_box.md,utils/watchdog/README.md, and das2rst wiring for the new module.history/compiler/TUNE_PROGRESS_UX.mdis the design record (archived on landing, perskills/doc_archiving.md).🤖 Generated with Claude Code