/ripwire-opt-remarks

name: ripwire-opt-remarks
audience: contributor
description: >
  Triage clang optimization remarks (-Rpass, -Rpass-missed, opt-record YAML) while editing ripwire's
  OWN C++: 'loop not vectorized', 'will not be inlined' — worth a diff, or is LTO/PGO the real fix?
  Contributor-only: about compiling this tool, never about running it.
allowed-tools: Bash, Read, Edit

Triaging clang optimization remarks in this repo

> Nearest neighbours: > • A measured slow operation, any codebase → **ripwire-perf-target**. Do that FIRST; remarks are the > second question, not the first. > • Maintenance risk / complexity in a subsystem → **ripwire-fresh-eyes**. Different axis entirely.

Full record of the pass this skill came from, with every number: **`docs/OPTREMARKS.md`**.

The one rule

**A remark is an observation, not a defect.** Clang is reporting a decision, usually the right one. The pass that produced this skill read ~1.1 M remarks over `src/`. **Zero of them justified a source edit.** Two justified a BUILD change (LTO, then PGO), and the best-looking source-level candidate was implemented, verified to do exactly what the remark asked, measured, and reverted. Budget your attention accordingly: on this codebase, the answer to a remark is far more often a compiler flag than a diff.

Step 0 — profile first, or you will optimize a millisecond

cmake -S . -B build_prof -DRIPWIRE_PROFILE=ON && cmake --build build_prof -j 6
./build_prof/ripwire . --no-cache 2>&1 >/dev/null | sed -n '/hottest scopes/,/PROF_TSV/p'

On this codebase the answer is stable and counterintuitive: **PageRank is ~1 ms** of a ~2.7 s cold CPU profile, the build-model sorts are ~1.7 ms, and ~29% sits in two phases now living in `src/ingest_sidecap.h` (`captureTagsFacts`, `captureSideFacts`) that spend it calling tree-sitter. So a remark in `src/pagerank.cpp`, `src/infra/sortutil.h` or `src/infra/radixSort.inl` is dismissible on arithmetic before you read it. Re-derive this table if you touch the pipeline; do not trust this paragraph forever.

Step 1 — collect

scripts/optremarks.sh --passes 'inline|loop-vectorize|slp-vectorizer|licm|gvn|.*unswitch|loop-idiom'
python3 scripts/optremarks.py --hot --top 40

`--hot` narrows to a literal list (`HOT_FILES`) rather than a heuristic, so what the report calls "hot" stays reviewable. That list once went stale silently — the ingest split moved the hot phases into `src/ingest_*.h` sections of the same TU, every listed path still existed, and `--hot` quietly fell to 0.2% coverage of that TU. `test/optremarkshotcheck.sh` now gates the list against the tree (`docs/OPTREMARKS.md` §8). If you split or rename a hot file, run that gate: it will tell you which files you now owe a decision on, and `COLD_FILES` is where a deliberate exclusion goes, with its reason.

Never in `build/` — CMake refuses it by name, because `build/ripwire` is the binary every gate and bench number is measured against. Never with a build type: `RIPWIRE_ARCH_FLAGS` is already `-O2`, and `Release` would define `NDEBUG` and blind the degrade-path gates. Run wide **once** to learn which classes fire (`src/main.cpp` unfiltered exceeds 800 MB of YAML), then narrow.

Step 2 — the remark classes, and what each one is worth here

Real signal

**`Missed inline NoDefinition`, callee in another TU that you own the build of.** *Meaning:* the definition is not visible, full stop — not a cost-model opinion. *Confirmed here:* 831 of 1,437 distinct `NoDefinition` sites in the ingest translation unit name a tree-sitter C accessor (`ts_node_child_by_field_name`, `ts_node_is_null`, `ts_node_type`, …), each a two-line function compiled into a separate C object, inside the phases that are ~29% of a cold run. *Fix pattern:* make the definition available — **link-time optimization**, not a source edit. *Measured:* `-DRIPWIRE_LTO=ON`, four interleaved A/Bs (9/21/21/31 runs per arm) → **cold 1–6% faster, warm 0–3%**, every cold statistic in every run favouring LTO; output byte-identical, build time +2.6×. **Now the default** (`-DRIPWIRE_LTO=OFF` for a fast edit loop). Note the shape of that claim: four runs put the cold median anywhere from −0.8% to −5.9%, so the honest number is the range and the *unanimous direction*, not the best run. *Tell it from noise:* if the callee is libc (`fopen`, `fread`, `stat`) the same remark is worthless — inlining a syscall wrapper saves nothing. The class is only interesting when the callee is **tiny and yours to build**.

**`Missed gvn LoadClobbered … clobbered by call`, in bulk, in one function.** *Meaning:* fires per load per pass; 90,971 of them in this record. Useless individually. *Fix pattern:* there isn't one for a single site. Its value is **aggregate** — a dense cluster in one function means that function is call-bound, and that is how the `NoDefinition` finding above was located. Read it as a heat map, never as a work item.

Restated algorithm — dismiss as a class

**`loop-vectorize` early-exit family:** `TooManyUncountableEarlyExits`, `PotentiallyFaultingEarlyExitLoop`, `CantComputeNumberOfIterations`, `UnsupportedUncountableLoop`, `LoopContainsUnsupportedSwitch`, `WritesInEarlyExitLoop`. *Confirmed here:* hundreds, concentrated in `src/docparse.h`, `src/arch.h`, `src/lexical.h`, `src/graph.h`, `src/infra/radixSort.inl:402`. Every one is a scanner that `break`s on a delimiter. A loop whose trip count depends on the bytes it is reading is uncountable **by construction**. There is nothing to fix without changing the algorithm.

**`Missed inline NeverInline` naming `__clang_call_terminate`.** 275 in the hot set. The exception-landing-pad helper; `noinline` is deliberate and it never runs on a hot path. Filter first, every time.

**`Missed loop-vectorize MissedDetails`.** Always paired with an `Analysis` row that gives the actual reason. Read the `Analysis` row; the `Missed` row carries no information of its own.

**`Missed gvn LoadClobbered … in favor of