mirror of
https://github.com/nim-lang/Nim.git
synced 2026-09-01 19:33:42 +00:00
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.
This commit is contained in:
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user