From bad9d3b2bcd5cde58b593060648268d1be8f9791 Mon Sep 17 00:00:00 2001 From: araq Date: Mon, 31 Aug 2026 10:18:09 +0200 Subject: [PATCH] IC: route a cg definition by who owns it, not by who asked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- compiler/cgen.nim | 53 +++++++++++++++++++++++++++++------------ compiler/cgendata.nim | 10 +++++++- compiler/nifbackend.nim | 7 ++++++ 3 files changed, 54 insertions(+), 16 deletions(-) diff --git a/compiler/cgen.nim b/compiler/cgen.nim index 5dd5075d8b..1f2752561a 100644 --- a/compiler/cgen.nim +++ b/compiler/cgen.nim @@ -67,28 +67,51 @@ proc addForwardedProc(m: BModule, prc: PSym) = proc newModule*(g: BModuleList; module: PSym; conf: ConfigRef; idgen: IdGenerator): BModule proc getCFile*(m: BModule): AbsoluteFile +proc ownerModule(m: BModule; s: PSym): BModule = + ## The BModule of `s`'s own module, created on demand. A NIF backend loads + ## modules lazily, so the owner may have no BModule yet even though the symbol + ## resolved. + var ms = getModule(s) + registerModule m.g.graph, ms + if ms.position >= m.g.mods.len: + result = newModule(m.g, ms, m.config, idGeneratorForBackend(ms)) + else: + result = m.g.mods[ms.position] + if result == nil: + result = newModule(m.g, ms, m.config, idGeneratorForBackend(ms)) + proc findPendingModule(m: BModule, s: PSym): BModule = - # TODO fixme if m.config.cmd == cmdNifC and m.config.icBackendStage == "cg": - # Per-module backend codegen: only module M (`m`) is emitted in this - # process, so every demanded definition — whether a normal proc owned by - # another (here unwritten) module or a minted instance/hook — is emitted - # into M's TU. Definitions owned elsewhere are emitted again by their own - # module's cg process; the merge stage keeps one per C name and turns the - # rest into prototypes (which already live in the unmarked protos section). + # Per-module backend codegen. `m.g.icEmitted` is the set of modules THIS + # process writes a TU for, so it — not the identity of whichever TU happened + # to demand `s` — decides where the definition goes: + # + # * owner in `icEmitted`: this process is writing that module's TU, so the + # definition belongs in it and nowhere else. That is the ordinary + # whole-program routing below, and honouring it is what lets one process + # emit SEVERAL modules without their definitions collapsing into the first + # TU to ask for them. With the set at its current size of one, the owner + # IS `m` and this returns exactly what the old unconditional `return m` + # did — the point of the branch is that it stops being a special case. + # + # * owner elsewhere: the module is not written in this process, so the + # definition has nowhere else to go and is emitted here as well + # (emit-everywhere). The process that owns it emits it too; `merge` keeps + # one per C name and turns the rest into prototypes, which already live in + # the unmarked protos section. + # + # `getModule` walks the owner chain and yields nil if it never reaches a + # module (backend-minted symbols can be parented outside one), which is a + # definition with no owning TU: emit it here. + let ms = getModule(s) + if ms != nil and ms.kind == skModule and m.g.icEmitted.contains(ms.position): + return ownerModule(m, s) return m if m.config.symbolFiles == v2Sf or optCompress in m.config.globalOptions: let ms = s.itemId.module #getModule(s) result = m.g.mods[ms] elif m.config.cmd in {cmdNifC, cmdM}: - var ms = getModule(s) - registerModule m.g.graph, ms - if ms.position >= m.g.mods.len: - result = newModule(m.g, ms, m.config, idGeneratorForBackend(ms)) - else: - result = m.g.mods[ms.position] - if result == nil: - result = newModule(m.g, ms, m.config, idGeneratorForBackend(ms)) + result = ownerModule(m, s) else: var ms = getModule(s) result = m.g.mods[ms.position] diff --git a/compiler/cgendata.nim b/compiler/cgendata.nim index ae4cc58391..88fb00f48f 100644 --- a/compiler/cgendata.nim +++ b/compiler/cgendata.nim @@ -142,6 +142,13 @@ type # not a list of IDs nor can it be made to be one. mangledPrcs*: HashSet[string] + icEmitted*: IntSet + ## Under `--icBackendStage:cg`: the positions of the modules THIS process + ## writes a translation unit for. `cgen.findPendingModule` consults it to + ## decide where a demanded definition goes — see the comment there. Empty + ## outside that stage, which is why every other backend keeps the ordinary + ## whole-program routing. + TCGen = object of PPassContext # represents a C source file s*: TCFileSections # sections of the C file flags*: set[CodegenFlag] @@ -238,7 +245,8 @@ proc newProc*(prc: PSym, module: BModule): BProc = proc newModuleList*(g: ModuleGraph): BModuleList = BModuleList(typeInfoMarker: initTable[SigHash, tuple[str: Rope, owner: int32]](), - config: g.config, graph: g, nimtvDeclared: initIntSet()) + config: g.config, graph: g, nimtvDeclared: initIntSet(), + icEmitted: initIntSet()) iterator cgenModules*(g: BModuleList): BModule = for m in g.modulesClosed: diff --git a/compiler/nifbackend.nim b/compiler/nifbackend.nim index 8c14f9e971..5caf89ca7d 100644 --- a/compiler/nifbackend.nim +++ b/compiler/nifbackend.nim @@ -553,6 +553,13 @@ proc generateCgStage(g: ModuleGraph; mainFileIdx: FileIndex) = "per-module codegen: module not found for suffix: " & g.config.icBackendModule) return + # Declare which modules this process writes a TU for. `findPendingModule` + # reads it to route a demanded definition to its owner when the owner is one + # of them, and into the current TU otherwise. One member today — the set is + # what a batched `cg` grows, and what keeps its members' definitions in their + # own TUs instead of in whichever one demanded them first. + BModuleList(g.backend).icEmitted.incl target.module.position + # The `lower` stage already wrote each module's transformed bodies + lifted # hooks into its `.t.nif`, which the loaders above read directly (toNifFilename # resolves the `.t.nif`); transformed bodies arrive via loadSymFromCursor and