From 087c82f985c2f9dbdd77415c34a6a137b476e956 Mon Sep 17 00:00:00 2001 From: araq Date: Mon, 31 Aug 2026 15:38:15 +0200 Subject: [PATCH] IC: two assumptions that only held while a cg process wrote one TU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- compiler/cgen.nim | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/compiler/cgen.nim b/compiler/cgen.nim index 962d713695..3f6e5012b4 100644 --- a/compiler/cgen.nim +++ b/compiler/cgen.nim @@ -891,6 +891,16 @@ proc localVarDecl(res: var Builder, p: BProc; n: AnyNode, backendEnsureMutable s fillLoc(s.locImpl, locLocalVar, n, OnStack) if s.kind == skLet: incl(s, lfNoDeepCopy) + else: + # Already named by an EARLIER emission of this same routine — an inline proc + # regenerated per user, or (under a batched `cg`) a definition emitted into + # two of this process's TUs. `fillLocalName` caches the C name on the PSym + # but takes the uniquifying counter from the BProc, and this BProc is a new + # one whose `sigConflicts` never saw that name. Claim it, or the next local + # of the same base name minted HERE starts from `_1` again and redeclares + # it: gcc "redeclaration of 'i_1' with no linkage", 64 of Atlas's 204 `.c` + # at batch size 4. + p.sigConflicts.inc(s.name.s.mangle) genCLineDir(res, p, n.info, p.config) @@ -2366,13 +2376,23 @@ proc genProcPrototype(m: BModule, sym: PSym) = genMemberProcHeader(m, sym, scratch, false, true) return if lfDynamicLib in sym.loc.flags: - if m.config.cmd == cmdNifC and m.config.icBackendStage == "cg": - # Under IC per-module cg every demander emits the dynlib proc's DEFINITION - # locally (findPendingModule returns `m`, so symInDynamicLib follows this - # call and the merge stage keeps one def per C name). Emitting the - # cross-module `extern` proto here would register `sym.id` in - # `m.declaredThings` and thereby make that `symInDynamicLib` skip, leaving - # the `Dl_*` symbol declared-but-never-defined -> undefined at link. + # Does THIS TU emit the dynlib proc's definition? Under IC cg it does + # whenever `findPendingModule` routes the symbol here — which it does unless + # the owner is another member of this process's batch. Mirrored rather than + # called, because `findPendingModule` creates a `BModule` on demand and a + # prototype has no business doing that. + let owner = getModule(sym) + let emittedByABatchSibling = + owner != nil and owner.kind == skModule and + owner.position != m.module.position and + m.g.icEmitted.contains(owner.position) + if m.config.cmd == cmdNifC and m.config.icBackendStage == "cg" and + not emittedByABatchSibling: + # This TU emits the DEFINITION itself: `symInDynamicLib` follows this call + # and the merge stage keeps one def per C name. Emitting the cross-module + # `extern` proto here would register `sym.id` in `m.declaredThings` and + # thereby make that `symInDynamicLib` skip, leaving the `Dl_*` symbol + # declared-but-never-defined -> undefined at link. discard "definition emitted by symInDynamicLib" elif sym.itemId.module != m.module.position and not containsOrIncl(m.declaredThings, sym.id):