wasm-tier-2-phase-B-and-C #28

Merged

navicore merged 3 commits from wasm-tier-2-phase-B-and-C into main

2026-06-20 22:08:20 +00:00

navicore commented

2026-06-20 20:58:32 +00:00

Owner

No description provided.

navicore added 2 commits

2026-06-20 20:58:32 +00:00

● Phase B — codegen: reactor target ✅ 38323574c9

B1 — Target::Worker arm (lib.rs, main.rs)
  - New Target::Worker variant, documented as the Tier-2 reactor (wasm32-unknown-unknown, no
  main, exports plg_init + the buffer ABI).
  - compile_files routes Worker to an explicit "link step lands in Phase C" error — the IR is
  written correctly first, so --keep-ir lets you inspect it. This is the clean B/C seam
  (codegen now, linker next), and it keeps the tree compiling and green.
  - compile_to_ir_target(source, target) added (with compile_to_ir delegating to it for Native)
  so the reactor entry can be pinned in golden tests the same way native is.
  - main.rs output-extension arm gives Worker a .wasm extension.

  B2 — reactor entry codegen (codegen/program.rs)
  - declare void @plg_rt_set_machine(ptr) added to RUNTIME_DECLS.
  - Triple arm: Worker → "wasm32-unknown-unknown".
  - Entry emission restructured: the shared plg_rt_init(...) call is factored once, then Worker
  emits an exported define void @plg_init() that builds the Machine and calls
  plg_rt_set_machine (no main, no argv, no plg_rt_main); the CLI targets keep their
  main/__main_argc_argv path unchanged.

  B3 — golden IR tests (tests/golden_ir.rs)
  - reactor_target_emits_plg_init_and_no_main: pins the wasm32-unknown-unknown triple, define
  void @plg_init(), the plg_rt_set_machine hand-off, and the shared init call — and asserts no
  @main, no __main_argc_argv, no call @plg_rt_main.
  - Added the complement native_target_emits_main_not_plg_init so a future regression in either
  direction is caught.

  Verification

  - Full workspace suite green (golden IR now 17 tests, was 15); the v1 differential /
  integration suites confirm native codegen is byte-identical — the entry refactor changed
  nothing for Native/Wasm.
  - fmt + clippy clean workspace-wide.

  One honest scope note: B's verification is structural (golden IR). The IR-is-valid-LLVM gate
  is Phase C — wasm-ld consuming the module is what will hard-fail malformed IR. The reactor
  entry mirrors the known-good native entry, so the risk is low, but it isn't proven-valid
  until C links it. That's by the plan's design (B = codegen, C = link), and it's also why
  these two belong in one PR — the worker target isn't reachable end-to-end (or CLI-selectable
  via parse_target, which is C3) until C lands.

  C (link_wasm_reactor + build.rs/feature embed + CLI wiring) is the next block whenever you're
  ready.

● Phase C — link + build + CLI ✅

CI - Linux / CI - Linux x86_64 (pull_request) Successful in 1m34s

Details

fc56ee4460

