ReviewOS

zig-utils/zig-js

VM: remove legacy packed-array sum loop body kernel

#1077
Closed chris opened this 8 days ago ·
8 days ago

Parent: #498 Related: #1074, #1076, #491

The live quickening inventory classifies packed-array sum as legacy_pattern_kernel / legacy_narrow. The VM recognizes one exact eighteen-bytecode counted loop, then substitutes a batch of numeric element reads and local updates. A separate compile-time candidate hint admits that body. This conflicts with the no body/benchmark recognizer constraint.

Executed baseline on c061420f: the existing kernel-hit witness passes 2/2, explicitly asserting sum-kernel and push-kernel hits in isolated and synchronized modes. sum(fill()) returns 36 with exactly 316 logical steps in each mode.

An additional executed probe finds a counter/accumulator aliasing bug: aliasedSum([4,4,4]) returns 3 in the parent VM, versus 5 in Node and the tree walker. The changed VM returns 5, pinned in isolated and shared execution. The old batch computed counter and accumulator separately, then overwrote their shared slot with its counter.

Acceptance:

  • Remove the packed-sum candidate hint, exact body recognizer/executor, sum plan type/tag, and hit metric; retain general indexed, length, and intrinsic-push opcode paths.
  • Ordinary execution preserves aliased counter assignments and retains result 36 and 316 steps in both modes, and preserves holes, inherited elements, accessors, Proxy ordering, numeric coercion, and live array mutation.
  • Actual no-GIL workers, moving-GC roots, source hooks, budget/stop checkpoints, and allocation failures are covered.
  • Focused normal/ReleaseSafe/unsuppressed TSan checks and exact before/after affected test262 and tree/required-VM comparisons show no new failures.
  • Regenerate the live inventory/docs/manifest; preserve historical evidence and explicit labels for remaining legacy work.

This is removal of body-specific work substitution, not current throughput evidence or completion of #498.

8 days ago

The removal also fixes an executed aliasing bug in the old kernel:

function aliasedSum(values) {
  let i = 0;
  while (i < values.length) {
    i = i + values[i];
    i = i + 1;
  }
  return i;
}
aliasedSum([4, 4, 4]);

Node and the parent tree walker return 5. The parent required VM returns 3. The changed tree walker and required VM both return 5.

The kernel's counter and accumulator locals can be the same binding. Its batch maintains them separately, then writes the counter over the shared slot. Ordinary bytecode preserves the intervening assignment, so the first iteration sets i to 4 and then 5 before rechecking the bound. A regression now pins this in isolated and shared execution; focused sanitizer/ReleaseSafe checks are in progress.

The completed exact before/after corpus comparison remains unchanged across 3,552 cases, with 0 diagnostic failures and the same five pre-existing tail-call differences.

8 days ago

Landed in 3ffad54f.

Removed the exact eighteen-bytecode body recognizer/executor, compile-time packed-sum candidate hint, sum plan type/tag, and hit metric. General indexed, length, and intrinsic-push opcode paths remain.

This also fixes the executed counter/accumulator aliasing bug above: the parent VM returns 3, versus 5 in Node and the tree walker. The changed VM returns 5, pinned in isolated and shared execution.

Validation:

  • Original sum(fill()): 36 and 316 logical steps in both synchronization modes.
  • Execution group 6/6, alias regression 2/2, and relocation regression 2/2, each in normal, ReleaseSafe, and unsuppressed TSan. Coverage includes actual four-worker no-GIL execution, Node-confirmed Proxy/getter/coercion/mutation effects, exception identity, eight source-hook updates, and exact budget/stop checkpoints. The moving-GC test requires the protected Array's address to change, then checks the warmed sum returns 36 and a subsequent indexed update produces 72.
  • Expanded activation allocation-failure test 2/2; retained array fast-path test 2/2.
  • Exact parent/after diagnostic and tree/required-VM comparisons: Array 3,081, for 385, while 38, addition 48 — 3,552 total, 0 diagnostic failures before → after, 0 flips. The same five pre-existing tail-call differences remain; each passes in the VM and fails in the tree walker on both binaries.
  • Inventory self-test and validation pass with 13 guarded families, 5 remaining legacy families, and 5 explicit unsupported families. Docs links pass for 378 files; the 51-page build matches the refreshed manifest.

These are scoped local checks; full integration gates run in CI. Historical performance artifacts remain unchanged, and this removal does not establish current throughput or complete #498.

Sign in to comment on this issue.