mirror of
https://github.com/nim-lang/Nim.git
synced 2026-09-01 19:33:42 +00:00
3f014bbd371714af1cf80ec8ce50e727ee437eb3
8921 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
araq
|
3f014bbd37 |
IC: instrument nim m, and find 1.05s of interface stubs nobody reads
The frontend was the one phase with no timers inside it — 180 processes
and 9.9s of Atlas's cold build, entirely opaque. `tStage` around
`commandCheck` and `tWriteNif` around `writeNifModule` close that, and
with the loading slots ast2nif already had, a `nim m` process is now
fully accounted:
startup (exec+runtime+config) 0.17 s 2%
loading imported `.s.bif` 4.57 s 46%
writing this module's `.s.bif` 1.84 s 18%
sem + parse 3.41 s 34%
Two thirds of the frontend is artifact I/O, not compilation. That is the
per-module model's structural cost: 180 processes each rebuild their
imports' interfaces from nothing.
One part of it is not structural at all. `interfHidden` accounts for
1.05s of the loading: 1.70M hidden-symbol stubs against 0.29M exported
ones, built by every `nim m` for every module it imports. The table is
reached ONLY through `modulegraphs.interfSelect` with `optImportHidden`,
and that flag is set in exactly one place — an `import x {.all.}`. The
backend already skips building it for this reason; the frontend cannot
skip it unconditionally, but it does not have to build it eagerly either.
Measured with a probe rather than estimated: skipping it takes
`InterfTables` 1161ms -> 80ms, the frontend 9.98s -> 8.93s, and the whole
Atlas build 22.19s -> 20.47s wall.
Not taking it in this commit. The correct form is lazy population on
first `interfSelect(true)` — deciding up front cannot work, because a
macro-generated `{.all.}` import is invisible syntactically and guessing
wrong loses symbols silently. `loaderCtx` is the hook (the module index
survives in the DecodeContext), but this is symbol visibility, and it
deserves its own pass rather than the tail of a long one.
`bnode.nim` carries the numbers and the design note. All behind
`-d:icBNodeProf`. ic 41/41, `koch boot -d:release` equal executables.
|
||
|
araq
|
d680599038 |
IC: instrument the backend phases, and tag which process each line is
I claimed 19s of the Atlas backend's CPU was work no timer accounted
for. It is not, and the 19s was an arithmetic artifact: I subtracted a
serially-measured frontend and a separately-measured gcc from a
PARALLEL build's CPU total. Mixing the two is meaningless.
Measured properly, with new coarse slots — `Stage` for the whole stage
body (so `Process - Stage` is exec + runtime init + config replay +
graph setup) and per-stage `LowerOwned`/`LowerHooks`/`LowerWrite`,
`CgGen`/`CgInit`/`CgFinish`/`CgWrite` — Atlas at batch size 16,
serially:
stage procs process wall in stage startup
frontend 180 9.90 s 0.00 s 9.90 s
lower 14 3.55 s 3.50 s 0.04 s
cg 14 3.27 s 3.23 s 0.05 s
lower: LoadClosure 756ms Owned 917ms Hooks 4ms Write 1681ms
cg: LoadClosure 548ms Gen 1912ms Init 168ms Finish 169ms Write 464ms
So the backend's nim-side work is 6.8s, not 19s, and 6.7s of it is
inside the stage bodies with 0.09s of startup across 28 processes —
batching having already removed the per-process cost that used to
dominate. The two largest items are writing the `.t.bif` (1.68s) and
cg's demand-driven generation (1.91s, of which `genProcBody` is 0.6s).
The bulk of the build is elsewhere: the frontend's 9.9s and gcc's 12.2s.
The dump line now carries `stage=<name>`, and that is not cosmetic. A
`nim m` process arms the profiler through ast2nif but never enters a
backend stage, so untagged, those 180 frontend runs put their entire
runtime in the "startup" column — which is exactly the phantom that made
the 19s look plausible in the first place.
All of it is behind `-d:icBNodeProf` and compiles to nothing without it:
ic 41/41, `koch boot -d:release` equal executables.
|
||
|
araq
|
087c82f985 |
IC: two assumptions that only held while a cg process wrote one TU
Both are broken by batching, and both produce C that does not compile.
Found by pointing a batched `--ic:on` at Atlas; neither shows up on a
small synthetic target.
**A local's C name is cached on the PSym, its counter lives on the
BProc.** `fillLocalName` mints `i_1` and stores it on the symbol, taking
the suffix from `p.sigConflicts`. Emit the same routine into a second TU
and the new BProc's table has never seen `i`, while the body's locals
already carry names and skip the minting path entirely — so the next
local named `i` minted HERE also gets `i_1`. gcc: "redeclaration of
'i_1' with no linkage", in 64 of Atlas's 204 `.c` at batch size 4. So a
local that arrives already named claims its name in this BProc.
Note this is not purely a batching concern: `assignLocalVar` already
says inline procs "are regenerated for each module that uses them", which
is the same shape.
**`genProcPrototype` asserted its own routing.** Its IC branch emits
nothing for a dynlib proc, on the stated grounds that "findPendingModule
returns `m`, so symInDynamicLib follows this call" — true when a process
wrote one TU, false once a batch sibling can own the definition. The
demander then had neither the definition nor an extern for the `Dl_*`
pointer: "implicit declaration of function 'Dl_369101372_'" in net.nim's
OpenSSL calls. It now asks whether a sibling owns it, mirroring
`findPendingModule` rather than calling it — that proc creates a BModule
on demand, which a prototype has no business doing.
Atlas, 204 modules, cold, with both fixed — every batch size builds and
produces a working binary:
batch wall user sys CPU
1 11.51 s 52.29 s 16.39 s 68.7 s
4 10.34 s 41.53 s 10.93 s 52.5 s
16 10.21 s 36.01 s 8.48 s 44.5 s
32 10.52 s 33.66 s 8.01 s 41.7 s
CPU -39%, wall -11% and flat past 16 — the backend's 41 s of CPU for
4.8 s of wall was largely process startup and re-loading the same
dependency closure, and batching is what stops paying it. Duplication is
NOT what moved: 1.95x -> 1.90x, because `findPendingModule` routes to the
owner only when the owner is IN the batch and most demands are for
modules outside it. That 2x is a separate seam.
The default is still batch size 1, where both fixes are inert by
construction: 204 of 204 `.c` byte-identical to the previous commit.
ic 41/41, `koch boot -d:release` equal executables.
|
||
|
araq
|
bff0bc45fd |
IC: export s makes s importable even without a * on its declaration
`nim c --ic:on` could not compile anything that reached `std/tempfiles`:
lib/std/tempfiles.nim(115, 38) Error: type mismatch
Expression: initRand()
Expected one of: [1] proc initRand(seed: int64)
That is `std/random`. It declares `proc initRand(): Rand` WITHOUT a `*`
and then, forty lines later, `since (1, 5, 1): export initRand`.
Two mechanisms make a symbol importable and the writer only knew one.
`sfExported` is the `*` on the declaration; `semExport` instead calls
`reexportSym`, which adds the symbol to the module's interface table and
sets no flag. `writeSymDef` decided the NIF `x` marker — "importable as a
bare identifier" — from `sfExported` alone, so a symbol exported the
second way shipped as private and its importer reported an undeclared
identifier. Nothing about this is IC-specific in the source; it is
ordinary stdlib code that classic compilation accepts.
So ask the interface too: `reexportedLocalSyms` collects the symbols a
module defines that reached its interface without the flag, and they get
the marker. Symbols that are genuinely private still do not — the
regression test's sibling case (`proc g()`, never exported) is still
rejected under both `--ic:on` and `--ic:off`.
This was found by pointing `--ic:on` at Atlas, which is now the first
sizeable third-party program it builds. `nim c --ic:off` and `--ic:on`
produce the same working binary from the same 209-module closure.
ic 41/41 (the new test is the 41st), `koch boot -d:release` equal
executables.
|
||
|
araq
|
8299401888 |
IC: render every module's .c in one emit process
`emit` is the one backend stage with nothing to get wrong. It loads no module graph — it derives its output path rather than resolving it — and `renderCFromArtifact` filters text against the merge decision without touching an AST. It is a pure function of one `.c.nif` and one global artifact, so grouping the calls cannot change what comes out. Measured rather than assumed: 67 separate emit processes and one process with all 67 members produce byte-identical `.c`, in 0.502 s against 0.041 s. So emit takes a single nifmake rule covering every live module. Where that shows up is narrower than the 12x suggests. Cold serial 8.21 s -> 7.82 s; cold parallel 2.81 s -> 2.83 s, i.e. nothing, because 0.46 s spread over 16 cores is already invisible. An edit to `system.nim` is 2.15 s either way: emit was never what made that slow. The honest case is CPU rather than wall clock, plus one fewer thing to pay for on a machine that is not idle — the fire-all is structural (every `emit` re-fires whenever `merge` rewrites the decision, by design) and now costs one process start instead of 67. Also fixes a bug this exposed: `isMain` was a per-BATCH flag. That is the right question for `lower`/`cg`, where main loads the whole program and is never batched with anything, but emit batches freely — and main in a mixed batch had its `.c` path derived from its module SUFFIX instead of its source file, so it was never written at all. `-d:icBatchSize:N` still splits emit the old way, for comparing against the fan-out. ic 40/40, `koch boot -d:release` equal executables, and the 67 generated `.c` are byte-identical to the previous commit's. |
||
|
araq
|
5d77a71043 |
IC: stop baking a compile command the process cannot know
Every generated `.c` carries a "Command for C compiler" comment built
from `conf.compileOptions`. Under the per-module backend that is a lie,
and a non-deterministic one.
A global `{.passC.}` — system's `-pthread`, say — reaches
`conf.compileOptions` only in a process that compiled the module
declaring it, and a `cg` process sees one module's import closure. So
the command each `.c` records is a partial snapshot, and WHICH part
depends on how modules were grouped into processes. Measured on a
67-module program: 2 of 67 `.c` carried `-pthread` at batch size 1, 4 at
size 4, 5 at size 8 — against 16 of 16 for a whole-program `nim c`.
The object files were never affected: the `link` stage applies every
module's recorded directives (`replayer.applyBackendActions`) before
compiling anything. Only the comment was wrong. But it is also the reason
two `.c` files differed between batch sizes for no reason of their own,
which makes it noise in the one oracle that matters here — the final `.c`
set.
So under `--icBackendStage` the line says where the command actually
comes from instead of guessing at it. Whole-program `nim c` is untouched:
byte-identical `.c`, and it still prints the real command, which there it
genuinely knows.
ic 40/40, `koch boot -d:release` equal executables.
|
||
|
araq
|
e5bba5a0fe |
IC: let one backend process handle a batch of modules
The plumbing behind the routing fix. `--icBackendModules:<a,b,c>` gives
the lower/cg/emit stages a LIST, `loadDepClosure` loads the batch's union
closure once instead of once per module, each stage loops over its
members, and `deps.nim` emits one nifmake rule per batch declaring all
its members' outputs. `-d:icBatchSize:N` is the dial; `0` means one batch
per job.
The default is 1, which reproduces the per-module fan-out exactly: `.c`,
`.c.nif` AND `.t.bif` byte-identical to the previous commit, ic 40/40,
`koch boot -d:release` equal executables.
Turning the dial up found three real bugs, none of which could exist
while a process wrote one TU:
* `loadDepClosure` deduplicated batch members against `visited`, which
already contains system — so a batch whose member IS system loaded
nothing and produced no artifact at all. System is an ordinary live
node with its own `.t.bif`/`.c.nif`; members now have their own set.
* `cg` finished each member's TU as it went. `finishModule` closes a TU,
and a later member's codegen routes definitions INTO an earlier
member's TU, which then silently dropped them. Generate every member,
then finish every member.
* `emitsBodyInThisModule(m, prc)` was asked with the DEMANDING module
where it means the module the body goes INTO. Identical while
`findPendingModule` always returned `m`; with a batch the definition
was marked declared in its owner's TU and then emitted by nobody — 18
undefined symbols at link.
Where it stands at batch size 4 and 8: builds, links, runs correctly, and
CPU drops from 9.8 s to 7.2 s / 6.2 s on a 67-module program. But the
artifacts are NOT invariant along the dial — 16 of 67 `.c` differ at 4 —
and the divergence is of two kinds. Most are the recorded gcc command
comment: per-module `{.passC.}` flags leak between batch members through
`writeBackendActions`. The rest is real: the set of minted type-bound
hooks moves (four `=destroy`/`=trace`/RTTI hooks vanish, one appears),
because which process mints a hook decides who owns it and batching
changes that. Duplication is also still 2.19x, unchanged — at these
sizes most demands are still for modules outside the batch.
So the dial stays at 1 until that invariant holds. It exists to be
turned, and it now reports what it finds when you do.
|
||
|
araq
|
bad9d3b2bc |
IC: route a cg definition by who owns it, not by who asked
`findPendingModule` decides which translation unit a demanded definition is emitted into. Under `--icBackendStage:cg` it was `return m` with a `# TODO fixme` over it: the destination was whichever TU did the asking, because the stage was built around a single `--icBackendModule` and there was never a second answer to give. That hardcoding is what makes the backend inflexible to batching. It is not that codegen cannot place a definition with its owner — the branch four lines down already does exactly that, creating the `BModule` on demand — it is that the cg stage could not express "this process writes several modules' TUs", so the question never got asked. So ask it. `BModuleList.icEmitted` is the set of module positions this process writes a TU for, and routing consults it: an owner in the set gets its own definition (the ordinary whole-program routing, now reachable from cg), an owner outside it means the definition has nowhere else to go and is emitted here as well — today's emit-everywhere, which `merge` still deduplicates. Duplication therefore falls smoothly as the set grows rather than switching over at some threshold. The stage puts one module in the set, so this run is a no-op by construction: the owner is `m` and the branch returns what `return m` returned. Verified as one — a 67-module `--ic:on` build produces `.c` AND `.c.nif` byte-identical to the previous commit's, `testament cat ic` is 40/40, and `koch boot -d:release` reaches "executables are equal". A change that provably alters nothing needs a live-probe check, or it is indistinguishable from dead code. Temporarily adding `system`'s position to the set — a module these processes do NOT write — moved 12 generic instantiations (`addQuoted_i*`, `clamp_i*`) out of the TUs that demanded them and into a TU nobody wrote, failing the link with exactly those 12 undefined symbols and shrinking generated `.c.nif` volume by 4%. The routing is live; what it still lacks is the plumbing to write a TU per batch member, which is the next step. |
||
|
araq
|
98b1e0ab50 |
IC: own nifstreams here, and move the nimony pin to 1721aab3
`nifstreams` is the classic NIF streaming surface — global `pool`,
`PackedToken`, `next(s)` — that this compiler's IC modules (ast2nif,
deps, modulegraphs, pipelines) are written against. It lived in
`dist/nimony/src/lib` even though nimony's own code is under standing
orders never to import it, so nothing over there ever ran a line of it.
That is how it broke. Nimony's nifcore refactoring unified the token
kinds, and the adapter's `next` started returning `tagLitToken(...)` for
an opener — kind `TagLit`, not the `ParLe` its own header promises
structural scanners. Nothing in nimony noticed. Here, `deps.nim` walks
the import graph by testing exactly `t.kind == ParLe`, and the failure
has no error path: an opener it does not recognise is just an unknown
token, so it skips the subtree and reads on. Every `.deps.nif` still
"parsed"; the graph came out missing most of its edges. `nim c --ic:on`
then scheduled modules ahead of their imports and died on a `.s.bif`
nothing had written yet:
Error: nim m requires precompiled NIF for import: .../streams.nim
(expected: .../str6a47nh.s.bif)
`json`'s rule listed 5 of its 10 dependencies.
So the adapter moves into `compiler/`, where its one consumer lives, its
test runs, and its contract is ours to keep. `next` now emits a real
`ParLe`, with the tag id in the full 28-bit payload rather than
`TagLit`'s 9-bit field and a `tagId` shadowing nifpools' to match:
`globalTags` holds 355 tags before this compiler registers one of its
own dialect, and a 512-tag ceiling is not one this surface can live
under. The `when isMainModule` self-test states the promise; reinstating
`tagLitToken` fails it on the first assertion.
Everything the adapter adapts (nifpools, nifreader, lineinfos) still
comes from `dist/nimony` — only the adapter moved. With it here, the pin
can move to nimony master: `bif.load` there fills a loaded module's
pools with `addOrdered` instead of hashing every entry it just read back
in stored-id order.
Verified: `koch boot -d:release` reaches "executables are equal",
`testament cat ic` is 40/40, and a 67-module `--ic:on` build produces
`.c` files byte-identical to the old pin's. Interleaved on that build,
old pin 8.40/8.39/8.43 s vs new 8.17/8.14/8.14 s.
|
||
|
araq
|
b6248a0b80 |
IC: read an exported symbol's kind from its header, not by decoding it
`addReexportedEnumFields` forced every exported symbol through `loadSym` and
THEN asked whether it was a non-pure enum type. Almost none are: 34815 symbols
on a 68-module build, of which the enum handling wants the handful that are
types. A sym def is written `(sd <name> <marker> <kind> …)`, so the kind is
three tokens in and needs no decode at all.
addReexportedEnumFields 290ms -> 5ms
export branch 484ms -> 154ms
processTopLevel 1086ms -> 787ms
loadDepClosure 2126ms -> 1978ms
cold --ic:on build 8.69s -> 8.51s (baseline built alongside)
`peekSymKind` mirrors `loadSymFromCursor`'s walk and the two have to change
together, so it is graded rather than trusted. `-d:icPeekKindCheck` compares
every peek against the load it replaces: a full build is 34508 peeks, zero
disagreements, and sabotaging the peek to answer `skProc` where the def says
`skType` fires on the first symbol.
The FIRST sabotage did not fire, and that is the part worth recording. Dropping
a `skip` from the walk lands on a non-`TagLit`, which answers `skUnknown` — the
designed fallback, correct but slower — so the equality assertion never saw it.
A walk that had drifted out of step would therefore look exactly like a clean
run. So the check has a second half: `PeekFallback` counts how often the peek
cannot read the header and `-d:icBNodeProf` reports it beside `PeekKind`. It is
0, which is the claim that the walk is in step; an equality oracle alone could
not make it.
Verified: both configurations build; `tests/ic` 40/40; 67/67 generated `.c`
byte-identical, cursor still identical to `PNode`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
|
||
|
araq
|
4c6d7d2d20 |
IC: time the export branch's two halves, so the next lever is sized
`processTopLevel`'s `export` branch is 484ms of an 8.64s build, and the split matters for what to do about it: `resolveSym` is 118ms, `addReexportedEnumFields` is 290ms over 34815 exported symbols — 8.3us each. That cost is one line: `addReexportedEnumFields` calls `loadSym` to force the symbol out of `Partial` state and THEN asks whether it is a non-pure enum type. Almost none of them are, so nearly all of that is a full symbol decode done to answer a question the def's header already contains — a sym is written as `(sd <name> <marker> <kind> ...)`, so the kind is two tokens in. Not fixed here, deliberately. A peek would have to hand-walk that layout, and mis-stepping it misclassifies silently rather than failing; it is worth doing with an oracle beside it, the way `indexFromBif` was. Sized and recorded so the next person starts from a number instead of a hunch. Timers only — no behaviour change; verified 67/67 `.c` byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE |
||
|
araq
|
1a70426a5b |
IC: dispatch the top-level walk on a tag id, not on ~20 tag names
`processTopLevel` decided what a top-level node was with an `elif` chain of
about twenty `tagIs` calls, each of which resolves a tag NAME and compares
strings. The two commonest outcomes — a real statement, and `implementation` —
sit at the END of that chain, so the average node walked all of it. There are
1.46M such nodes on a 68-module build.
Resolved once per tag id into a `TopTag` and dispatched with a `case`. The memo
holds its `TagPool` by reference for the same reason `indexFromBif`'s and
`bnode`'s do: that is what keeps a freed pool from being replaced at the same
address and answering from the wrong table.
processTopLevel 1314ms -> 1071ms
loadDepClosure 2309ms -> 2126ms
cold --ic:on build 8.92s -> 8.64s
The last `elif` folded three conditions together (`LoadFullAst in flags` OR one
of let/var/pragma); as a `case` that splits into the three tags, which always
load, and `ttOther`, which loads only under `LoadFullAst`. Same truth table,
now visible.
A note on the measurement, because it nearly cost this change: the first timing
of it read 11.35s against a remembered 8.91s. Re-running BOTH binaries
back-to-back gave 8.914/8.931 for the old and 8.641/8.641 for the new — the
first number was a `koch temp` build still settling. Compare against a baseline
measured beside it, never against one from earlier.
Verified: both configurations build; `tests/ic` 40/40; 67/67 generated `.c`
byte-identical, cursor still identical to `PNode`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
|
||
|
araq
|
20a3375571 |
IC: a backend stage stops building the interface half it cannot read
`populateInterfaceTablesFromIndex` stubbed a `PSym` for every global symbol a
module declares and inserted it into `interf` and/or `interfHidden`. On a
68-module target that is 2.37M stubs across the build — and 2.04M of them go
into `interfHidden` alone, which a `cg`/`emit` stage has no way to reach.
`interfHidden` is selected exclusively by `modulegraphs.interfSelect`, and only
when `optImportHidden` is in the module's options. That flag is set in exactly
one place — `importer.importModuleAs`, i.e. during sem. A backend stage builds
its module symbols in `moduleFromNifFile` and never imports anything, so the
flag cannot be set and the table cannot be selected. Skipping the hidden-only
branch there is unobservable, not merely unlikely to be observed.
Exported symbols still go into both tables. That is 0.33M inserts against the
2.0M saved, and it leaves `interfHidden` a coherent view — a module with no
hidden symbols — rather than an empty one, should anything ever consult it.
interface tables 1401ms -> 863ms
loadDepClosure 2771ms -> 2309ms
cold --ic:on build 9.38s -> 8.91s
Skipping the tables WHOLESALE in a backend stage does not work, and the failure
is worth recording since it is the obvious next thought: the exported half is
what `magicsys.getSysSym` resolves through, so the stage dies with "system
module needs: pointer". Skipping them in the FRONTEND is worse still — `nim m`
loses `defined`.
Verified: both configurations build; `tests/ic` 40/40; 67/67 generated `.c`
byte-identical, cursor still identical to `PNode`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
|
||
|
araq
|
2605cd3213 |
IC: take the module index from the .bif instead of rescanning for it
`moduleId` threw away the index `bif.load` hands it and rebuilt an equivalent one by walking the module's ENTIRE token stream, allocating a string per `SymbolDef` — once per module, per backend process. That was 909ms of a 10.1s cold `--ic:on` build. Taking the carried index instead: 220ms, and the build drops to 9.4s. `bif.store` already builds that index in one forward traversal at write time (`bif.buildIndex`) and writes it into the file, and `bif.load` reads it back with `pos` already a TOKEN index of the declaration's enclosing tag — the very thing the rescan recomputed. The comment claiming the file's offsets are "meaningless once the file is parsed" was true of the older byte-offset `readEmbeddedIndex`; it stopped being true when `bif` started storing token positions. The two agree BY CONSTRUCTION, and the reason is worth stating because "the file has an index" would not be enough on its own: the writer filters with `bif.isGlobalSymbol(name, dottedSuffix)`, every `storeBif` call site passes `"." & extractModuleSuffix(path)` — the same suffix the reader forms — and the visibility rule is the same test on the same token. Checked rather than argued, all the same. The old rescan stays as `rescanPosIndex` behind `-d:icIndexCheck`, which compares the two entry by entry on every module load: a full build agrees exactly (11220 entries for the system module alone), and sabotaging the comparison makes it fire, so the clean run says something. `ensureSemBuf` had the same rescan for the `.s.bif` companion; it uses the carried index too. Verified: both configurations build; `tests/ic` 40/40; 67/67 generated `.c` byte-identical to before, and cursor still identical to `PNode`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE |
||
|
araq
|
dbaed3d38a |
IC: measure the whole backend, and record where its time actually is
The profiling added with the cursor path was local to `bnode`, so it could only
answer questions about the cursor path. Moved to `compiler/icprof.nim` — no
compiler imports, so any stage can use it without a cycle — and extended to the
stage boundaries: the closure load and its three phases, `transformBody`,
`handOffBody`, `genProcBody`.
The budget that produces, on a cold `--ic:on` build of a 68-module target
(10.1s wall, summed over 177 backend processes):
loadDepClosure 3306ms of which moduleId 1285ms
processTopLevel 1516ms
interface tbls 1323ms
genProcBody 333ms
handOffBody 60ms
transformBody 28ms
This is worth having written down because it reprices the migration this branch
is doing. Reading a routine body off a cursor rather than a tree is finished and
costs nothing — `genProcBody` is the same either way. But FINISHING the job,
reading a `.t.bif` body directly and never materialising the `PNode`, can only
win back `handOffBody` + `transformBody`: under 1% of the build. The 41% is in
getting the closure's INTERFACE into memory, which no amount of body-reading
touches.
The remaining blockers in `bnode`'s header — `TLoc.lode` above all, 72 call
sites and hard, because a symbol's `loc.lode` outlives the body it was built in
and `lode == nil` is a sentinel — are worth exactly that under 1% until
something else changes. Said so in the header, replacing the older 0.20s/0.16s
figures, since that paragraph is the map read first.
The obvious lever on the real cost was tried and is not taken:
`{SkipInterfaceTables}` for dep-of-a-dep loads in `loadDepClosure` builds and
runs correctly but returns ~200ms, because most of that phase is the target and
system modules rather than the transitive ones. Not worth a name that silently
fails to resolve, so the flag stays restricted to `loadTransitiveHooks`.
Verified: both configurations build; `tests/ic` 40/40; the instrumentation
changes no codegen — 67/67 `.c` identical, cursor still identical to `PNode`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
|
||
|
araq
|
727d70d2ba |
IC: keep the node bridge out of the nimKochBootstrap build
`koch boot` failed with `bodynav.nim(278) Error: undeclared identifier:
'program'`. `transf` imported `nodebridge` unconditionally, which pulls in
`bodynav`, which resolves names through `ast.program` — and `program` does not
EXIST under `-d:nimKochBootstrap`. That define disables the IC subsystem
wholesale: `ast.nim` guards `program` and the loader callbacks with it, and
`koch.bootic` records that it also disables `commandIc`. The bridge was being
compiled into a build with nothing for it to talk to.
Guarded the same way `ast.nim` already guards `program`. `handOffBody` goes
with it; its only caller is `cgen`, under `-d:newIcBackend`.
`koch temp` cannot catch this — it does not set the define — so the break
survived several rounds of verification that all used it. Predates this
branch's recent work: `transf`'s unconditional import and `bodynav`'s use of
`program` are both there at
|
||
|
araq
|
2db27d4826 |
Merge branch 'devel' into araq-ic-fixes2
One conflict, in `canRaiseDisp`, where both sides added to the same guard: * devel (#26145) short-circuits `skMethod` to "can raise", because a base method's inferred effects describe only the base body, not every vtable target; * this branch resets `markCanRaiseBranch` to 5 on entry, so that a `-d: icCanRaiseLog` differential attributes an answer to the branch that actually decided it rather than to whatever the previous call left behind. Both kept: the reset first, then devel's method branch, then the existing flags branch. Marker 5 already means "decided here, neither predicate ran", which is what the new branch does too, so it needs no new number — the comment now says both short-circuits land there. Verified on the merged tree: `tests/ic` 40/40, including devel's new `timportcalias`; the cursor-driven and `PNode`-driven backends still generate byte-identical C (67/67 under `--ic:on`); ccgbugs 146, concepts 48, method 22, destructor 97, arc 140, gc 78, closure 23, iter 71, exception 47, all clean. `tests/generics/tparser_generator.nim` fails, and does NOT come from this merge: it fails the same way on pristine `origin/devel`, built and run in a throwaway worktree to check. The compile succeeds; the spec expects no output and the compiler now emits `typed`-deprecation and unused-import warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE |
||
|
araq
|
d27e1e2712 |
IC: give the cursor path a way to be measured, and record what it cost
The last commit's finding took three refuted hypotheses to reach. The switches
that produced it are kept, and so is the answer, because the next person will
ask the same question:
* `-d:icBNodeProf` counts every accessor and times the phases (`handOffBody`,
`genProcBody`, the analyses, `sym`/`typ`/`info`/`origin`). Each backend
process appends a line to `$NIM_IC_BNODE_PROF`, so a parallel build still
produces attributable output. Costs nothing when the define is off.
* `-d:icBridgeOnly` builds the buffer but generates off the tree. It is a
MEASUREMENT switch, not a mode, and it is the only way to separate what the
encoder costs from what reading costs — which is how encoding was shown to
be free and the reader identified as the thing to profile.
Cursor-driven generation is now the same speed as tree-driven: over the 2420
routines of a 68-module target, `genProcBody` is 369ms off a cursor against
366ms off a `PNode`. It was 1877ms.
The three suspects that measured out as wrong are written down so they are not
re-guessed: the tag-name string compares in `typ`/`flags` (0.3% of the build),
`Cursor`'s reference-counting lifetime hooks (6M calls, 60ms of 1900ms), and
the structural accessors as a whole — `kind`, `son` and the iterators together
are 32ms of 1877ms.
The one real change here is the tag memo, which now resolves a tag's
`TNodeKind` and its wrapper role together instead of comparing tag NAMES on
every `typ`/`flags`/`hasExplicitNilType` call. Small — it is the 0.3% above —
but it removes 197k string comparisons and lets those three read a `case`.
`tagCachePool` holds the pool by reference for the same reason `readPool` does:
that is what stops a freed pool from being replaced at the same address.
Verified with the parent commit: `tests/ic` 39/39, grinder clean, `.c`
byte-identical both under `--ic:on` and without it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
|
||
|
araq
|
db47363784 |
IC: memoize the FileId -> FileIndex resolution, halving a cold --ic:on build
`oldLineInfo` resolved every decoded token's line info by copying a path out of the buffer's filename pool and hashing it through `msgs.fileInfoIdx`. Every time — no cache, on a step that runs once per node the decoder loads and once per statement the generator emits. On a 68-module target that is 259k calls at 5.2us. Memoized per pool it is 34ms, and the cold build goes from 21.5s to 10.1s. `revTab` could not serve this: it is keyed by a `FileId` in the WRITER's global `pool.files`, while a decoded token's `FileId` indexes the buffer's OWN filename pool — a fresh one per `bif`-loaded module. So the cache is keyed by (pool, FileId), as a `seq` because ids are small and dense within one pool. `readPool` holds a REFERENCE rather than a raw pointer, and that is the whole safety argument: it keeps the pool alive, so a freed pool cannot be replaced by a new one at the same address and answer from the wrong file table. Verified: `tests/ic` 39/39; the cursor-vs-`PNode` differential grinder clean over ~150k graded nodes on `tools/icgrind` — and confirmed live by sabotaging the pool-identity check, which makes it fire on `info` immediately; all 67 `.c` files an `--ic:on` build generates byte-identical to before; all 216 a non-IC build generates likewise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE |
||
|
Araq
|
8afd306b0d |
IC: correct the module docs to the state the seam is actually in, and measure it
`bnode`'s header still said the generator "has to move together, and it needs
write-side capability this seam does not have". It has moved, and `origin` was
the write-side answer, so the section described a state two commits out of date —
the kind of stale map that sends the next person looking for a problem that is
already solved. Rewritten to say what runs on a cursor now, that `origin` is what
keeps `TLoc.lode` a `PNode`, and that the generator's own in-place rewrites run
on the origin (with the hazard spelled out: where a mutation is read back,
generation has to continue on the origin, because the buffer is a snapshot).
Two blockers were also described wrongly:
* THE ALIAS FAMILY is no longer blocked by field identity — that is exact on a
bridged buffer. I checked whether `isPartOf` could migrate now and it cannot,
for a different reason: every call site passes `d.lode` as one operand and that
is a `PNode`, so a generic `isPartOf` would still be handed a `PNode` on one
side and buy nothing. It moves when `TLoc.lode` does.
* `sym`'s note said `isPartOf` "CANNOT be migrated as written" without
qualifying that this is a FILE-path property; a bridged buffer hands back the
object it was given, and the grinder asserts exactly that.
And the cost, which I flagged twice as unmeasured and is now measured on a
50-module target:
baseline (PNode) 6.75s
bridge built, not read 6.79s -- encoding is inside the noise
generator driven off it 8.85s -- +31%
So the encoder is as cheap as claimed and the whole cost is in READING: `son` is
O(i), `kind` indexes a memo per call, `sym`/`typ` go through the nav, `origin` is
a hash lookup per location built. None of it is inherent and none of it has been
optimised. Since a compile is mostly frontend, codegen itself is slowed by well
over 31%.
Verified: both configurations build; cursor-driven and `PNode`-driven output
still identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR
|
||
|
Araq
|
603953376d |
IC: add -d:icSymCount, and use it to settle the full-compiler comparison
Counts every `newSym` mint, by kind, and reports on exit. A gensym's number is its item id, so ONE extra symbol anywhere shifts every later name in the generated C — which makes a mint count far more sensitive than diffing output, and localises a difference by symbol kind instead of by whichever file happened to show it. It was written to answer a question I had reported wrongly. Comparing the cursor-driven and `PNode`-driven builds over the whole compiler showed 3 of 215 `.c` files differing, and I attributed 2 of them to the compiler being nondeterministic. That control was invalid: I had compared two runs across an edit to `ccgexprs.nim`, so what I read as nondeterminism was line numbers moving inside assertion strings. The compiler is DETERMINISTIC — same binary, same source, twice, 0 of 215 differ. What the count establishes: the two builds mint exactly the same symbols, 515_551 of them, with no per-kind difference at all. So nothing in the cursor path creates or skips a symbol, and the earlier gensym-numbered differences — which had zero non-gensym lines — were ordering, not extra work. On the current tree the comparison is clean: cursor-driven and `PNode`-driven produce BYTE-IDENTICAL `.c` for all 215 files of `compiler/nim.nim`. I have not isolated why the same comparison showed 3 differences one commit earlier; the commit between was behaviour-neutral by inspection, so I am recording the observation rather than a claim about its cause. Verified: both build configurations compile; the diagnostic is behind a define and costs nothing otherwise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
31a7b6bc85 |
IC: undo gratuitous rewrites the bulk migration made
Reading the previous commit's diff, as promised, turned up edits that were semantically neutral but said something false: a regex pass had rewritten `t.n == nil` (a TYPE's record tree) as `t.n.isNilNode`, done the same inside the grinder where both sides are `PNode`s, and relabelled `asgnComplexity` and `containsOpaqueImportcFieldAux` as `AnyNode` when both walk `PType.n` and can never see a cursor. It also edited a comment. None of it changed behaviour — `isNilNode` on a `PNode` is `== nil` — but it made unrelated code look like it participates in the seam, which is exactly the kind of noise that makes a later reader think the type-record walkers are migration candidates when the module doc says they are not. Reverted, and the two type-record procs now say why they stay. Verified: both builds compile; cursor-driven and `PNode`-driven output still identical (50/50). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
709fd00861 |
IC: migrate expr and the emitters — cgen generates from a cursor
`genProcBody` is handed `BNode(bodyBuf.rootCursor)` under `-d:newIcBackend`, so
`expr` and the ~160 procs under it read the routine body through a cursor rather
than a tree. This had to land as one change: `expr` dispatches to all of them, so
they move together or the dispatch converts at every node.
The evidence that it works is not that it compiles. Cursor-driven and
`PNode`-driven builds emit BYTE-IDENTICAL `.c` (50/50 on an 89k-line target,
12/12 on the grind target), the built program runs and prints the right thing,
and — the part that makes the first number mean something — sabotaging
`bnode.intVal` changes all 12 files. The generator is genuinely reading through
the cursor, not quietly falling back.
Four kinds of site could not simply take `AnyNode`, and each is marked where it
sits rather than left for the next person to rediscover:
* THE GENERATOR REWRITES. `mAppendSeqElem`, `mNewSeq`, `genSetLengthSeq`,
`genWasMoved` and `genArrToSeq` replace a child or a type IN PLACE, and
`genEnumToStr`/`mAsgn`/`spawn` build fresh trees. Those run on `origin(n)` —
the very node the buffer was encoded from — so the mutation lands exactly
where it always did. Where the mutation is then READ (`genArrToSeq` retypes a
bracket, `genArg` replaces a `var` param's type), generation continues on the
origin too, because the buffer does not see the write and a cursor would keep
reading the slot as encoded.
* NILABLE NODES stay `PNode`: a cursor has no standalone nil. That is the
assignment DESTINATION throughout the call family (`genCall` passes nil), the
`check` of an object-constructor field, `exvar`, `stepNode`, the `fin` of a
try statement.
* `PNode`-KEYED TABLES AND ANALYSES take `origin`: `dataCache`, `isPartOf`,
`lhsDoesAlias`, `potentialAlias`, the type-record walkers.
* SHARED PREDICATES in `ast.nim` cannot see `BNode`, so `skipHiddenAddr`,
`isInfixAs` and `getStr` join `canRaise`/`getInt` as templates instantiated
for both. `skipPragmaExpr` is a deliberate exception: it sits above the point
in `ast.nim` where `firstSon` for a `PNode` exists, so `bnode` carries a
one-line spelling with a pointer back.
Two Nim details worth recording. Repeated occurrences of a type class in one
signature share ONE implicit generic, so any proc whose two node parameters can
differ in representation needs explicit params — `genSingleVar`,
`genFieldObjConstr`, `callGlobalVarCppCtor`. And a `{.dirty.}` template inside a
generic resolves its identifiers at instantiation, so `genClosureCall`'s local
`rawProc` had to be bound before the template that uses it or it lost to the
module-level proc of the same name.
Verified: grind clean (1431 bodies, 260_431 nodes, 0 disagreements, origins
exact); the default path is byte-identical to HEAD; all four build
configurations compile.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR
|
||
|
Araq
|
e5bafa48c5 |
IC: make the location primitives representation-agnostic
`initLoc`, `fillLoc`, `putIntoDest` and `putDataIntoDest` take `AnyNode` and store `origin(lode)`. That is the gate the previous commit was for: 99 of the 180 generator procs build a `TLoc` from their node, so until these four accept a cursor none of those 99 could migrate, and with them accepting one they all can — `TLoc.lode` is still a `PNode`, and on a bridged buffer it is the SAME `PNode` a tree-driven build would have stored. `bnode.origin` is the ambient accessor (through `currentNav`, like `sym` and `typ`), so a generator proc does not have to be handed the buffer to build a location. It answers nil for a file-backed body, which is correct — those tokens came from no `PNode` — and asserts on a bridged one, where a node head always has an origin and a miss means the cursor is not where the caller thinks. The origin check in the grinder now goes through that ambient path rather than calling `originOf` directly, because the direct call is not the path the generator will take and testing it would have proved the wrong thing. Verified: grind clean, origins exact by reference at every node of 1431 bodies; the default path is byte-identical to HEAD; all four build configurations compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
9f34f5e42b |
IC: give the bridge origin tracking, so TLoc.lode stops blocking the generator
Measured before designing: of the 180 `PNode`-taking procs in `cgen`/`ccgexprs`/ `ccgstmts`/`ccgcalls`, **99 build a `TLoc` from a node**. So the generator cannot move to the seam without an answer for `TLoc.lode`, and the obvious answers are both bad — leaving it a `PNode` means a cursor-driven proc cannot fill it, and changing its representation means editing `TLoc`, which lives in `astdef` at the bottom of the module graph, pushing the seam far below the backend and forcing a flag day. There is a third answer, and nifcore already had the piece it needs. `cursorToPosition` is documented as a stable per-token key, and `TokenBuf.len` is where the next token lands — so the encoder records `position -> PNode` as it walks, and `originOf` inverts it. A cursor-driven generator can then put the ORIGINAL node into a location: `TLoc.lode` stays a `PNode`, and the identity comparisons already in the backend (`preventNrvo`'s `dest != le`, `isPartOf(d.lode, …)`) keep meaning what they meant, because it is the same object and not an equal copy. Asserted, not assumed: the bridge grinder now walks cursor and tree together and requires `originOf(c) == a` by REFERENCE at every node — 1431 bodies, 0 failures. Recording the position one token off makes it fail on the first body, so the check is not vacuous. This unblocks the generator migration without a flag day: `expr` and its ~60 emitters can move to `AnyNode` with `TLoc` untouched. Verified: grind clean (1431 bodies, 260_431 nodes, 0 disagreements); the default path is byte-identical to HEAD; all four build configurations compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
e988e0366c |
IC: hand the transformed body to cgen as a buffer, and read the analysis off it
`transf.handOffBody` is the seam between the two halves: rewriting ends, and from there the reading side works off a cursor. `cgen.genProcLvl3` takes the handoff once the body is final and runs `allPathsAsgnResult` — which has been `AnyNode` since the predicates landed — over the CURSOR rather than the `PNode`. Three things this had to get right, none of them obvious from the outside. WHERE the handoff goes. A bridged buffer is a SNAPSHOT, so it has to be taken after the last rewrite. Destructor injection runs after `transformBody` returns, and `easyResultAsgn` sets `nfPreventCg` on the tree later still — a buffer taken at the end of `transformBody` would describe a tree that no longer exists by the time anything read it. So the API lives in `transf`, which owns that invariant, and the call site is in `cgen`, which is where rewriting actually finishes. The one flag `easyResultAsgn` writes afterwards is read only by the `PNode` generator, and it sits in the branch that does not run the analysis at all. WHY it is gated on `-d:newIcBackend`. `expr` cannot migrate a piece at a time: it dispatches to ~60 emitters, so either all of them take `AnyNode` or the dispatch converts at every node. Until that happens the generator still needs a `PNode`, and building a buffer per routine in a default build would cost a tree walk and buy nothing. The analyses are the part that moves now. THAT IT IS ACTUALLY LOAD-BEARING. "Byte-identical output" is worth nothing if the new path never ran, so: cursor-driven and `PNode`-driven builds produce identical `.c` (50/50 on an 89k-line target, 12/12 on the grind target), and forcing the cursor-driven answer wrong changes 10 of 12 files. The first number alone would have been consistent with the code being dead. Verified: grind clean (1431 bodies, 260_431 nodes, 0 disagreements, neither tolerance taken); the default path is byte-identical to HEAD; all four build configurations compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
47144c7962 |
IC: a PNode <-> TokenBuf bridge, so rewrites stay on PNode
The generator does not migrate to a `Cursor` and should not: transf, destructor injection, closure lifting and the tree codegen builds as it goes all CONSTRUCT nodes, and a cursor is a read pointer into a shared token buffer. `nodebridge` is the seam instead — a rewriting pass keeps producing a `PNode`, and anything that only reads is handed a `TokenBuf`, from which a `Cursor` (and so a `BNode`) is a pointer. The bridge is NOT the `.bif` format, and the difference is the point. A `.bif` is read by a different process, so every symbol and type has to be written as a NAME to look up again. A bridged buffer is read by the process that built it, so a symbol reference is `(bsym <idx>)` into a side table holding the very `PSym` the encoder was handed, and the type slot is `(btyp <idx>)` the same way. The node shape is otherwise identical to the file format, so `bnode` reads a bridged buffer with the accessors it already has; `bodynav` grows one branch each in `symAt`/`typeAt`, and `bnode` learns that `bsym` is an `nkSym`. That buys the property this branch has been blocked on: **`sym` IS IDEMPOTENT ON A BRIDGED BUFFER, FIELDS INCLUDED.** On the file path it cannot be — `loadFieldStub` mints a fresh stub per use because two distinct fields can share a name and a position across types — which is what stops `aliases.isPartOf` moving to the seam. A bridge hands back the object it was given. The grinder now asserts exactly that: field syms are excluded from the idempotence check on the file path and INCLUDED on a bridged one. Verification is the existing oracle pointed at a harder target. `grindBNode` compares two decodings of one file and has to excuse two differences; the bridge is compared against its own live input and must excuse NEITHER, so both tolerances are counted and the bridge asserts it took neither. `toPNode` is covered without a hand-written comparator that could share the encoder's bugs: decode, RE-ENCODE, and grade the second buffer against the ORIGINAL tree, so anything the decoder drops shows up as a disagreement. Reach, measured: 1431 bodies and 260_431 nodes, against 782 and 67_857 for the file path — the bridge sees every body, including the one-line `nkAsgn` ones `ast2nif` never defers and the grinder therefore never saw. 0 disagreements. Not vacuous: dropping node flags, perturbing the sym index and dropping the type slot each make it fail immediately (`flags`, `sym identity`, `typ nil-ness`). A bridged buffer must never be written to a file — `(bsym …)` means nothing without the tables beside it. `ast2nif` remains the only serializer. Verified: 215/215 byte-identical `.c` against HEAD on the default path; all four build configurations compile, plus `nodebridge` standalone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
c1a815fa07 |
IC: take the last leaf readers, and map where the leaf migration ends
`skipAddr`, `skipAddrDeref` and `isInactiveDestructorCall` move to the seam — all three pure, none of them touching a field symbol. The node-returning ones are graded by a `checkNodeResult` template rather than a hand-written pair, so adding the next one is a line. The more useful half of this commit is the map. Counting rather than guessing: of the 191 `PNode`-taking procs across `cgen` and the `ccg*` files, 160 EMIT — they take a `Builder`/`TLoc` out-param or write into `p` directly. Those do not move one at a time. They are one mutual recursion rooted at `expr`/`genStmts`, so the generator moves as a unit or not at all, and doing that needs write-side capability the seam does not have. What is left over is not a backlog, it is six named blockers, and `bnode`'s module doc now says which proc each one holds up and why: * a type's RECORD TREE is not a body (`PType.n` stays `PNode` by design, so `asgnComplexity` and friends were never candidates); * RETURNS A NODE OR NIL (`getPragmaStmt` — no nil token exists to return); * WRITES TO THE NODE (`easyResultAsgn` sets `nfPreventCg`; the seam is read-only); * NEEDS RENDERING (`preventNrvo` interpolates `$le` into a warning); * NEEDS STABLE FIELD IDENTITY (`lhsDoesAlias`, `potentialAlias`, through `isPartOf` — see the `sym` note, this one is blocked on a property rather than on effort); * MIXED REPRESENTATION (`potentialAlias`, `getPotentialReads` carry a `seq[PNode]` beside the node). Stating it this way because the alternative is someone re-deriving each blocker by trying the migration and watching it fail, which is how three of the six were found. Verified: grind clean (67_857 nodes, 0 disagreements); 215/215 byte-identical `.c` against HEAD on the default path; all four build configurations compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
fe39ad5fba |
IC: sym is not a function of its argument for fields — find out, then say so
The goal was `aliases.isPartOf`, the deepest thing on the migration list: mutual recursion over two trees at once, reaching `sym`, `typ`, `intVal` and `isDeepConstExpr`. It does not migrate, and the reason is worth more than the migration would have been. `bnode.sym` IS NOT IDEMPOTENT for object fields. Two calls on the SAME token yield two different `skField` `PSym`s with consecutive item ids — field uses bypass the nav's memo and go to `loadFieldStub`, which mints per use because two distinct fields can share a name AND a position across types, so one shared stub would mistype one of them. `isPartOf` compares `a[1].sym.id != b[1].sym.id` to decide whether two accessor chains touch the same field, so on a cursor it answers `arNo` where the AST answers `arYes`: wrong alias analysis feeding NRVO and observable-store decisions. The grinder caught it on the first run after the migration, on a self-comparison `isPartOf(n, n)` — a shape production never passes, which is exactly why it was worth grading. The property was then confirmed directly rather than inferred: call `sym` twice on one token and print both, and the ids differ. So `aliases.nim` is reverted, and what stays is the knowledge: * `bnode.sym` says which symbols it is stable for and which it is not, what codegen actually consumes for a field instead (the name it re-navigates the reclist with, plus the position for tuples — the same tolerance the grinder applies), and why this module cannot fix it alone: a stable field identity needs the token's own position as a key, and `nifcore.Cursor` keeps that pointer private. * The grinder asserts idempotence for every NON-field sym at every node. It passes, and admitting fields makes it fail immediately — so the check states a precise boundary rather than a vague warning, and the day fields join it will be visible. `getInt` moves into a template so `bnode` can instantiate it (the `canRaiseImpl` pattern), and `astalgo.sameValue` becomes `AnyNode` and is graded — both are literal-only and unaffected by the field problem. `preventNrvo` records the OTHER kind of blocker while it is fresh: its alias analysis is fine, but the `warnObservableStores` message renders the node, and rendering is a capability the seam does not have at all. Verified: grind clean (67_857 nodes, 0 disagreements); 215/215 byte-identical `.c` against HEAD on the default path; all four build configurations compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
06b1bf8f9a |
IC: take the seam into trees and ccgutils
`trees.nim` can import `bnode` — nothing in `bnode`'s import closure reaches
`trees`, checked rather than assumed — so the shared helpers move to `AnyNode`
instead of being reimplemented behind the seam: `getMagic`, `whichPragma`,
`getRoot`, `isDeepConstExpr`, plus `ccgutils.stmtsContainPragma`. That unblocks
three more codegen procs, `canMove`, `notYetAlive` and `ifSwitchSplitPoint`,
which needed them and nothing else.
`stmtsContainPragma` could not simply stay `getPragmaStmt(n, w) != nil`, and
the reason is worth recording because it will recur: a proc that returns a node
OR NIL is the one shape the seam cannot serve. `.bif` spells a missing child as
a `DotToken` *inside* a tree; there is no nil token to hand back as a return
value and a `Cursor` is not nilable. So the predicate is split out — and,
because that leaves two copies of one traversal, `grindPredicates` now asserts
the two agree at every node instead of trusting them to.
Measuring the answers, not just the agreement, again earned its keep. Six of
the new checks came back with a wide spread (`getMagic` 7780 non-`mNone` over
many magics, `getRoot` 19506 non-nil syms compared by identity, `isDeepConstExpr`
7917 true, `notYetAlive` 9653 true). Two came back CONSTANT — `stmtsContainPragma`
false at all 67_721 nodes and `ifSwitchSplitPoint` zero at all 24 — because
nothing in the closure uses `{.linearScanEnd.}` or `{.computedGoto.}`. Both are
now exercised on both answers by shapes added to `tools/icgrind`. A check that
grades a constant is indistinguishable from a passing check in the output, so
this only shows up if the distribution is looked at.
Verified: grind clean over the whole `--ic:on` closure (67_857 nodes, 0
disagreements); the target's `--ic:on` output matches its `nim c` output;
215/215 byte-identical `.c` against HEAD on the default path; all four build
configurations compile.
Sabotaging `bnode.secondSon` — an accessor the lockstep walk does NOT itself
use, since it descends by index — is caught only by this layer, and is: it
fires on `getRoot`, `isDeepConstExpr`, `reifiedOpenArray` and
`skipTrivialIndirections`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR
|
||
|
Araq
|
837082eb89 |
IC: grade every migrated predicate at every node, and migrate nine more
Nine `PNode`-typed codegen procs become `AnyNode`, all pure readers: `bodyCanRaise` (the consumer of the `canRaise` work), `isAssignedImmediately`, `branchHasTooBigRange`, `hasNoInit`, `reifiedOpenArray`, `skipTrivialIndirections`, `isSimpleExpr`, `isConstClosure` and `fewCmps`. The signatures are the small part. A migrated proc that nothing calls with a `BNode` is not even type-checked, so the substance is `grindPredicates`: every one of them runs on BOTH spellings of the SAME node, at EVERY node of every graded body, inside the walk `grindLockstep` was already doing. 67_721 nodes on the reference target, 0 disagreements. Two things had to be gated, and neither by widening a guard until the run went green. The tolerated `(ht . <sym>)` type difference is benign for the vocabulary check and NOT benign for a predicate that reads `typ` — and because the predicates recurse, one excused node poisons every ancestor's answer too. `grindLockstep` now reports whether a subtree is free of it, and only clean subtrees are graded. `isAssignedImmediately` and `fewCmps` hand `n.typ` to `getSize`/`mapType`, which are total only over types the C backend can lay out. Asked at an arbitrary node they meet a `tyGenericParam` or a `tyAnything` and abort — that is a question with no answer in either spelling, not a disagreement between them. Both are graded FROM THE PARENT, at the position production calls them from. Declarative subtrees are skipped for the same reason, along the boundary `bodyCanRaise` already draws. Both exclusions are counted and printed beside the graded count, so a run that grades nothing cannot pass for a run that grades everything. `tools/icgrind` versions the grind target, because two ways of silently getting no coverage turned up while writing it: the main module's routines are never graded, and `ast2nif` defers only `nkStmtList` bodies, so a one-line `proc f(x: int): int = case x ...` is invisible to the oracle. Shapes added for `branchHasTooBigRange` and `fewCmps` produced exactly zero coverage until both were found by counting rather than assumed. Verified: 215/215 byte-identical `.c` against HEAD on the default path; sabotaging `isAtom` and sabotaging the parent-driven node selection each make the grinder fail on the first body it reaches. Recorded rather than papered over: `isConstClosure` is graded only on its false side — the whole closure contains one `nkClosure` node. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
512d2a8f26 |
IC: revert the (ht . <sym>) lazy-type pin — it broke sem
The previous commit made `(ht . <sym>)` — a sym node the writer gave an
EXPLICITLY nil type — load back with `nfLazyType`, so `ast.typ` answered
`sym.typ` instead of nil. The stated reason was to remove a load-order
dependence, and the direction was wrong: that nil is load-bearing.
`writeSymNode` only emits the wrapper when the node's own type DIFFERED from
its symbol's, so a nil there says the node genuinely had no type while the
symbol had one. A type symbol used as a VALUE is exactly that shape:
`newException(KeyError, ...)` passes a typedesc, whose node carries no type
while the symbol carries the object type. Handing it `sym.typ` makes sem read
the typedesc as an expression of the type it denotes, and `--ic:on`
compilation of anything instantiating `tables.[]` dies with "only a 'ref
object' can be raised". A four-line program is enough:
import std/tables
var t = initTable[string, int]()
t["a"] = 1
echo t["a"]
The load-order dependence is real but is not fixed by pinning the flag EITHER
way — setting it breaks sem as above, clearing it would strip the fallback
from the not-yet-loaded-stub population that `nifcBackendActive` exists to
serve. Left alone deliberately, with the reasoning recorded at the site.
`bnode.typ` answers the faithful nil, and the grinder excludes this one shape
via `hasExplicitNilType` — narrowly, only when the cursor says nil and the AST
is saying exactly the symbol's type.
Why the suite did not catch it: `tests/ic` passed 39/39 throughout. The same
four-line program reproduces from the scratchpad and from the repo root, and
PASSES under `tests/ic` — `--skipParentCfg --skipProjCfg` makes it fail there
too, so `tests/config.nims` is what masks it, most plausibly because
evaluating a NimScript config runs the VM and perturbs the very load order the
bug depends on. A test file under `tests/` therefore cannot guard this class,
and no test is added rather than one that passes on the buggy compiler.
Also in this commit, and the reason the bug was found at all:
* `effectsOf` / `raisesNothing` replace the raw subscripting of `fn.typ.n` in
`canRaiseImpl`, so the effect-list layout is written down in one place and
the templates carry no knowledge of it. `raisesNothing` is stated as the
NEGATIVE on purpose — the safe default is "can raise", so the one narrow
shape that licenses dropping an exception check is the one spelled out, and
an unanticipated shape falls conservative by construction.
* `-d:icCanRaiseLog` logs every `canRaiseDisp` verdict keyed by name, disamb
and OWNING MODULE, with the deciding branch. What "the canRaise helpers work
on a `.bif`" means is that the type the decoder materialises carries the same
effect list the from-source one did — a claim about the WRITER that the
BNode/PNode grinder structurally cannot make, since both spellings ask the
same `PType` and agree however wrong it is. The only oracle is the same
program built without IC: 234 callees comparable, 0 disagreeing, 23 of them
reaching the effect-list branch in both builds.
Two instrumentation bugs worth recording, because both produced confident
wrong numbers first: keying by name+disamb alone collided (`len.0` names a
different routine per module) and reported one false disagreement; and the
branch marker was a global that `canRaiseDisp` left stale on its early
return, which inflated effect-list coverage from 23 to a claimed 142.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR
|
||
|
Araq
|
5799220c98 |
IC: grind the Cursor vocabulary against the PNode loader, and fix what it found
The `BNode` accessors had no oracle. The self-test in `bnode.nim` checks them against each other and against the raw token stream, which a uniformly wrong vocabulary satisfies — it passed 5.2M assertions on an off-by-two model. This adds the missing oracle and lets it drive the next migration step. `cgen.grindLockstep` (opt-in, `NIM_IC_BNODE_GRIND=1` on an `--ic:on` build) walks the `.bif` cursor and the materialised `PNode` for the SAME body side by side and requires `kind`, `len`, `info`, `flags`, the literal payloads, `sym` and `typ` to agree at every node of every routine body in the dependency closure. The `PNode` is the oracle, so it compares everything rather than what someone thought to check. It found two bugs, both silent: * `BNode.typ` answered `nil` for every bare `Symbol`. `ast.typ` does not: it falls back to `n.sym.typ` when the loader set `nfLazyType`, which it does for exactly that shape. The consequence is not less information but a DIFFERENT answer — `canRaise` asks `fn.typ.kind == tyProc` about a call's callee, so nil turns "this call can raise" into "it cannot" and drops the goto-exception check after the call. * `(ht . <sym>)` — an explicitly nil node type — made `n.typ` LOAD-ORDER DEPENDENT in the loader itself. `newSymNode` marks the node lazy only if the symbol was still an unloaded stub at that moment, so the same `.bif` node answered `sym.typ` or `nil` depending on what happened to touch that symbol first. Pinned to the lazy reading, so the answer is a property of the file rather than of the traversal order. With `typ` correct, the blocker recorded in `allPathsAsgnResult` is gone. `ast.canRaise`/`canRaiseConservative` cannot become `AnyNode` procs where they live — `BNode` is defined in `bnode.nim`, which imports `ast` — so their bodies move into templates that `bnode` instantiates for its own node type. One source of truth, no cycle, no second copy. `ccgcalls.canRaiseDisp` and `cgen.allPathsAsgnResult` follow. Adds the leaf accessors (`intVal`, `floatVal`, `strVal`, `ident`, `flags`) because nothing in `ccgexprs` can migrate without them, and `rawDesc` for diagnosing a disagreement in terms of what the token stream literally says. `compiler/bodynav.nim` replaces the `BodyScope` snapshot with a scope chain the traversal maintains — `openScope`/`closeScope`/`registerDefHere`, lookup falling through to the decoder — ported from Nimony's `typenav`. The scope becomes a product of the walk, so nothing is copied ahead of time and nothing can be stale. How much of a live problem the snapshot was is measured rather than assumed: `-d:icLocalSymStats` reports `localHit=0 fieldStub=2 miss=0 sdReg=5902 extractReg=45` over the stdlib closure, i.e. definitions register constantly and not one use ever resolved through the table, because `isLocalSym` is a hardwired `false`. The hazard was latent; this keeps it latent once that stops being true. Verified: 0 disagreements over the whole `--ic:on` closure with the walk driving the nav; deliberately breaking `intVal`, `flags` and the nav key each make the grinder fire on the first bodies it reaches, so the clean run is not vacuous; the default path emits 215/215 byte-identical `.c` against the pre-change compiler and does not compile `bodynav` at all; `tests/ic` 39/39. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Ryan McConnell
|
8cb406cd7a |
Fix 26144; exception propagation for non-raising virtual methods (#26145)
ref #26144 The C backend must not use `sfNeverRaises` to remove exception checks from virtual method calls. The flag describes only the selected base method body, while a vtable override may raise a catchable exception. This change makes `canRaiseDisp` conservative for `skMethod` symbols and adds a regression test covering an exception raised by a child method invoked through a base reference. |
||
|
ringabout
|
802bcf5a2d |
fixes #26132; =destroy should accept non-parametrized generic (#26142)
fixes #26132 |
||
|
araq
|
2447dfdc7d |
cgen: name the PType child instead of subscripting it
The same treatment the `PNode` side just got, for the reason that applies to types: `t[0]` is the return type, the base class, the index type or the generic head depending on the kind, and the subscript says none of that. Every child access in the cgen files that has a named accessor now uses it — `baseClass` for the eleven object-hierarchy walks, `elementType` for the seq/openArray element, `returnType`, `genericHead`, `firstGenericParam` — and the two loops that walked a type's children become `paramTypes` and `kids`. Left indexed on purpose: a parameter reached by ARGUMENT position (`typ[i]` in ccgcalls/ccgstmts), a tuple field, and a generic parameter at an explicit index. There the index is the clearest thing to write. Every substitution is exact rather than merely close. `[]` with index 0 is unconditionally `sonsImpl[0]`, so `baseClass`/`returnType`/`genericHead` cannot diverge; `elementType` is `sonsImpl[^1]` and is used only where the type has a single son; `paramTypes` and `kids` are literally the loops they replace. `ast.sons(t: PType)` gets the warning it has been missing. Despite the name it is not the counterpart of the `sons` ITERATOR over a `PNode`: it returns the raw seq, and a `tyProc` keeps its parameter types in `n`, so that seq holds only the return type while `[]`/`len`/`kids` route parameters through `n[i].sym.typ`. `for x in t.sons` therefore compiles, reads exactly like the `PNode` idiom, and visits a different set of types — which is what `ccgutils.encodeType` would have started doing had it been converted to `sons` rather than `kids`. Marking the proc deprecated and rebuilding shows one call site in the whole compiler (`previouslyInferred`), so the trap is latent, not active. `bnode.nim` also records that there is deliberately no `BType` beside `BNode`: types stay `PType`s under `newIcBackend` — `BNode.typ` returns one — because the backend asks them questions (`skipTypes`, `getSize`, `lengthOrd`, the record walk over `t.n`) that a raw cursor cannot answer. Pure refactor: all 216 generated `.c` files byte-identical to the parent commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE |
||
|
araq
|
437876efbd |
IC: say what BNode.sym/typ/info actually need
The stubs guessed. Checking `ast2nif` shows all three need a resolution
context and so cannot stay unary accessors on the cursor:
* a `Symbol` token holds only a NAME; `loadSymStub` resolves it from a
`DecodeContext` plus the owning module plus that routine body's `localSyms`,
because an unsuffixed name is body-local and is in no index;
* `createTypeStub` likewise takes a `DecodeContext`;
* `rawLineInfo` yields a `NifLineInfo` in the `.bif`'s own file pool, which
`oldLineInfo` maps to a `TLineInfo` through a `LineInfoWriter` holding the
`ConfigRef`.
So the open question is not how to write these three but where codegen gets
that context — a parameter, or module-global state for the span of one module's
`cg` stage, as `ast2nif` already keeps for its writer. Recorded so it is not
rediscovered.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
|
||
|
araq
|
20b70c8b1e |
IC: implement the Cursor half of the codegen vocabulary
`BNode`'s `Cursor` branch stopped at `{.error.}` stubs. Everything structural
now works, so `-d:newIcBackend` no longer fails on the shape of the tree — the
remaining stubs (`sym`, `typ`, `info`) are the three that need the decoder's
symbol/type/line-info maps, which is the honest next boundary.
Two corrections to the seam, both found by looking at what actually reads a
`.bif`:
* it was pointed at `nifcursors`, the WRITER cursor over `PackedToken`s.
`bif.load` produces a `nifcore.TokenBuf` and `ast2nif` decodes it with a
`nifcore.Cursor`; that is the type the backend will get.
* the cost model said reading child `i` is O(size of children 0..<i). It is
O(i): a `TagLit` token stores the width of its whole subtree, so
`nifcore.skip` is one pointer add no matter how big the subtree is. Indexed
loops are still quadratic and still worth removing, but in the number of
children, not in tree size.
`kind` is the accessor the stub called pivotal, and the note on it was wrong in
a way that matters: a `.bif` carries its OWN tag pool, so a tag id means
nothing outside its file and the "build a tag-id -> TNodeKind table once" plan
cannot work. It memoizes per pool and drops the memo when the pool changes.
Since no call site executes any of this yet, `when isMainModule` walks real
`.bif` files and checks the vocabulary against itself and against an uncached
tag lookup. Over 70 files (`.s.bif`, `.t.bif`, `.iface.bif`) from the
testworkspace corpus: 2.05M nodes, 9.19M assertions, all passing. Both halves
of the harness were confirmed live by sabotage — `secondSon` returning child 2
trips it, and so does dropping the tag-pool memo invalidation, which is what
proves ids really do differ between files.
The default build is untouched: all 216 generated `.c` files still byte-
identical to the parent commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
|
||
|
araq
|
9e076d74c0 |
cgen: no [] on a PNode outside tree construction
Completes the vocabulary migration started with `BNode`: every child READ in
the cgen files now goes through an accessor that a `.bif` `Cursor` can also
serve, so flipping `newIcBackend` is a matter of implementing the vocabulary
rather than rewriting call sites.
* constant indices -> `firstSon` / `secondSon` / `lastSon` / `son(n, k)`,
including the `namePos`/`paramsPos`/`bodyPos`/... slot reads;
* indexed loops -> `sons` / `sonsFrom` / `sonsButLast` and the index-yielding
`isons` / `isonsButLast`. `for i in 0..<n.len: n[i]` is quadratic once
`BNode` is a `Cursor`, because reaching child `i` costs one `skip` per
preceding SUBTREE;
* `n.len == 0` / `> 0` on a node -> `hasSons`, which does not count.
`astdef` gains `sonsButLast(n, count)` and `isonsButLast` — the
`nkOfBranch`/`nkExceptBranch` shape, whose last child is the branch body, and
with `count = 2` the `nkVarTuple`/`nkIdentDefs` shape.
Three places needed more than a rename:
* the C++/goto/setjmp try generators re-subscripted `t[i]` up to ten times
per iteration of their `while i < t.len` walk; the branch node is now
hoisted once per step;
* `genParams` scans the arguments BACKWARDS to decide which need a temporary,
which a `Cursor` cannot do at all. It materializes them in one forward pass
and indexes that — the same order of work, since `needTmp` already
allocates per call;
* loops that stop at a computed position (`casePos`, `until`, `splitPoint`)
walk forward and break instead of counting up to the bound.
What is left is exactly what a `Cursor` backend will not do: writes that build
a fresh `nkProcDef`, and subscripts of a `PType`, `string`, `seq` or `Table`.
`bnode.nim` records the invariant and the `PType` trap — `ast.sons(t: PType)`
is a proc returning `var TTypeSeq`, not the iterator of the same name — which
the type checker enforces, since the `firstSon`/`secondSon`/`lastSon`/`son`
family exists for `PNode` only.
Pure refactor, verified as one: all 216 generated `.c` files byte-identical to
the parent commit; metamorphic IC 16/16; icSuite 19/19 fragments; categories
gc 78, arc 140, destructor 97, closure 23, iter 71, trmacros 6, cpp 50,
exception 47, casestmt 16 with one pre-existing environmental failure
(tests/cpp/tasync_cpp.nim: `cannot open file: jester`).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
|
||
|
araq
|
8d4ddd5516 |
IC: introduce BNode, the seam for running codegen off a .bif Cursor
Building `PNode` trees out of a module's `.bif` is the bulk of the `lower` and
`cg` stages: their cost tracks the size of the dependency CLOSURE a stage loads,
not the module it compiles (a 370-byte module costs 0.20s/0.16s in lower/cg, the
main module 3.40s/3.16s). Since the link stage stopped loading the graph those
two are ~85% of the serial backend critical path. The goal is to let the backend
read the `.bif` directly through a `Cursor`, constructing no `PNode` at all.
`BNode` is the seam: a `Cursor` with `-d:newIcBackend`, a `PNode` without, which
is what every build does today. Codegen migrates one area at a time and the
compiler keeps building throughout.
The seam is thin on purpose. `ast`/`astdef` already give `PNode` the whole
vocabulary — `kind`, `len`, `safeLen`, `sym`, `typ`, `info`, `firstSon`,
`secondSon`, `lastSon` and `sons`/`isons`/`sonsFrom` — so bnode deliberately does
NOT redefine them for `PNode`: an identical second overload makes every call site
ambiguous (tried it; `lastSon` breaks first). It adds only `son` and `hasSons`,
and supplies the full vocabulary on the `Cursor` side as `{.error.}` stubs, so
flipping the define names the exact missing accessor AT ITS CALL SITE instead of
collapsing into a cascade of type errors.
Two notes for whoever continues this:
* The migration front is SIGNATURES, not call sites. Converting `n[i]` to the
iterator vocabulary changes nothing while the value is still `PNode`-typed —
flipping the define then reports a plain type mismatch. Changing one proc's
parameter from `PNode` to `BNode` is what moves it, and is a no-op with the
define off. `containsResult` and `allPathsAsgnResult` are converted as the
worked example.
* `BNode`'s cost model differs and the vocabulary is shaped around it: reading
child `i` of a `Cursor` is O(size of children 0..<i), so prefer `sons`/
`sonsFrom`/`isons` over indexing, and avoid `len` in a loop condition.
Note `type BNode = when defined(x): Cursor else: PNode` does not parse — `when`
is not an expression in type position; it needs a `when` block over two `type`
sections.
Inert: all 219 generated `.c` files and the linked binary are byte-identical to
the parent commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
araq
|
f84f53bff4 |
cgen: replace indexed child loops with the sons/isons/sonsFrom iterators
`for i in k..<n.len: ... n[i] ...` is the dominant shape for walking a `PNode`'s children in the code generator: 41 such loops across the cgen files, and 777 indexed node accesses in total. It reads worse than iterating, it bounds-checks every subscript, and it is quadratic the moment the backend reads children off a NIF `Cursor` rather than a materialised tree (a child is `firstSon` plus one `skip` per preceding sibling, and `skip` steps over a whole subtree). 31 of the 41 are converted: * 20 to `sons`/`sonsFrom` — the index only ever subscripted `n`. * 9 to `isons`, which now takes a `start` index (defaulting to 0, so its eight existing call sites are unchanged). These genuinely need `i`: a parallel index into the routine's `PType` (`typ.n[i]`, `typ[i]`), a `needTmp[i-1]` lookup, an `i == field.position` test, `$i` in a generated struct name, or the index passed straight to `genOtherArg`. * 2 to `sonsFrom` with a variable start (`firstParam`, `offset`). `sonsFrom` is new, next to `sons`/`isons` in astdef. The remaining 10 are deliberate. Eight are not `PNode` at all — `varargs[Snippet]`, `seq[PSym]`, `string`, and `PType`, where `sons` is a `proc ...: var TTypeSeq` rather than an iterator, so a blind rewrite would compile into something quite different. Two iterate `0..<it.len-1`, excluding the last child, which no iterator expresses cleanly. Pure refactor, and verified as one: all 219 generated `.c` files of a 219-module program and the linked binary are byte-identical to the parent commit. That is the bar that matters here, because the index arithmetic (`i-1`, `i == position`, `$i`) is the easy thing to get wrong. It also caught a real slip on the way: `genFieldCheck` reassigns its loop variable, which a `for` binding cannot do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
araq
|
e0c0724b62 |
IC: the link stage no longer loads the module graph
`--icBackendStage:link` called `loadBackendModules` — a whole-program deserialization — for exactly two things: each module's `.c` path via `getCFile`, and its recorded C compile/link directives via `replayBackendActions`. That was 3.7s of the ~11s serial backend critical path on a 219-module program, spent recovering a list of paths and a handful of strings. Both now come from artifacts the earlier stages already produce: * `.c` paths from the driver's existing `LiveModulesFile` manifest (which `merge` already reads); the `.c` sits beside each listed `.c.nif`, so no new manifest was needed. The merge-decision fallback for conditionally-imported nodes that own a live symbol is kept. * C directives from a new `<module>.c.cflags` sidecar. The module's own `cg` already replays them, so it writes them down too — one tab-separated line per `compile`/`link`/`passl`/`passc`/`localpassc`/`cppdefine` action. `localpassc` needs the module's source path, which only the writer can resolve, so it is baked into the line. Written unconditionally, empty included: it is a declared nifmake output of the `cg` rule and a missing output re-fires the rule for ever. link: 3.70s -> 0.44s. A one-line code edit on the corpus goes 15.2s -> 11.8s; cold 124s -> 120s; no-op unchanged at 0.27s. Green: 16/16 metamorphic IC tests, the 17-file `koch ic` suite, 13/13 differential edit checks against `nim c`, and `bootic` iteration 1 (the full fixed-point check was still running when this was committed). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
araq
|
d81e764f98 |
IC: the cg stage must not write emit's .c
`registerModuleCode` writes the module's `.c` and registers it for compilation.
Under `--icBackendStage:cg` it must do neither: `cg`'s product is the `.c.nif`,
the `.c` is rendered by `emit` from that artifact plus the GLOBAL merge
decision, and the compile registration belongs to `link`.
Writing it in `cg` puts a second, differently-filtered `.c` — `cg` can only
filter by the liveness its own process sees — at the exact path `emit` declares
as its nifmake output. Two stages claim one output, and the `.c` ends up newer
than `emit`'s own `.c.nif` input, so nifmake considers `emit` up to date and
skips it: `cg`'s unfiltered text goes to the linker and every emit-everywhere
body is defined twice ("multiple definition of eqdup__u727466987__OOZ…").
Nothing surfaces this today because `merge` rewrites the decision file on every
run and every `emit` lists it as an input, so all of them re-fire and overwrite
the stray file. The fire-all is therefore load-bearing, not the "insurance" it
is documented as — and it is what makes a content-stable merge decision
(219 fewer processes and ~13s less backend CPU per edit on a 219-module
program) impossible: that experiment is exactly how this was found.
No behaviour change on its own, since the fire-all still runs. Green: `bootic`
fixed point, 16/16 metamorphic IC tests, the 17-file `koch ic` suite, and 13/13
differential edit checks against `nim c`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Ryan McConnell
|
33ee586913 |
fixes #11797; fix C type hashes for imported aliases (#26150)
Fixes #11797. Imported scalar and pointer aliases inherit their external C spelling, but receive a different Nim symbol. Signature hashing previously used that symbol identity, so aliases that emit exactly the same C type could produce different backend names for tuples, sequences, and other generic types. For example, `cint` and `type CIntAlias = cint` both emit `int`, but `seq[cint]` and `seq[CIntAlias]` could be emitted as incompatible C structs. The Nim type checker nevertheless permits assignments and calls between them, causing the generated C or C++ compilation to fail. This changes the backend hash to use the external type spelling when available. A symbol-based fallback remains for imported types without a resolved spelling. The change deliberately does not collapse imported types into their underlying Nim builtin. Types such as `pid_t`, imported pointers with qualifiers, and platform typedefs may require distinct backend representations. ## NIF and incremental compilation This does not change NIF serialization, NIF type keys, or the IC cache format. The bug is in backend type-name generation. An IC regression test is included to ensure that the corrected backend identity is preserved when compilation passes through the NIF pipeline. ## Tests The regressions cover: - tuple and sequence assignments between an imported type and its alias - cross-module sequence parameters and mutation - C and C++ backends - NIF-backed incremental compilation Existing C-type tests were also run under C/C++, refc, and ARC. ## Remaining scope This does not solve the broader question of compatibility between imported and builtin types that have different backend identities, such as `seq[cdouble]` and `seq[float]`. That remains tracked by #19374. |
||
|
Ryan McConnell
|
0be9b4f3f6 |
fix 26147; new-style concepts: broken generic (Case B) (#26151)
ref #26147 |
||
|
ringabout
|
c36c527db3 |
fixes #26143; Possible memory error (#26154)
fixes #26143 follows up https://github.com/nim-lang/Nim/pull/20307 |
||
|
araq
|
3c53629164 |
IC: don't build interface tables for dep-of-a-dep module loads
`loadTransitiveHooks` loads a module only to register its hooks, macro-cache replay and generic-instance offers. It is a dep-of-a-dep, not an import, so none of its symbols are visible to the module being semchecked — yet it went through the full `loadNifModule`, which builds the interface string tables by calling `loadSymFromIndexEntry` on every index entry, and `processTopLevel`, whose `export` branch calls `resolveSym` on every exported symbol. Both write into an `interf`/`interfHidden` pair that is scratch, shared across all iterations, and never read. Their only other effect is warming the name-keyed `c.syms` cache, which `resolveSym` refills lazily from the same index on a miss. New `LoadFlag.SkipInterfaceTables`, set only by `loadTransitiveHooks`, gates both. Measured on a 219-module program: a one-line edit rebuild goes 5.58s -> 3.21s (-42%), and the compiler's own cold IC build 202s -> 164s (-19%), since every `nim m` in a cold build pays this too. Profile of the `nim m` before: direct imports 17 modules/1.40s, transitive hooks 209 modules/2.99s, re-exports 39/0.23s. This is NOT byte-inert, unlike the previous commit: skipping the eager stub creation shifts the per-module load-order counter, so lazily-created stubs get different serialized item ids and 44 of 238 `.s.bif` change. The artifacts stay deterministic (238/238 reproducible across two independent cold builds) and 16/16 metamorphic tests pass, including `clean cache == incremental cache` and `a no-op edit changes no artifact` — so an existing nimcache re-sems those modules once and then converges. Also green: the 17-file `koch ic` suite, 13/13 differential edit checks against `nim c`, both `bootic` fixed points (debug and release), and 482 tests across tests/arc, destructor, closure, iter, gc, async with a failure set identical to the parent commit's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
araq
|
1f504865ed |
IC: one definition of a backend-minted symbol's disambiguator
The rule deciding which integer identifies a backend-minted symbol —
content-derived `disamb` for a lifted hook (`setHookDisamb`), `itemId.item`
otherwise — was written out at three sites: `mangleProcNameExt` and
`ccgutils.makeUnique` for the C name, `ast2nif.toNifSymName` for the NIF name,
each carrying its own copy of the ten-line rationale.
They drifted, which is exactly
|
||
|
araq
|
0c0cc1e496 | remove owner-checking-tower | ||
|
araq
|
1387093f99 | don't redo nifler steps unnecessarily |