C2 — embed the reactor archive (D5) (lib.rs, build.rs, Cargo.toml, justfile)
  - WORKER_RUNTIME_LIB added (parallel to WASM_RUNTIME_LIB), Some exactly when the wasm feature
  is on.
  - build.rs refactored to a shared embed_wasm_archive helper; under CARGO_FEATURE_WASM it now
  embeds both wasm32-wasip1 and wasm32-unknown-unknown archives — one feature, one install
  story (D5).
  - just build-runtime-wasm-reactor added; install-wasm now depends on both archive builds.
  Feature doc updated.

  C1 — link_wasm_reactor (link.rs)
  - Extracted llvm_tools() (llc + rust-lld discovery) so Tier 1 (wasm_toolchain, which also
  needs wasi-libc) and the reactor (which needs no libc) share it.
  - New link_wasm_reactor: llc -mtriple=wasm32-unknown-unknown -mattr=+tail-call → obj, then
  wasm-ld --no-entry --allow-undefined --export=… against the reactor archive — no crt, no -lc.
  Exports pinned in a REACTOR_EXPORTS const.

  C3 — CLI wiring (main.rs, lib.rs)
  - parse_target accepts worker and wasm32-unknown-unknown (D1); supported-list message +
  --target help updated.
  - compile_files routes Target::Worker → link_wasm_reactor (the Phase-B error stub is gone).
  - New parse_target test module pinning all target mappings + the rejection message.

  Verification — proven end-to-end at the link level

  Built both archives, built plgc --features wasm, and ran plgc build examples/deps.pl --target
  worker:
  - Produces a valid 1.7M wasm32-unknown-unknown module (matches the gate's ~1.65M).
  - Exports are exactly plg_init, plg_rt_run_query, plg_rt_alloc, plg_rt_free + memory — and no
  _start (--no-entry confirmed).
  - Zero imports — fully self-contained, no WASI calls; instantiable in a bare V8 isolate.
  --allow-undefined turned out to be a no-op here, which is the ideal outcome.

  Full workspace suite green; clippy clean on default and --features wasm; fmt clean. Default
  cargo install is untouched (worker archive embeds only under the feature; WORKER_RUNTIME_LIB
  is None otherwise — and binary_size + the standalone-contract tests still pass).

  Scope boundary: C produces and validates the .wasm; it does not yet run it through V8 over
  HTTP — that needs the JS glue (worker.js/wrangler.toml) and a workerd/Node driver, which is
  Phase D. So the "answers HTTP byte-identical to native" DoD gate lands with D (+ the
  automated 1M-deep proof in E). Two toolchain prereqs got installed along the way
  (wasm32-unknown-unknown, llvm-tools-preview, and wasm32-wasip1 which was also missing) — all
  are the documented local prereqs.

  D (Worker glue + wasm-worker-serve) is the next block — and it's the one that finally
  exercises the reactor in a real isolate.

navicore commented

2026-06-20 21:42:23 +00:00

Author

Owner

Review — #28 Phase B+C: codegen reactor target + link/build/CLI

Clean target addition. The Worker variant slots into the existing Target machinery without disturbing the native/Tier-1 paths, and the factoring is good: the shared init_call string means both entry shapes build the Machine identically and only diverge on what they do with it; llvm_tools() is extracted as the common toolchain locator so wasm_toolchain (Tier 1, needs wasi-libc) and link_wasm_reactor (Tier 2, needs no libc) don't duplicate the sysroot/rust-lld discovery; and embed_wasm_archive dedups the build.rs panic+env-var logic across the two archives. The "one wasm feature embeds both archives" call (D5) keeps the install story to a single just install-wasm, and WORKER_RUNTIME_LIB being Some exactly when WASM_RUNTIME_LIB is means a --target worker request on a non-wasm build fails with the same actionable message. I verified the four REACTOR_EXPORTS names match the runtime's actual #[no_mangle] symbols (plg_init from codegen; plg_rt_run_query/plg_rt_alloc/plg_rt_free in reactor.rs), and that plg_rt_set_machine is correctly not exported (it's reached only through plg_init). -mattr=+tail-call is carried into the reactor llc invocation with the rationale comment — the load-bearing flag.

Real concerns

1. The doc comment references a "reactor smoke test" that doesn't exist. link_wasm_reactor:

--allow-undefined defers any runtime import to instantiation (mirrors Tier 1) — a genuinely missing import then surfaces when the host instantiates the module, which the reactor smoke test runs.

There is no reactor smoke test in the repo. The only smoke recipe is wasm-smoke, which is Tier 1 (wasm32-wasi, driven by wasmtime) and is itself marked "LOCAL only — not in CI yet" — it never touches the reactor ABI. So the comment promises coverage that isn't there. Either add the test or drop the clause.

2. The entire link_wasm_reactor path has zero automated coverage, and --allow-undefined hides the worst failure mode. The two new golden_ir tests are good, but they only exercise codegen — compile_to_ir_target(..., Worker) stops before linking. Nothing in the PR links a Worker module and asserts that (a) it instantiates and (b) the four REACTOR_EXPORTS are actually present as wasm exports. That matters specifically because of the flag combination: with --allow-undefined, a missing or misnamed export degrades to a wasm import rather than a link error — so "the host can't find plg_rt_run_query" is exactly the failure that won't surface at build time. CI doesn't run wasm at all, so a regression in the link flags, an export rename, or a stray undefined import that workerd doesn't provide would all ship green. The gate spike proved workerd works, but that was the throwaway wasm-t2 branch; nothing here keeps it from regressing. A reactor-smoke recipe — even local-only, mirroring wasm-smoke, that compiles --target worker, instantiates under Node/workerd, and round-trips one query while asserting the four exports exist — would close both #1 and #2 at once.

Small observations

3. Both wasm targets default -o to stem.wasm. Target::Wasm | Target::Worker => stem.with_extension("wasm"), so compiling the same source to both targets without an explicit -o silently overwrites. Minor (users pass -o), but a distinct default (e.g. .worker.wasm) would remove the footgun.

4. No golden test pins the Tier-1 wasm32-wasi entry. The new tests cover native (@main) and worker (@plg_init), but the __main_argc_argv entry for Tier 1 now lives in a nested arm of the restructured match. Since this PR reshaped that match, a third assertion locking the Tier-1 entry would pin all three entry shapes against future drift. Pre-existing gap, cheap to close while you're here.

Net: #2 is the one I'd want addressed before merge — the reactor link path is the substance of this PR and currently the only thing exercising it is a comment pointing at a test that doesn't exist. The codegen/CLI/build plumbing itself looks correct.

## Review — #28 Phase B+C: codegen reactor target + link/build/CLI Clean target addition. The `Worker` variant slots into the existing `Target` machinery without disturbing the native/Tier-1 paths, and the factoring is good: the shared `init_call` string means both entry shapes build the Machine identically and only diverge on what they do with it; `llvm_tools()` is extracted as the common toolchain locator so `wasm_toolchain` (Tier 1, needs wasi-libc) and `link_wasm_reactor` (Tier 2, needs no libc) don't duplicate the sysroot/`rust-lld` discovery; and `embed_wasm_archive` dedups the build.rs panic+env-var logic across the two archives. The "one `wasm` feature embeds both archives" call (D5) keeps the install story to a single `just install-wasm`, and `WORKER_RUNTIME_LIB` being `Some` exactly when `WASM_RUNTIME_LIB` is means a `--target worker` request on a non-wasm build fails with the same actionable message. I verified the four `REACTOR_EXPORTS` names match the runtime's actual `#[no_mangle]` symbols (`plg_init` from codegen; `plg_rt_run_query`/`plg_rt_alloc`/`plg_rt_free` in reactor.rs), and that `plg_rt_set_machine` is correctly *not* exported (it's reached only through `plg_init`). `-mattr=+tail-call` is carried into the reactor `llc` invocation with the rationale comment — the load-bearing flag. ### Real concerns **1. The doc comment references a "reactor smoke test" that doesn't exist.** `link_wasm_reactor`: > `--allow-undefined` defers any runtime import to instantiation (mirrors Tier 1) — a genuinely missing import then surfaces when the host instantiates the module, **which the reactor smoke test runs.** There is no reactor smoke test in the repo. The only smoke recipe is `wasm-smoke`, which is Tier 1 (`wasm32-wasi`, driven by `wasmtime`) and is itself marked "LOCAL only — not in CI yet" — it never touches the reactor ABI. So the comment promises coverage that isn't there. Either add the test or drop the clause. **2. The entire `link_wasm_reactor` path has zero automated coverage, and `--allow-undefined` hides the worst failure mode.** The two new golden_ir tests are good, but they only exercise *codegen* — `compile_to_ir_target(..., Worker)` stops before linking. Nothing in the PR links a Worker module and asserts that (a) it instantiates and (b) the four `REACTOR_EXPORTS` are actually present as wasm exports. That matters specifically because of the flag combination: with `--allow-undefined`, a missing or misnamed export degrades to a wasm *import* rather than a link error — so "the host can't find `plg_rt_run_query`" is exactly the failure that won't surface at build time. CI doesn't run wasm at all, so a regression in the link flags, an export rename, or a stray undefined import that workerd doesn't provide would all ship green. The gate spike proved workerd works, but that was the throwaway `wasm-t2` branch; nothing here keeps it from regressing. A `reactor-smoke` recipe — even local-only, mirroring `wasm-smoke`, that compiles `--target worker`, instantiates under Node/workerd, and round-trips one query while asserting the four exports exist — would close both #1 and #2 at once. ### Small observations **3. Both wasm targets default `-o` to `stem.wasm`.** `Target::Wasm | Target::Worker => stem.with_extension("wasm")`, so compiling the same source to both targets without an explicit `-o` silently overwrites. Minor (users pass `-o`), but a distinct default (e.g. `.worker.wasm`) would remove the footgun. **4. No golden test pins the Tier-1 `wasm32-wasi` entry.** The new tests cover native (`@main`) and worker (`@plg_init`), but the `__main_argc_argv` entry for Tier 1 now lives in a nested arm of the restructured `match`. Since this PR reshaped that match, a third assertion locking the Tier-1 entry would pin all three entry shapes against future drift. Pre-existing gap, cheap to close while you're here. Net: #2 is the one I'd want addressed before merge — the reactor link path is the substance of this PR and currently the only thing exercising it is a comment pointing at a test that doesn't exist. The codegen/CLI/build plumbing itself looks correct.

navicore referenced this pull request from a commit

2026-06-20 22:03:21 +00:00

PR #28 feedback addressed

navicore added 1 commit

2026-06-20 22:03:21 +00:00

PR #28 feedback addressed

CI - Linux / CI - Linux x86_64 (pull_request) Successful in 1m1s

Details

9aea55dc3f

#1 — stale doc comment, fixed. link_wasm_reactor's comment now names the real recipe (just
  reactor-smoke) and explains what it checks (instantiation + the four exports, since
  --allow-undefined would otherwise hide a missing export).

  #2 — reactor link path now has real coverage (the must-fix). New just reactor-smoke
  (local-only, mirroring wasm-smoke) + a committed scripts/reactor-smoke.mjs driver:
  - Asserts the four exports (plg_init, plg_rt_run_query, plg_rt_alloc, plg_rt_free) + memory
  exist — directly closing the --allow-undefined-hides-a-missing-export hole the review
  flagged.
  - Instantiates under Node's V8 (the Workers engine) and round-trips queries byte-identically
  to native (stripping the reactor's always-present ,"output":"" field before compare).
  - Automates the headline gate: 1,000,000-deep call/1 runs in constant stack in the isolate
  via return_call — proving the musttail lowering holds on V8, now as a repeatable check rather
  than the throwaway spike.
  - Ran it: all three queries ✅ + the 1M-deep proof ✅.

  #3 — overwrite footgun removed. The worker target now defaults -o to stem.worker.wasm (Tier 1
  stays stem.wasm), so compiling one source to both targets without -o no longer silently
  overwrites.

  #4 — all three entry shapes now pinned. Added tier1_wasm_target_emits_argc_argv_entry
  alongside the native and reactor golden tests, locking __main_argc_argv against drift in the
  restructured match (golden_ir now 18 tests).

  Full workspace green, fmt + clippy clean.

  Note on the standing caveat the review made (CI doesn't run wasm): reactor-smoke is
  local-only by design, same as wasm-smoke — wiring both into a separate CI job is Phase E
  (E2), which the plan already scopes. D (Worker glue + wasm-worker-serve) is next.