Skip to content

Commit 1626503

Browse files
authored
finalir: a for keeps its iterator call and loop variables (#2528)
`trFor` lowered a `for` to a bare `(loop body)`, skipping the iterator call and the loop variables. That is fine for an analysis that throws its input away and fatal for one whose output is compiled, which is where the Final IR is headed (doc/internals/contracts_elim_rtchecks.md). It now emits `(for <iterCall> <vars> (stmts ...))`, with the body lowered exactly as a `loop`'s is: ending in `(continue .)`, `break` a forward `jmp` to the trailing exit label, which stays forced because the iterator is not inlined until hexer's `elimForLoops`. The prover analyses `(for ...)` directly: `traverseLoop`'s body analysis splits out as `analyseLoopBody` and the exit-label recording as `recordLoopExitLabel`, both shared with the new `traverseFor`. Two things fall out of no longer discarding the operands — the iterator call is analysed like any other call, so a contract its arguments must satisfy is discharged at the `for`; and the loop variables are declared and marked initialized (`declareForVars`) rather than being symbols nothing had ever declared. `trLoopFromBody` loses its now-dead `forBorrow` and `forceExitLabel` parameters.
1 parent a7e74a2 commit 1626503

70 files changed

Lines changed: 3147 additions & 2331 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
# Eliminating run-time checks
2+
3+
The contract pass (`src/nimony/contracts_fir.nim`) decides which run-time checks
4+
are needed, so it also records that decision in the module it publishes. The
5+
backend then emits a check only where the pass left one owed.
6+
7+
Two kinds of checks are affected:
8+
9+
- **Index checks.** `(arrat arr idx hi [lo])` becomes `nimIcheckAB`/`nimIcheckB`
10+
(and the unsigned pair) in `desugar` and `lengcgen`.
11+
- **`.requires` guards.** `desugar.trRequires` turns a routine's `.requires`
12+
into an `if not cond: panic` in the callee's prologue. Note that `s[i]` on a
13+
`seq` or `string` is a `.requires` call, not an `(arrat …)`, so this is where
14+
most of the benefit is.
15+
16+
Implementation points:
17+
18+
- `src/finalir/finalir.nim`: the lowering to the Final IR.
19+
- `src/nimony/contracts_fir.nim`: the prover; `applyVerdicts` writes its
20+
verdicts into the lowered module.
21+
- `src/nimony/semmain.nim`: `lowerAndProve` lowers, proves and publishes.
22+
- `doc/internals/final_ir.md`: the Final IR itself.
23+
24+
---
25+
26+
## The published module is the Final IR
27+
28+
`semmain.lowerAndProve` lowers the module right after `derefs`, runs the prover
29+
on the lowered buffer, and publishes that same buffer. There is one artifact per
30+
module, not two: a second one would duplicate the index, the staleness rules and
31+
the dependency graph.
32+
33+
What gets lowered follows one rule:
34+
35+
> **Lower everything except what is re-sem'd somewhere else.** Generic routine
36+
> bodies, template bodies and concept bodies stay in Nimony IR.
37+
38+
These are copied, substituted and sem'd again by the module that uses them, so
39+
lowering them would make sem accept `ite`/`loop`/`jmp` as input. The prover
40+
skips the same set for the same reason. The lowering already passes a
41+
non-concrete routine through verbatim, so the rule needed no extra work.
42+
43+
The prover's `kill`/`unknown` facts are stripped before the write
44+
(`finalir.stripAnalysisFacts`); the backend derives destruction from the `scope`
45+
tags. The validator checks the published module against `phasePostFinalIr`:
46+
the post-sem tags (for unlowered generic bodies) plus the Final IR's own.
47+
48+
Consumers of the published module:
49+
50+
- `indexgen` and `idetools` read top-level declarations and token positions,
51+
which the lowering leaves alone.
52+
- `renderer` stays Nimony-shaped; diagnostics already rendered lowered trees.
53+
- `semvalidator` needs branches as written, so it reads `<mod>.sem.nif`, which
54+
`--keepsemtree` writes. The compiler itself never reads that file.
55+
- `exprexec` compiles a program through the normal path, and `expreval` only
56+
folds constant expressions during sem. Neither sees the lowered form.
57+
58+
Hexer reads the Final IR throughout and lowers nothing at its entry. The one
59+
exception is the hooks the lifter creates in hexer, which it lowers with
60+
`toFinalIr(analysisFacts = false)`. `lengcgen` turns `ite`/`loop` into Leng
61+
`if`/`while true`, because the native back end and most optimizer passes do not
62+
know Leng's own `ite`/`loop`.
63+
64+
## `for` stays in the Final IR
65+
66+
A `for` is lowered to
67+
68+
```
69+
(for <iterCall> <vars> (scope <body> (continue .))) (lab exit)
70+
```
71+
72+
Only the body is lowered, exactly like a `loop` body: it ends in its only
73+
back-edge, and `break` is a forward `(jmp exit)`. The exit label is emitted even
74+
when nothing jumps to it; otherwise everything after the loop would look
75+
unreachable to the prover.
76+
77+
Iterator inlining stays in hexer (`iterinliner`). Because the body is not
78+
inlined when the prover runs:
79+
80+
- the prover analyses the iterator call's arguments at the `for`;
81+
- `declareForVars` declares the loop variables as initialized, and a `var T`
82+
or `lent T` binder as a borrow of the iterator's first argument. A binder is
83+
exempt from the `let`-reassignment check, because tuple unpacking and a
84+
closure iterator's resume assign to it;
85+
- `forRangeAssumes` emits `(assume …)` for what the iterator's `.ensures`
86+
promises, which is how `s[i]` under `for i in 0 ..< s.len` is proven.
87+
88+
### Why late inlining is sound
89+
90+
A check decided by the contract pass belongs to the module the code was
91+
**defined** in and travels with the code. So when hexer inlines an iterator
92+
body elsewhere:
93+
94+
- a body from a lax module inlined into a strict one keeps its checks;
95+
- a body from a strict module inlined into a lax one stays clean;
96+
- anything a later pass creates is checked, since nothing downstream removes
97+
checks.
98+
99+
## Scopes and `continue`
100+
101+
Branch and loop bodies are `(scope …)`. `destroyer` treats a `scope` as a
102+
destructor scope and a `stmts` as transparent, and with the flat `lab`/`jmp`
103+
layout a branch body is a sibling in the enclosing statement list, so a `stmts`
104+
would let a branch-local live to the end of the enclosing region. A
105+
statement-position `stmts` stays transparent: `{.keepOverflowFlag.}: let x = …`
106+
and every declaration `xelim` hoists are used after it.
107+
108+
A source-level `continue` becomes a `jmp` to a label in front of the back-edge,
109+
so `(continue .)` is always the last statement of a loop body. If the body has
110+
such a `continue`, the body gets a scope of its own and the label goes *after*
111+
it: a `jmp` may leave a scope, but it must not skip a declaration inside one,
112+
or the scope's end would destroy an uninitialized value
113+
(`tcontinue_skips_decl`).
114+
115+
## `.requires`: function duplication
116+
117+
Every top-level routine with a `.requires` becomes two:
118+
119+
- the **wrapper** keeps the routine's own symbol and header, so `desugar` still
120+
turns its `.requires` into the guard. Its body forwards every parameter:
121+
122+
```
123+
(result :r T .) (asgn r (call f`body p1 … pn)) (ret r)
124+
```
125+
126+
This is the shape sem already produces for a one-call routine, so no pass
127+
needed changes. Shoggoth's `tailcalls` turns it into `(ret (call …))`.
128+
- the **body** (`bodyOfRequires`, named `derivedName(stem, "body")`) carries the
129+
contract as `(assume …)`, so no guard is emitted for it. Its parameters get
130+
fresh names (`freshBodyName`); the wrapper keeps the originals because the
131+
guard's panic message names them.
132+
133+
The call-site verdict picks the symbol:
134+
135+
| verdict | emits |
136+
| --- | --- |
137+
| proven | a call to the **body** |
138+
| unprovable, run-time contracts | a call to the **wrapper** |
139+
| unprovable, `staticContracts` | an error |
140+
| disproven | an error |
141+
142+
### Why not a guard at the call site
143+
144+
A call site exists only for a resolved direct call. A proc value, a closure, a
145+
vtable slot, a hook or a callback stored in a field has none, so the contract
146+
would have to live in the proc type to cover them. With the split, anything
147+
that is not a resolved direct call can only name the wrapper, which has the
148+
guard. The guard also stays in one place instead of being copied to every call
149+
site.
150+
151+
### Rules
152+
153+
- **The body's name is derived by a fixed rule**, so a module that proves a
154+
call to an imported routine can name the body without a lookup.
155+
- **The wrapper must be cheap.** In practice the inliner folds it into its
156+
unproven callers and dead-code elimination drops it.
157+
- **Generics split per instance**, in the module that creates the instance,
158+
which is also where instances are judged
159+
(`tests/nimony/contracts/tstrictinstances.nim`).
160+
- **A proven recursive call goes to the body.** The prover starts a routine's
161+
analysis with its own `.requires` as facts, so such calls are provable.
162+
- **Not split:** a `method` (a call to it is the dispatch), an iterator, a hook,
163+
a routine without a body of its own, and a routine nested in another (its
164+
copy would have to exist in both the wrapper and the body of the enclosing
165+
routine).
166+
167+
## `arrat`
168+
169+
There is no callee to split, so the decision stays at the site. A proven index
170+
loses its bounds: `(arrat a i)`, or `(arrat a i . lo)` for an array that does
171+
not start at zero (NIFC arrays do, so `lo` is still subtracted). For an index
172+
that keeps its bounds, `desugar`/`lengcgen` emit the check. The prover marks
173+
rather than emits because emitting means hoisting a call into statement
174+
position, which inside an `and`/`or` operand needs the short-circuit handling
175+
`desugar` already has (`trShortCircuit`).
176+
177+
## Lowering invariants
178+
179+
The lowering's idiom `n = sub(n); <read children>; n = xStart; skip n` does not
180+
check that every child was read, so anything unexpected would disappear
181+
silently. `trIf`, `genIfViaCx`, `trCase` and `trTry` therefore call `bug` on a
182+
child they do not expect. `xelim` nests every `elif` chain into one `elif` plus
183+
`else`, which `trIf` relies on.
184+
185+
Every statement kind that reaches `trStmt` has its own branch.
186+
`-d:firFallbackProbe` prints the statements that fall through to the
187+
operand-lowering fallback; nothing does today.
188+
189+
An `{.assembler.}` body is passed through verbatim (its `if`s spelled `ite`),
190+
and the prover skips it.
191+
192+
## Results
193+
194+
Over `tall`: 1158 of 1279 index obligations and 3851 of 6260 `.requires` call
195+
sites are proven. Against the same compiler without the redirect to the body,
196+
the `matmul` and `nifbench` kernels run 1–6% faster and the generated C is 1.6%
197+
smaller, at the same compile time. Publishing the lowered module grows the
198+
`tjson` closure's nifs by 11.8%.
199+
200+
## Remaining work
201+
202+
1. **Remove the `activeChecks` path.** `--boundchecks:off` and `-d:danger` are
203+
still a backend flag: `desugar` and `lengcgen` use `activeChecks` to decide
204+
whether an owed check is emitted at all. The flag should become a third
205+
contract feature next to `runtimeContracts` and `staticContracts`
206+
(`features.nim`), under which the prover drops every obligation itself. Then
207+
`CheckMode`, the `activeChecks` fields and the `--flags` handling can go, as
208+
can `trRequires` and its helpers in `desugar`. The split can then depend on
209+
the build mode too: with checks off, no wrapper is needed. `RangeCheck`
210+
emits nothing today; it either goes as well or gets connected to the
211+
prover's `checkRangeAssign`.
212+
2. **`was`**: a way to map lowered code back to its source shape, added one
213+
consumer at a time, error messages first. Notes:
214+
- In-module macros never see the Final IR: expansion happens during sem,
215+
before the lowering. `was` is for reflection on imported bodies,
216+
`renderer.asNimCode` and idetools.
217+
- `try` and `case` keep their shape. What loses it: `while` (the condition
218+
becomes a guard in the body), `if`/`elif` chains, `and`/`or`, `break L`,
219+
`block` and its name, and `xelim`'s temps.
220+
- `(was STR)` already exists as a LengPragma taking a string. The new one
221+
carries a tag, so widen that one on purpose or pick another name.
222+
- A pass that rewrites inside a `was` region drops the annotation, and the
223+
validator enforces that: a stale `was` is worse than none.
224+
- Lowering less to need less `was` is not an option: the flat `elif` layout
225+
is where the Leng size win came from (`final_ir.md`, *What a flat layout
226+
costs*).
227+
3. **Leng `loop`.** The Leng consumers read the infinite `(loop body)` form, but
228+
nothing produces a Leng `loop` yet, and `continue` is not a `LengStmt`, so
229+
its back-edge cannot be written on the Leng side.
230+
231+
## Open questions
232+
233+
- A `.requires` that cannot be checked at run time has no guard to put in a
234+
wrapper. Such a routine needs no wrapper, and therefore has no safe indirect
235+
use.

doc/internals/final_ir.md

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ way they are.
1212

1313
Implementation points:
1414

15-
- `src/njvl/finalir.nim` — the lowering to the structured control-flow form
16-
(`loop`/`ite`/`lab`/`jmp`). Currently reached from `src/nimony/contracts_fir.nim`
17-
(contract and nil analysis), not yet from the backend pipeline.
15+
- `src/finalir/finalir.nim` — the lowering to the structured control-flow form
16+
(`loop`/`ite`/`lab`/`jmp`). It runs in nimsem, as the module's last step
17+
(`semmain.lowerAndProve`): the contract and nil analysis
18+
(`src/nimony/contracts_fir.nim`) reads its output, and that same buffer is
19+
what nimsem publishes, so the whole backend starts from it.
1820
- `src/hexer/xelim.nim` — the `Goal` enum; `TowardsFinalIr` is the mode
1921
`finalir.nim` runs `lowerExprs` in.
2022
- `src/hexer/pipeline.nim` — the backend pass order.
@@ -258,24 +260,35 @@ clears it".
258260
## Current pipeline
259261

260262
```
261-
desugar → lambdalift → xelim1 → eraiser → duplifier → destroyer → cps →
262-
vtables → constparams → xelim_final
263+
nimsem: … → derefs → finalir → contracts → <published module nif>
264+
hexer: iterinliner → desugar → lambdalift → eraiser → duplifier →
265+
destroyer → cps → vtables → constparams → xelim_final → lengcgen
263266
```
264267

265-
- **`xelim1`** establishes the normal form. It is the only pass that
266-
*creates* it.
268+
Every pass from `finalir` on reads and writes the Final IR, `lengcgen`
269+
included: it spells `ite` as a Leng `if` and `loop` as `while true` (dropping
270+
the trailing back-edge), because Leng's back ends and optimizer passes speak
271+
those. Nimony `if`/`while`/`block`/`break` reaching it is a bug. Two producers
272+
are not hexer passes proper and get the lowering by other means: the lifter
273+
runs in `nimsem` too and so emits Nimony IR, and `pipeline.transform` lowers
274+
its hooks with `toFinalIr`; an `{.assembler.}` body is taken verbatim except
275+
that its `if`s are spelled `ite` (`finalir.trAsmStmt`).
276+
277+
- **`finalir`** establishes the normal form (it runs `xelim` in
278+
`TowardsFinalIr` mode). It is the only pass that *creates* it, and it runs
279+
in nimsem: a module is published lowered. A body that is re-sem'd elsewhere
280+
— a generic routine, a template — is published as sem left it.
267281
- **`eraiser`** (`src/hexer/eraiser.nim`) emits the `canRaise` temp and its
268-
`if failed(tmp): raise tmp` check as statements. It moved *behind* `xelim1`
269-
in the process: it used to run first precisely because it needed a repair
270-
pass after it.
282+
`ite failed(tmp): raise tmp` check as statements.
271283
- **`duplifier`** (`src/hexer/duplifier.nim`) does the same for its owning
272284
temps — `bindToTemp`/`finishOwningTemp`, `trNewobj`'s decl + OOM check +
273285
payload assignment, and `genLastRead`'s bitcopy + `=wasMoved`.
274286
- **`xelim_final`** is *not* a repair pass. `LowerCasts` performs two real
275287
lowerings: it unnests calls (the Final-IR "calls are unnested statements"
276288
rule) and binds a cast's source and result to variables, which the NIFC
277289
backends require. It does still flatten `vtables`/`constparams` temps as a
278-
side effect — see *Remaining work*.
290+
side effect — see *Remaining work*. The `and`/`or` it materializes are
291+
spelled `ite` here, `if` in the `TowardsFinalIr` run (`openIfElse`).
279292

280293
`xelim2` is gone.
281294

doc/tags.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,9 @@
162162
| `(raises ...)` | LengPragma, NimonyPragma | proc annotation; optional list of exception types the proc may raise |
163163
| `(errs)` | LengPragma | proc annotation |
164164
| `(static T)`; `(static)` | LengPragma, NimonyType, NiflerKind | `static` type or annotation |
165-
| `(ite X S S S STR_LIT?)` | ControlFlowKind, FinalIrKind, LengStmt | if-then-else followed by `join` information followed by an optional label |
165+
| `(ite X S .S S? STR_LIT?)` | ControlFlowKind, FinalIrKind, LengStmt | if-then-else, optionally followed by `join` information and by a label. The Final IR and `controlflow.nim` use the three-child form, whose else-part may be `.`; the `join` slot is Leng's |
166166
| `(itec X S S)` | FinalIrKind, LengStmt | if-then-else (that was a `case`) |
167-
| `(loop S X S S)` | FinalIrKind, LengStmt | `loop` components are (before-cond, cond, loop-body, after) |
167+
| `(loop S)` | FinalIrKind, LengStmt | infinite loop; the body ends in `(continue .)`, its sole back-edge, and every forward exit is a `(jmp …)` |
168168
| `(v X INT_LIT)` | FinalIrKind | `versioned` locations |
169169
| `(etupat X INT_LIT)` | FinalIrKind | tupat expression for error handling |
170170
| `(unknown X)` | FinalIrKind | location's contents is unknown at this point |

0 commit comments

Comments
 (0)