diff --git a/DISCUSS.md b/DISCUSS.md index 73e76e0..b326f8f 100644 --- a/DISCUSS.md +++ b/DISCUSS.md @@ -1259,3 +1259,174 @@ that can run, runs, and prints what LLVM's build prints — down to stderr. What is left in `x86.ml` is not conditions. It is a container return convention, a redefinition cell, two arithmetic edge cases the language has not decided, and no debug info. None of those is the shape conditions were: each is a known thing in a known place, and the guard is not underneath any of them. + +## 18. The container return convention was never there, and the redefinition cell now is + +Item 17's no-plan bucket opened with two correctness items and a loose end it was honest about. Both items are closed +and the loose end is the reason the first one closed the way it did rather than the way it was written down. **LLVM is +untouched and still the default and the release backend.** `--x86` is still off by default and still refused with +`--debug`, `--sanitize` and every wasm target; it is **no longer refused with `--dev`**, and question 3 is the argument +for that. + +### Question 1 — the counts, and a corpus that moved + +`spike/x86/survey.sh`, unchanged in what it compares: every program in `test/programs` and every probe in `spike/x86`, +built both ways with the same bounds-check setting, run, and diffed on **stdout, stderr and exit status**. + +| | item 17 | measured here, before | after | +|---|---|---|---| +| **MATCH** | 89 | **93** | **97** | +| **DIFFER** | 0 | 0 | **0** | +| **refused by name** | 0 | **2** | **0** | +| skipped: does not compile | 25 | 28 | 28 | +| skipped: no `main` | 6 | 6 | 6 | +| skipped: never terminates | 2 | 2 | 2 | + +**The baseline is not the one the brief quoted, and that is the first finding.** Item 17 measured 89/0/0; this lane +measured 93 MATCH and **2 refusals** before writing anything. The corpus moved underneath: another lane landed +`(slice-from-ptr p n)`, and it arrived as two refusals rather than one — `slice-from-ptr.flan` and `bounds.flan` both +stopped building through `--x86`. A backend that refuses by name does not rot quietly, but it does rot, and nothing +was watching. That is an argument for running the survey in CI rather than in a lane. + +The form itself is nothing: a `Slice _` is `{ptr, i64}` here exactly as it is in `emit.ml`, so it is one store of the +pointer and one of the length and no new representation at all. The half worth writing down is that **the check has to +be signed**. There is nothing to compare the length against — only the caller knows what is behind that pointer — so +what is checked is that the promise is not absurd, and `check_slice`'s own compares are *unsigned*. A negative `i32` +sign-extended to 64 bits is a huge unsigned value that `jbe` waves straight through, and the result is a slice about +2^64 long that reads as a pass and faults somewhere else entirely. + +Nothing in the corpus walks that path, because every length in `slice-from-ptr.flan` is a literal and a negative +literal is refused by `check.ml` before any code is emitted. `spike/x86/p7-slice-from-ptr.flan` takes the length as a +parameter and runs it through a `restart-case`, which puts the condition's `low`/`high`/`length` on stdout beside the +LLVM build's. + +The other two of the four new MATCHes are this lane's own probes, below. + +### Question 2 — why `flan_vec_as_slice` avoided the aggregate-return refusal + +**It is the first of the two possibilities item 17 named: the refusal is narrower than it reads, and nothing is going +right by accident.** + +`flan_vec_as_slice`'s Flan-level return type is `Unit`. `check.ml:3721` builds it as `rt loc Types.Unit`, +`flan_rt.c:1165` is `void flan_vec_as_slice(flan_vec *v, void *out, ...)`, and `emit.ml:2464` declares it `void`. So +`is_void rty` answers first and the `is_agg rty` test below it is never reached. The slice comes back through an +out-pointer the checker allocated, which is not one symbol's accident but the convention: + +- **Every aggregate-valued runtime result crosses through an out-pointer.** Every other `rt` builder in `check.ml` + answers `Unit`, an `Int`, a `Ptr`, an `Alloc` or a `Handle`. `flan_pool_resolve` answers `Ptr elem` and the `Option` + is built in Flan; `Argv` is its own `Tast` node with its own out-pointer and never comes through here at all. +- **`crossable`**, the other user of the same code path, admits `String` and `Slice _` only as *a parameter* and + refuses an aggregate return from a `declare` outright. + +**So there is no sret convention to build for `Rt`, and building one would have been worse than the refusal.** This is +the C boundary, where the header says the backend must match SysV rather than pick: a 16-byte slice comes back in +`rax:rdx`, not through the internal hidden-pointer convention. There is no classifier in the file, there is nothing to +test one against, and "untestable and wrong" is a bad trade against a line that costs nothing. The line stays as a +guard against those two rules changing, and now names which rules and what the work would actually be. + +**Item 16's claim that the container runtime is unexercised went with it, and it was already stale when item 17 +repeated it.** `Vec` and `Map` run through `vec.flan`, `vec-of-vec.flan`, `vec-in-struct.flan`, `maps.flan` and +`map-iter.flan`; `Pool` — which neither report checked — runs through `registry.flan`, `handles.flan`, +`generics.flan` and `pool-stale-region.flan`. All match, and they have since item 17's guard landed. + +### Question 3 — the cell, and why `--x86 --dev` is no longer refused + +`FnAddr (Fnval n)` emitted the symbol. It now reads the cell — and **so does every direct call**, which is the half +that matters and is what `emit.ml`'s `body_of` does: a redefinition is one store, and its whole purpose is to reach +call sites that already exist. What is emitted, all of it behind `dev`: + +- **One cell per function** in `.data`, `.globl`, initialised to the body this build compiled. Spelled exactly as + `Emit.cellname` spells it, because that is the point of having one here — an LLVM-built redefinition module binds + `@"flan.cell." = external global ptr` against whatever built the host. `nm -D` over the two builds of the same + program gives identical sets of 68 cell symbols. +- **The cell load placed after the arguments.** `emit.ml` has that as a load-bearing comment: a redefinition landing + between two calls must not land in the middle of one. `CallPtr` stays the other way round, for `emit.ml`'s reason. +- **The `flan_dev_reg_enable` constructor**, which arms the allocation registry. + +Not emitted: `Emit.cellptr`, the deeper spelling for a name the host was never built with. It cannot arise in a +whole-program build, where `known` is true of everything, and it belongs with the redefinition module that would +introduce such a name. + +**The refusal is relaxed, and the argument is that `flan dev` never reaches that fork.** `--x86` is read in exactly one +place, `flan build`'s argument list; the daemon builds its host through `Build.executable` and its modules through +`Build.shared` without it, and there is no spelling that hands it one. So the flag now means what it says — a host +whose call sites are redefinable, built by this backend — and nothing claims the module that would redefine through +them exists. + +### Question 4 — what made that believable, because the corpus cannot + +Two measurements, and the second is the one that matters. + +**The corpus with `--dev` on both sides: 97 MATCH, 0 DIFFER.** `SURVEY_FLAGS=--dev` is opt-in so the counts in question +1 stay the same measurement. Before the constructor was added it read **96/1**: `registry.flan` asks `(live? ...)` and +got four zeroes, because the allocation registry was never armed. That is the whole of what a dev host does +differently besides the cells, and it is worth saying that the corpus found it — one program out of 97 observes it. + +**And the thing the corpus structurally cannot test.** A dev build starts with every cell pointing at the body this +build compiled, so it prints exactly what a release build prints *whether or not anything reads the cell*. The +property that makes the whole corpus a safe test of the cells is the property that makes it a useless one. + +So `spike/x86/cells.sh` changes what a cell holds. It preloads a shared object whose constructor looks up +`flan.cell.twice` with `dlsym` — the cells are in `.dynsym` because a dev build is `-rdynamic` — and stores a different +body there. That is the one store a redefinition ends in, done from outside, with no compiler involved. Four builds, +and the two controls are half the test: + +| | | +|---|---| +| `llvm --dev` | 22 22 — the cell is read | +| `x86 --dev` | 22 22 — **this lane's claim** | +| `llvm` | 42 42 — no cell; `dlsym` answers NULL | +| `x86` | 42 42 | + +`spike/x86/p8-cell.flan` has both call shapes, because they are two different cases in both backends: a direct call by +name, and a function *value*, which is the one `FnAddr` that is not the symbol. The release rows are what say the +change came from the indirection and not from ordinary symbol interposition. + +### Question 5 — the licence in the header now has an edge, and it is the cell + +This is the thing the next lane inherits, and it is worth more than either item above. + +`x86.ml`'s header licenses its own calling convention on the grounds that **a dev build is compiled entirely by this +backend and a release build entirely by LLVM, so the two never meet in one process.** That is what dissolved item 15's +sharpest obstacle and it is why there is no classifier in the file. + +**Publishing a cell an LLVM-built module can store into is the first thing that could make it false.** The two +conventions agree on scalars and disagree on every aggregate — this backend passes each by pointer and returns one +through a hidden `sret`, LLVM classifies — so an `Emit.redefinition` module dlopened into an `--x86` host would be +correct exactly until the first redefined function took or returned a struct. `cells.sh` does not reach it, because +the body it installs is `(i64, void *) -> i64` and the conventions agree there. + +Nothing in the toolchain does that today: `flan reload` and `flan dev` build host and module through LLVM together, +and neither accepts `--x86`. But the lane that wires this backend into the dev loop will be the one that does it, and +**the answer then is a redefinition emitter here, not a classifier.** Written into both `x86.ml`'s header and +`build.ml`'s refusal so that it is found before it is discovered. + +### The honest no-plan bucket + +- **A redefinition emitter**, which is question 5 and is new to this list: `Emit.redefinition` has no counterpart here, + so a `--x86 --dev` host has cells nothing in the toolchain can yet write. +- **`f64` → `i64` out of range**, and **`INT64_MIN / -1`**. Unchanged from items 15, 16 and 17: `idiv` raises `SIGFPE` + where LLVM says undefined. A language decision, not a backend one, and nobody has taken it in three reports. +- **`(uninit)` and `unreachable`** still differ from LLVM on purpose and are still written down only in `x86.ml`. +- **The `flan_transfer_fail` branch** — a defer starting a second transfer while the first unwinds. Emitted, refused + loudly, still untested. +- **`"defers on a transfer path nothing reaches"`** — still reached by no program in the corpus. +- **`flan_dev_reg_note` is not dropped in a release build here.** `emit.ml` drops the whole family when `dev` is off; + this emits real calls to a registry that is disabled, so they are no-ops that cost a call each. Correct, not free, + and `emit.ml`'s stated reason for the drop — an escaped alloca `mem2reg` would refuse — does not apply to a backend + with no `mem2reg`. +- **Debug information.** None. `--x86` and `--debug` together are still refused. +- **Code size and speed.** Still not measured, and the list of what to measure has not got shorter: a guard after every + call, a bounds check spending three frame temporaries, every intermediate in memory, `rep movsb` for a block copy — + and now an extra load at every call site in a dev build, which is the one item on this list that `emit.ml` pays too. + +### The verdict + +**Neither of the two correctness items was the shape it was written down as, and finding that out was most of the +work.** The container return convention did not exist to be built; the loose end item 17 flagged was the answer and +not a symptom, and one afternoon spent tracing it saved a SysV classifier nobody could have tested. The cell was real, +took thirty lines, and could not be tested by anything in the corpus — which is why the useful artefact from it is a +preloaded `dlsym` and not a program. + +What is left is a redefinition emitter, two arithmetic edge cases the language still has not decided, and no debug +info. The backend is no longer the thing standing between here and the dev loop. diff --git a/HANDOFF-x86-rt.md b/HANDOFF-x86-rt.md new file mode 100644 index 0000000..79b07f5 --- /dev/null +++ b/HANDOFF-x86-rt.md @@ -0,0 +1,165 @@ +# Handoff — the x86 backend's last two correctness items + +Branch: `dev-loop`, worktree `agent-af9064091c0602dc8`. Three commits plus this one; nothing is half-written and +nothing is reverted. The long-form report is **DISCUSS.md item 18**; this file is the operational version. + +## 1. The `flan_vec_as_slice` answer — got it, and it is complete + +**`flan_vec_as_slice` never reaches the aggregate-return refusal because its Flan-level return type is `Unit`.** + +Primary sources, all three agreeing: + +- `lib/check.ml:3721` — `rt loc Types.Unit "flan_vec_as_slice"` +- `runtime/flan_rt.c:1165` — `void flan_vec_as_slice(flan_vec *v, void *out, int32_t lo, int32_t hi, ...)` +- `lib/emit.ml:2464` — `declare void @flan_vec_as_slice(ptr, ptr, i32, i32, i64, ptr, i64, ptr)` + +In `lib/x86.ml`'s `call_native` the two tests are in this order: + +``` +if not (is_void rty) then begin + if is_agg rty then unsupported ... +``` + +`rty` is the *node's* type, which is `Unit`, so `is_void` answers first and `is_agg` is never evaluated. The slice +leaves through the `void *out` pointer the checker allocated with `fresh_slot`. + +**So the refusal is narrower than it reads — the first of the two possibilities the brief named. Nothing is going +right by accident.** It is not one symbol's quirk either; two rules in `check.ml` make the line unreachable for +*every* caller of that code path: + +1. **The `rt` out-pointer convention.** Every `rt loc ...` builder in `check.ml` answers `Unit`, an `Int`, a + `Ptr`, an `Alloc`, a `Handle` or `Int U64`. Enumerated exhaustively by grepping `Tast.Rt` construction sites — + there are 26 and none is aggregate-typed. `flan_pool_resolve` answers `Ptr elem` and the `Option` is built in Flan; + `Argv` is its own `Tast` node with its own out-pointer and does not come through `call_native` at all. +2. **`crossable`** (`lib/check.ml`, the `Ast.Declare` arm, ~line 5228), the other user of `call_native`, admits + `String`/`Slice _` only when `what = "a parameter"` and refuses an aggregate return from a `declare` outright. + +**Conclusion, and it changed the work: do not build sret-for-`Rt`.** That path is the *C* boundary, where the header +says the backend must match SysV rather than pick its own. A 16-byte slice comes back in `rax:rdx`, not through this +backend's internal hidden-pointer convention, and there is no classifier in the file. Building one would have been +untestable (nothing in the language can produce a call that needs it) *and* wrong (wrong convention). The refusal +stays as a guard against those two rules changing, with the reasoning in a comment and a message that now names SysV +classification instead of reading like a missing feature. + +**Consequence:** item 16's "`Vec`, `Map` and `Pool` have not been exercised at all" was already stale when item 17 +repeated it. Verified with the survey: `Vec`/`Map` via `vec.flan`, `vec-of-vec.flan`, `vec-in-struct.flan`, +`maps.flan`, `map-iter.flan`; **`Pool` — which neither report checked** — via `registry.flan`, `handles.flan`, +`generics.flan`, `pool-stale-region.flan`. All MATCH. + +## 2. Survey counts, measured + +`spike/x86/survey.sh`, unchanged in what it compares (stdout + stderr + exit status, same bounds-check setting both +sides). + +| | brief said | measured before | after | +|---|---|---|---| +| MATCH | 89 | **93** | **97** | +| DIFFER | 0 | 0 | **0** | +| refused by name | 0 | **2** | **0** | +| skip: does not compile / no main / forever | 25 / 6 / 2 | 28 / 6 / 2 | 28 / 6 / 2 | + +**The brief's 89/0/0 baseline was stale.** Another lane landed `(slice-from-ptr p n)` after item 17, and it arrived as +two refusals — `slice-from-ptr.flan` and `bounds.flan` — both reported as `x86: primitive with 2 arguments`. Fixing +that is commit 1 and is reported separately so the "after" number is not misread as this lane's work. + +Also measured, opt-in and new: `SURVEY_FLAGS=--dev spike/x86/survey.sh` → **97 MATCH / 0 DIFFER**. + +## 3. What was built, file by file + +All **working and verified**; nothing in this list is unverified or reverted. + +| file | state | what | +|---|---|---| +| `lib/x86.ml` — `prim`, `Tast.SliceFromPtr` case | working | new. Two stores; the length check is **signed** (`cc_ge`), not `check_slice`'s unsigned compare | +| `lib/x86.ml` — `call_native` | working | refusal reworded + the two `check.ml` invariants written down. No behaviour change | +| `lib/x86.ml` — `csym` | working | new, `"flan.cell." ^ n` quoted; must stay byte-identical to `Emit.cellname` | +| `lib/x86.ml` — `lower`, `FnAddr (Fnval _)` | working | splits from `Flanfn`; loads the cell when `md.dev` | +| `lib/x86.ml` — `lower`, `Tast.Call` | working | passes `` `Cell `` instead of `` `Sym `` when `md.dev` | +| `lib/x86.ml` — `call_flan` | working | new `` `Cell `` target: `mov r11, [rip+cell]; call r11`, **after** `emit_args` | +| `lib/x86.ml` — `emit_cells` | working | new. `.data`, `.globl`, `.quad `, one per `p.Tast.fns` | +| `lib/x86.ml` — `program` | working | now `~checks ?dev`; emits cells and the `flan_dev_reg_enable` ctor when `dev` | +| `lib/x86.ml` — `layout_ctx` | working | now `~checks ~dev`; `Emit.m.dev` is no longer hardcoded `false` | +| `lib/build.ml` | working | `--dev` removed from the `--x86` refusal list; `~dev:opts.dev` threaded to `X86.program` | +| `spike/x86/survey.sh` | working | `SURVEY_FLAGS`, given to **both** sides. Default unchanged | +| `spike/x86/p7-slice-from-ptr.flan` | working, MATCH | negative length through a parameter and a `restart-case` | +| `spike/x86/p8-cell.flan` | working, MATCH | direct call + function value, for `cells.sh` | +| `spike/x86/cell-override.c` | working | `dlsym("flan.cell.twice")` + store, in a constructor | +| `spike/x86/cells.sh` | working, 4/4 ok | the only test of the cell that can exist | + +`lib/emit.ml` was **not** modified. No change to it was needed. + +## 4. What did not work, with the errors + +Nothing fought for an hour. Four short false starts, all mine and all one-line: + +- `handler-bind` clause syntax guessed as an `fn` literal: + `spike/x86/p7-slice-from-ptr.flan:30:18: a handler-bind clause is (Type [name] body ...)`. + The form is `(BoundsError [c] body ...)`; `test/programs/bounds-condition.flan:112` is the model. +- `(defn show [name [u8] ...])` for a literal argument: + `spike/x86/p7-slice-from-ptr.flan:37:14: expected [u8], found string`. A string literal wants `string`, not `[u8]`, + even though they are the same two words at the machine level. +- `cmp_imm` takes `~dst` and an `int`, not `~reg` and an `Int64`. +- `cell-override.c`: `error: 'NULL' undeclared` — needs `` beside ``. + +One environment note for the next lane, not a failure of this work: `dune test --root .` prints +`/usr/bin/ld: cannot open output file /tmp/build_*_dune/flan-devtest-robust.cache/flan-macros-*.so.*: Permission +denied` and `clang: error: linker command failed with exit code 1` twice. That is **inside the `dev-robust` fixture**, +which exists to prove a failed build leaves the session standing; the run still exits 0. Item 17's four raylib +fixtures did **not** fail here — `/tmp` had room throughout (6% used at start and at finish). + +## 5. Was `Fnval`'s cell reached, and the `--dev` call + +**Yes, reached and tested.** And the test is the interesting part, because *the corpus cannot do it*: a dev build +starts with every cell pointing at the body that build compiled, so it prints exactly what a release build prints +whether or not anything reads the cell. `spike/x86/cells.sh` preloads a `.so` whose constructor `dlsym`s +`flan.cell.twice` (the cells are in `.dynsym` — a dev build is `-rdynamic`) and stores a different body there. Four +builds; the two release rows are the control that says the effect is the indirection and not symbol interposition: + +``` +ok llvm --dev: 22 22 +ok x86 --dev: 22 22 +ok llvm : 42 42 +ok x86 : 42 42 +``` + +Also checked: `nm -D` over an LLVM `--dev` build and an x86 `--dev` build of the same program gives **identical sets of +68 `flan.cell.*` symbols**. That is the property the later lane depends on. + +**My call: yes, relax `--x86` with `--dev`, and it is relaxed.** The argument is narrow and verified: `--x86` is read +in exactly one place, `flan build`'s argument list in `bin/main.ml`. `flan dev` and `flan reload` build host and +module through `Build.executable` / `Build.shared` with no `x86` field set, and there is no spelling that hands them +one. So the daemon is unchanged and cannot reach the new path. What `--x86 --dev` gives is **a host whose call sites +are redefinable**; what it does not give is anything in the toolchain that can write a cell, because +`Emit.redefinition` has no counterpart here. + +**The one thing the next lane must read before it writes that counterpart** (now in `x86.ml`'s header and +`build.ml`'s refusal comment too): `x86.ml` licenses its own calling convention on the grounds that a dev build is +compiled entirely here and a release build entirely by LLVM, *so the two never meet in one process*. A cell an +LLVM-built module can store into is the first thing that can make that false. The two conventions agree on scalars and +disagree on **every aggregate** — here each goes by pointer with a hidden `sret`; LLVM classifies. An +`Emit.redefinition` module dlopened into an `--x86` host would be correct until the first redefined function took or +returned a struct. `cells.sh` does not reach it: the body it installs is `(i64, void *) -> i64`. +**The answer is a redefinition emitter here, not a classifier.** + +## 6. What remains, in the order to do it + +1. **A redefinition emitter in `lib/x86.ml`** — the counterpart to `Emit.redefinition`, producing a `.so`: cells as + `.globl` externs rather than definitions, bodies hidden, `Emit.cellptr`'s deeper spelling for names the host lacks, + the `flan_dev_cell` / `flan_dev_global` lookups, and a publish function. This is the only item that unblocks the + dev loop, and question 5 above is why it cannot be skipped by leaning on LLVM for modules. +2. **`flan_dev_reg_note` dropped in a release build** — in `prim`'s `Tast.Rt` dispatch, matching `emit.ml:1918`'s + `when (not f.md.dev) && ...` arm. Today `x86.ml` emits real calls into a disabled registry: correct, no-ops, one + call each. Cheap; a code-size item, not a correctness one. +3. **`f64` → `i64` out of range**, and **`INT64_MIN / -1`**, in `prim`'s `Tast.Cast` and `Div`/`Rem` arms. `idiv` + raises `SIGFPE` where LLVM says undefined. Blocked on a *language* decision, unchanged across items 15, 16, 17. +4. **The `flan_transfer_fail` branch** — a defer starting a second transfer while the first unwinds. Emitted in + `transfer_exit`, refused loudly, reached by no program. Needs a probe in `spike/x86`. +5. **`"defers on a transfer path nothing reaches"`** — the refusal in `emit_fn`. Same: exists so that if the reasoning + is wrong it says so, and no program reaches it. +6. **Debug information.** None; `--x86 --debug` still refused in `build.ml`. +7. **Code size and speed.** Still unmeasured, and the list grew: a guard after every call, three frame temporaries per + bounds check, every intermediate in memory, `rep movsb` block copies, and now an extra load per call site in a dev + build — which is the one item `emit.ml` pays too. + +**Also worth doing and not a backend item: run `spike/x86/survey.sh` in CI.** The 2 refusals this lane found were a +month-old lane's new prim, and nothing noticed. A backend that refuses by name does not rot quietly, but it does rot.