From 06b1bf8f9a286ab66eca675aa94bb7aaabc56cdc Mon Sep 17 00:00:00 2001 From: Araq Date: Sun, 30 Aug 2026 10:23:43 +0200 Subject: [PATCH] IC: take the seam into `trees` and `ccgutils` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR --- compiler/ccgcalls.nim | 2 +- compiler/ccgexprs.nim | 2 +- compiler/ccgstmts.nim | 2 +- compiler/ccgutils.nim | 26 +++++++++++++++++++++++--- compiler/cgen.nim | 25 +++++++++++++++++++++++++ compiler/trees.nim | 36 +++++++++++++++++++----------------- tools/icgrind/grindlib.nim | 38 ++++++++++++++++++++++++++++++++++++++ tools/icgrind/grindme.nim | 1 + 8 files changed, 109 insertions(+), 23 deletions(-) diff --git a/compiler/ccgcalls.nim b/compiler/ccgcalls.nim index f0ce207932..440314509f 100644 --- a/compiler/ccgcalls.nim +++ b/compiler/ccgcalls.nim @@ -904,7 +904,7 @@ proc genNamedParamCall(p: BProc, ri: PNode, d: var TLoc) = p.s(cpsStmts).addStmt(): p.s(cpsStmts).add(extract(pl)) -proc notYetAlive(n: PNode): bool {.inline.} = +proc notYetAlive(n: AnyNode): bool {.inline.} = let r = getRoot(n) result = r != nil and r.loc.lode == nil diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index b4a3f2542f..1db60d6025 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -148,7 +148,7 @@ proc getStorageLoc(n: PNode): TStorageLoc = result = getStorageLoc(n.firstSon) else: result = OnUnknown -proc canMove(p: BProc, n: PNode; dest: TLoc): bool = +proc canMove(p: BProc, n: AnyNode; dest: TLoc): bool = # for now we're conservative here: if n.kind == nkBracket: # This needs to be kept consistent with 'const' seq code diff --git a/compiler/ccgstmts.nim b/compiler/ccgstmts.nim index 6c13a67584..f0010846a2 100644 --- a/compiler/ccgstmts.nim +++ b/compiler/ccgstmts.nim @@ -1047,7 +1047,7 @@ proc branchHasTooBigRange(b: AnyNode): bool = it.secondSon.intVal - it.firstSon.intVal > RangeExpandLimit: return true -proc ifSwitchSplitPoint(p: BProc, n: PNode): int = +proc ifSwitchSplitPoint(p: BProc, n: AnyNode): int = result = 0 for i, branch in isons(n, 1): var stmtBlock = lastSon(branch) diff --git a/compiler/ccgutils.nim b/compiler/ccgutils.nim index 42c3c34148..60beaf8495 100644 --- a/compiler/ccgutils.nim +++ b/compiler/ccgutils.nim @@ -11,7 +11,7 @@ import ast, types, msgs, wordrecg, - platform, trees, options, cgendata, mangleutils, renderer, modulegraphs + platform, trees, options, cgendata, mangleutils, renderer, modulegraphs, bnode import std/[hashes, strutils, formatfloat] @@ -32,8 +32,28 @@ proc getPragmaStmt*(n: PNode, w: TSpecialWord): PNode = else: result = nil -proc stmtsContainPragma*(n: PNode, w: TSpecialWord): bool = - result = getPragmaStmt(n, w) != nil +proc stmtsContainPragma*(n: AnyNode, w: TSpecialWord): bool = + ## Deliberately NOT `getPragmaStmt(n, w) != nil`, and the reason is the one + ## shape the `AnyNode` seam cannot serve: a proc that returns a node OR nil. + ## `.bif` spells a missing child as a `DotToken` *inside* a tree, so there is + ## no nil token to hand back as a return value, and a `Cursor` is not nilable. + ## Predicates split out from such a proc are the way across. + ## + ## The duplicated traversal is the cost, and it is checked rather than + ## trusted: `grindPredicates` asserts this answers exactly + ## `getPragmaStmt(n, w) != nil` at every node, so the two cannot drift apart + ## silently. + case n.kind + of nkStmtList: + result = false + for it in sons(n): + if stmtsContainPragma(it, w): return true + of nkPragma: + result = false + for it in sons(n): + if whichPragma(it) == w: return true + else: + result = false proc hashString*(conf: ConfigRef; s: string): BiggestInt = # has to be the same algorithm as strmantle.hashString! diff --git a/compiler/cgen.nim b/compiler/cgen.nim index 0c2e738d3a..45d035af5b 100644 --- a/compiler/cgen.nim +++ b/compiler/cgen.nim @@ -1616,6 +1616,22 @@ when defined(newIcBackend): check "isSimpleExpr", isSimpleExpr(n) check "reifiedOpenArray", reifiedOpenArray(n) check "bodyCanRaise", bodyCanRaise(p, n) + check "getMagic", getMagic(n) + check "whichPragma", whichPragma(n) + check "getRoot", getRoot(n) + check "isDeepConstExpr", isDeepConstExpr(n) + check "stmtsContainPragma", stmtsContainPragma(n, wLinearScanEnd) + check "notYetAlive", notYetAlive(n) + + # `stmtsContainPragma` had to be re-derived rather than defined as + # `getPragmaStmt(...) != nil`, because a `Cursor` has no nil to return (see + # the note at its definition). That leaves two copies of one traversal, so + # the equivalence is asserted here instead of assumed — on the AST side, + # where `getPragmaStmt` exists. + for w in [wLinearScanEnd, wComputedGoto]: + if stmtsContainPragma(a, w) != (getPragmaStmt(a, w) != nil): + bail("stmtsContainPragma vs getPragmaStmt for " & $w, + $stmtsContainPragma(a, w), $(getPragmaStmt(a, w) != nil)) # `skipTrivialIndirections` returns a NODE, and the two spellings return # values of different types that cannot be compared directly. Kind plus @@ -1639,6 +1655,15 @@ when defined(newIcBackend): check "isConstClosure", isConstClosure(n) if a.kind == nkOfBranch and ordinalRanges(a): check "branchHasTooBigRange", branchHasTooBigRange(n) + if a.kind == nkCaseStmt and a.safeLen > 1 and + (block: + # `ifSwitchSplitPoint` reaches `branchHasTooBigRange`, so the same + # ordinal gate has to hold for every branch it will look at. + var ok = true + for br in sonsFrom(a, 1): + if br.kind == nkOfBranch and not ordinalRanges(br): ok = false + ok): + check "ifSwitchSplitPoint", ifSwitchSplitPoint(p, n) # Graded from the parent — see the note above on why these two cannot be # asked at an arbitrary node. `genVarTuple` asks about the tuple's last diff --git a/compiler/trees.nim b/compiler/trees.nim index 24226e79d5..3d2378df8e 100644 --- a/compiler/trees.nim +++ b/compiler/trees.nim @@ -10,7 +10,7 @@ # tree helper routines import - ast, wordrecg, idents + ast, wordrecg, idents, bnode proc cyclicTreeAux(n: PNode, visited: var seq[PNode]): bool = result = false @@ -83,12 +83,13 @@ proc sameTree*(a, b: PNode): bool = if not sameTree(a[i], b[i]): return result = true -proc getMagic*(op: PNode): TMagic = - if op == nil: return mNone +proc getMagic*(op: AnyNode): TMagic = + if op.isNilNode: return mNone case op.kind of nkCallKinds: - case op[0].kind - of nkSym: result = op[0].sym.magic + let callee = op.firstSon + case callee.kind + of nkSym: result = callee.sym.magic else: result = mNone else: result = mNone @@ -102,15 +103,16 @@ proc isCaseObj*(n: PNode): bool = for i in 0.. 0: n[0] else: n +proc whichPragma*(n: AnyNode): TSpecialWord = + let key = if n.kind in nkPragmaCallKinds and n.hasSons: n.firstSon else: n case key.kind of nkIdent: result = whichKeyword(key.ident) of nkSym: result = whichKeyword(key.sym.name) of nkCast: return wCast of nkClosedSymChoice, nkOpenSymChoice, nkOpenSym: - return whichPragma(key[0]) + return whichPragma(key.firstSon) of nkBracketExpr: if n.kind notin nkPragmaCallKinds: return wInvalid - result = whichPragma(key[0]) + result = whichPragma(key.firstSon) if result notin {wHint, wHintAsError, wWarning, wWarningAsError}: # note bracket pragmas, see processNote result = wInvalid @@ -205,7 +207,7 @@ proc extractRange*(k: TNodeKind, n: PNode, a, b: int): PNode = result = newNodeI(k, n.info, b-a+1) for i in 0..b-a: result[i] = n[i+a] -proc getRoot*(n: PNode): PSym = +proc getRoot*(n: AnyNode): PSym = ## ``getRoot`` takes a *path* ``n``. A path is an lvalue expression ## like ``obj.x[i].y``. The *root* of a path is the symbol that can be ## determined as the owner; ``obj`` in the example. @@ -217,11 +219,11 @@ proc getRoot*(n: PNode): PSym = result = nil of nkDotExpr, nkBracketExpr, nkHiddenDeref, nkDerefExpr, nkObjUpConv, nkObjDownConv, nkCheckedFieldExpr, nkHiddenAddr, nkAddr: - result = getRoot(n[0]) + result = getRoot(n.firstSon) of nkHiddenStdConv, nkHiddenSubConv, nkConv: - result = getRoot(n[1]) + result = getRoot(n.secondSon) of nkCallKinds: - if getMagic(n) == mSlice: result = getRoot(n[1]) + if getMagic(n) == mSlice: result = getRoot(n.secondSon) else: result = nil else: result = nil diff --git a/tools/icgrind/grindlib.nim b/tools/icgrind/grindlib.nim index e756c7d1c1..c64c7f14c8 100644 --- a/tools/icgrind/grindlib.nim +++ b/tools/icgrind/grindlib.nim @@ -96,3 +96,41 @@ proc guardedLib*(x: int): string = result = "err" finally: discard + +proc scanEnd*(x: int): int = + ## `stmtsContainPragma(wLinearScanEnd)` and, through it, a NON-ZERO + ## `ifSwitchSplitPoint`. Without this both answer the same thing at every node + ## in the closure — the stdlib uses neither pragma — and the grinder grades + ## two constants. + var r = 0 + case x + of 0: + r = 1 + of 1: + {.linearScanEnd.} + r = 2 + of 2: r = 3 + else: r = 4 + result = r + +type Op* = enum opAdd, opAdd2, opSub, opEnd + +proc computedGotoLoop*(inp: openArray[Op]): int = + ## `stmtsContainPragma(wComputedGoto)`, the other word the equivalence check + ## against `getPragmaStmt` looks for. The operand is an ENUM because + ## `computedGoto` requires an exhaustive case and rejects an `else`, and it + ## jumps straight from the end of one branch to the next dispatch — the + ## `while` condition is NOT re-evaluated, so termination has to come from an + ## explicit op. + var r = 0 + var i = 0 + while true: + {.computedGoto.} + let op = inp[i] + case op + of opAdd: r += 1 + of opAdd2: r += 2 + of opSub: r -= 1 + of opEnd: break + inc i + result = r diff --git a/tools/icgrind/grindme.nim b/tools/icgrind/grindme.nim index 568bee6f70..470aa4a852 100644 --- a/tools/icgrind/grindme.nim +++ b/tools/icgrind/grindme.nim @@ -44,3 +44,4 @@ when isMainModule: echo classifyChar('Q'), bigRange(150000), smallRange(5), inSets('e'), bigSet('q') echo viaOpen(@[1, 2, 3]), adder(4)(5), constClosure()(3), noInitVar() echo tuples() + echo scanEnd(1), " ", computedGotoLoop([opAdd, opAdd2, opSub, opEnd])