Keep hole-free prefix and int32 parameter upkeep off hot paths (#922)

packages/compiler/src/backend/llvm/dense-array-access.test.ts

@@ -196,9 +196,20 @@ test.each([32, 64] as const)(
     expect(writes[0]).toMatch(/store double .*, ptr /);
     // The replaced string below the bound is released after the store.
     expect(writes[1]).toMatch(/store ptr .*\n[^]*call void @scr_str_release\(/);
-    // A state-checked store at the bound extends it.
+    // A state-checked store at the bound extends it to the index plus one,
+    // not to the loaded bound plus one (no store-to-load chain in a loop).
     expect(fn).toMatch(new RegExp(`icmp eq ${size} .*`));
-    expect(fn).toMatch(/store i32 .*, ptr /);
+    const bumps = blocks("arr\\.set\\.bump");
+    expect(bumps).toHaveLength(2);
+    const loadedBounds = new Set(
+      [...fn.matchAll(/^\s*(%[\w.]+) = load i32, ptr /gm)].map((m) => m[1]),
+    );
+    for (const block of bumps) {
+      const next = block.match(/= add i32 (%[\w.]+), 1\n/);
+      expect(next).not.toBeNull();
+      expect(loadedBounds.has(next![1]!)).toBe(false);
+      expect(block).toMatch(/store i32 .*, ptr /);
+    }
   },
 );
 

packages/compiler/src/backend/llvm/dense-array-access.ts

@@ -399,9 +399,16 @@ export function emitDenseArraySet(
     afterBump = B.newLabel("arr.set.state");
   B.condBr(continues, bumpLabel, afterBump);
   B.startBlock(bumpLabel);
+  // The new bound comes from the index, not from the loaded bound: a loop
+  // of appending stores then carries no store-to-load chain through it.
+  let index32 = offset;
+  if (host.sizeType === "i64") {
+    index32 = B.tmp();
+    B.line(`${index32} = trunc i64 ${offset} to i32`);
+  }
   const nextFilled = B.tmp(),
     filledPtr = B.tmp();
-  B.line(`${nextFilled} = add i32 ${guard.filled}, 1`);
+  B.line(`${nextFilled} = add i32 ${index32}, 1`);
   B.line(
     `${filledPtr} = getelementptr inbounds %ScrArr, ptr ${array}, i32 0, i32 ${SCR_ARR_FIELD.filled}`,
   );

packages/compiler/src/backend/llvm/emitter.ts

@@ -8510,11 +8510,15 @@ export class LlEmitter {
   }
 
   /** The number of an int32-stored local's i32, with the i32 as its
-   * integer and ToUint32 forms. */
+   * integer and ToUint32 forms. A parameter the body never writes reads
+   * as its incoming double, which is the same number. */
   int32LocalValue(localId: string, bits: string, type: IrType): LlValue {
     const local = this.int32Locals.get(localId)!;
-    const number = this.B.tmp();
-    this.B.line(`${number} = ${local.signed ? "sitofp" : "uitofp"} i32 ${bits} to double`);
+    let number = `%p_${mangleLocal(localId)}`;
+    if (!local.incoming) {
+      number = this.B.tmp();
+      this.B.line(`${number} = ${local.signed ? "sitofp" : "uitofp"} i32 ${bits} to double`);
+    }
     return {
       name: number,
       type,

packages/compiler/src/backend/llvm/int32-locals.ts

@@ -14,6 +14,9 @@ import { everyExprChild, everyStmtChild } from "../../ir/traverse.js";
 export interface Int32Local {
   signed: boolean;
   range: IntegerRange;
+  /** A parameter the body never writes: its incoming double is still
+   * its value, so reads use it instead of widening the i32 again. */
+  incoming?: boolean;
 }
 
 /** Number locals whose every write is an exact int32 (or every write an
@@ -51,9 +54,12 @@ export function findInt32Locals(
     if (captured.has(local.id) || excluded.has(local.id)) continue;
     writes.set(local.id, []);
   }
+  const params = new Set<string>();
   for (const param of fn.params) {
     const list = writes.get(param.localId);
-    if (list) list.push(facts.params?.get(param.localId) ?? contract(param.localId));
+    if (!list) continue;
+    list.push(facts.params?.get(param.localId) ?? contract(param.localId));
+    params.add(param.localId);
   }
   const write = (id: string, range: IntegerRange | null | undefined, value?: IrExpr): void => {
     const list = writes.get(id);
@@ -95,8 +101,10 @@ export function findInt32Locals(
       range.min = Math.min(range.min, write.min);
       range.max = Math.max(range.max, write.max);
     }
-    if (withinInt32(range)) result.set(id, { signed: true, range });
-    else if (withinUint32(range)) result.set(id, { signed: false, range });
+    if (!withinInt32(range) && !withinUint32(range)) continue;
+    const local: Int32Local = { signed: withinInt32(range), range };
+    if (params.has(id) && list.length === 1) local.incoming = true;
+    result.set(id, local);
   }
   return { locals: result, values };
 }

packages/compiler/test/int32-local-emission.test.ts

@@ -78,10 +78,13 @@ function contract(seed: Int32, steps: Int32): Int32 {
   for (let i: Int32 = 0; i < steps; i = (i + 1) | 0) acc = (Math.imul(acc, 3) + i) | 0;
   return acc;
 }
+function fibo(n: number): number {
+  return n < 2 ? n : fibo(n - 1) + fibo(n - 2);
+}
 const bytes = new Uint8Array([1, 2, 3, 250]);
 console.log(rsync(bytes), mix("abcd"), counters(), unsigned(new Uint32Array([4294967295, 7])));
 console.log(fraction(5), mixed(3), captured(["ab", "c"]), capturedParam(3)());
-console.log([...frames(["ab", "cd"])].join(" "), contract(7, 20));
+console.log([...frames(["ab", "cd"])].join(" "), contract(7, 20), fibo(10));
 suspended(["ab"]).then((t) => console.log(t));
 `;
 
@@ -208,6 +211,17 @@ test("int32-closed locals are stored as i32 and keep doubles only for floating c
       expect(contract).toMatch(/alloca i32 ; seed\n/);
       expect(contract).toMatch(/ = fptosi double %p_\w+ to i32\n/);
       expect(carriers(contract)).toEqual([]);
+
+      // A parameter the body never writes reads as its incoming double:
+      // the base case returns it without converting the i32 back.
+      const fibo = live(body(ll, "fibo"));
+      expect(fibo).toMatch(/alloca i32 ; n\n/);
+      const nReads = new Set(
+        [...fibo.matchAll(/^\s*(%[\w.]+) = load i32, ptr %sc_l_n_\w+$/gm)].map((m) => m[1]),
+      );
+      const widened = [...fibo.matchAll(/^\s*%[\w.]+ = sitofp i32 (%[\w.]+) to double$/gm)];
+      expect(widened.filter((m) => nReads.has(m[1]))).toEqual([]);
+      expect(fibo).toMatch(/store double %p_sc_l_n_\w+, ptr /);
     }
 
     // Debug builds keep every number local a double for the variable view.

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/4910-hole-free-prefix-upkeep.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/4910-hole-free-prefix-upkeep.ts"
+ ],
+ "diags": []
+}

packages/runtime/src/scr_array.c

@@ -531,7 +531,7 @@ static void scr_arr_refill_to(ScrArr *a, size_t limit) {
   size_t end = a->len < a->cap ? a->len : a->cap;
   if (limit < end) end = limit;
   size_t f = a->filled;
-  while (f < end && a->present[f] == SCR_ARR_VALUE) f++;
+  if (f < end) f = scr_arr_value_run(a->present, f, end);
   a->filled = (uint32_t)f;
 }
 
@@ -2032,6 +2032,9 @@ ScrArr *scr_arr_slice(ScrArr *a, double start, double end) {
   out->len = n;
   if (to <= a->cap) {
     scr_arr_copy_dense(out, 0, a, from, n, false, false);
+    /* States copied from below the source's bound are present values. */
+    if (n > 0 && from < a->filled)
+      out->filled = (uint32_t)((to < a->filled ? to : a->filled) - from);
     scr_arr_refill(out);
     return out;
   }

packages/runtime/src/scr_runtime.h

@@ -1648,12 +1648,27 @@ static inline void scr_arr_cut(ScrArr *a, size_t index) {
   if (a->filled > index) a->filled = (uint32_t)index;
 }
 
+/* The first index in [f, end) whose state is not a present value (or end).
+ * Compares eight states per step: bulk copies (slice, concat, spread) end
+ * with this scan over the whole copied range. */
+static inline size_t scr_arr_value_run(const uint8_t *present, size_t f, size_t end) {
+  const uint64_t values = 0x0101010101010101ull * SCR_ARR_VALUE;
+  while (end - f >= 8) {
+    uint64_t word;
+    memcpy(&word, present + f, sizeof word);
+    if (word != values) break;
+    f += 8;
+  }
+  while (f < end && present[f] == SCR_ARR_VALUE) f++;
+  return f;
+}
+
 /* Extend the hole-free prefix over populated dense states. Starts at the
  * current bound, so after an append it costs only the appended range. */
 static inline void scr_arr_refill(ScrArr *a) {
   size_t end = a->len < a->cap ? a->len : a->cap;
   size_t f = a->filled;
-  while (f < end && a->present[f] == SCR_ARR_VALUE) f++;
+  if (f < end) f = scr_arr_value_run(a->present, f, end);
   a->filled = (uint32_t)f;
 }
 

packages/runtime/test/test_array.c

@@ -1322,6 +1322,25 @@ static void test_filled_prefix(void) {
   s = scr_arr_slice(b, 0, INFINITY);
   check(s->filled == 2 && filled_ok(s), "slice keeps holes out of the bound");
   scr_arr_release(s);
+  /* The rescan after a copy stops exactly at the first non-value state,
+   * whichever eight-state step holds it, and a slice that starts past a
+   * source's bound still finds the values after it. */
+  for (size_t hole = 0; hole < 37; hole++) {
+    ScrArr *w = scr_arr_new(SCR_ELEM_F64, 0);
+    for (int i = 0; i < 37; i++) scr_arr_push_f64(w, i);
+    if (hole % 2) scr_arr_set_undefined(w, (double)hole);
+    else scr_arr_delete(w, (double)hole);
+    for (size_t from = 0; from < 20; from += 3) {
+      ScrArr *c = scr_arr_slice(w, (double)from, INFINITY);
+      size_t want = hole >= from ? hole - from : 37 - from;
+      check(c->filled == want && filled_ok(c), "slice bound ends at the first non-value");
+      scr_arr_release(c);
+    }
+    ScrArr *c = scr_arr_slice(w, 30, 10);
+    check(c->len == 0 && c->filled == 0, "an empty slice has an empty bound");
+    scr_arr_release(c);
+    scr_arr_release(w);
+  }
   scr_arr_release(scr_arr_reverse(b));
   check(filled_ok(b), "reverse of a holey array recomputes the bound");
   scr_arr_release(scr_arr_sort_primitive(b, false));

tests/corpus/4910-hole-free-prefix-upkeep.ts

@@ -0,0 +1,119 @@
+// Hole-free prefix upkeep after appending stores and bulk copies, and
+// int32 parameters that the body reads but never writes.
+//
+// Appending stores into `new Array(n)` extend the prefix one index at a
+// time; stores past it leave gaps that later stores close. Bulk copies
+// (slice, concat, spread, fill) rescan the copied states, so holes and
+// present undefined land at every position around eight-state steps.
+function show(label: string, xs: (number | undefined)[]): void {
+  const parts: string[] = [];
+  for (let i = 0; i < xs.length; i++) parts.push(i in xs ? String(xs[i]) : "_");
+  let visits = 0;
+  xs.forEach(() => visits++);
+  console.log(label, xs.length, parts.join(","), "visits", visits);
+}
+
+// A hole at `at` without `delete`: copying a hole deletes the target.
+function punch<T>(xs: T[], at: number): void {
+  const n = xs.length;
+  xs.length = n + 1;
+  xs.copyWithin(at, n, n + 1);
+  xs.length = n;
+}
+
+function prefixSums(xs: number[]): number[] {
+  const out = new Array<number>(xs.length);
+  let s = 0;
+  for (let i = 0; i < xs.length; i++) {
+    s += xs[i];
+    out[i] = s;
+  }
+  return out;
+}
+
+function prefixInPlace(xs: number[]): void {
+  for (let i = 1; i < xs.length; i++) xs[i] += xs[i - 1];
+}
+
+const base: number[] = [];
+for (let i = 0; i < 37; i++) base.push((i * 7) % 11);
+show("sums", prefixSums(base));
+const copy = base.slice();
+prefixInPlace(copy);
+show("in place", copy);
+
+// Slices of a hole-free array: empty, reversed, negative, and past the end.
+show("slice empty", base.slice(5, 2));
+show("slice negative", base.slice(-5, -2));
+show("slice past end", base.slice(30, 100));
+const grown: (number | undefined)[] = base.slice(3, 9);
+grown[grown.length + 1] = 1;
+show("slice grown", grown);
+
+// Stores that skip ahead, then fill the gap from both ends.
+const gaps = new Array<number | undefined>(20);
+for (let i = 0; i < 20; i += 3) gaps[i] = i;
+show("strided", gaps);
+for (let i = 19; i >= 0; i--) if (!(i in gaps)) gaps[i] = -i;
+show("closed", gaps);
+for (let i = 0; i < gaps.length; i++) gaps[i] = (gaps[i] ?? 0) * 2;
+show("rewritten", gaps);
+
+// A hole or present undefined at each offset around the eight-state steps,
+// copied by slice, concat, spread, and fill.
+for (const at of [0, 1, 6, 7, 8, 9, 15, 16, 17, 23, 24, 30]) {
+  const holey: (number | undefined)[] = [];
+  for (let i = 0; i < 32; i++) holey.push(i);
+  punch(holey, at);
+  const sliced = holey.slice(at > 4 ? at - 4 : 0);
+  sliced[0] = 100;
+  sliced[sliced.length] = 200;
+  show("slice hole " + at, sliced);
+  const undef = holey.slice();
+  undef[at] = undefined;
+  show("slice undefined " + at, undef.slice(1));
+  const joined = holey.slice(0, at).concat(holey.slice(at));
+  joined[at] = 7;
+  show("concat " + at, joined);
+  const spread: (number | undefined)[] = [-1];
+  spread.push(...undef);
+  show("spread " + at, spread);
+  const filled = new Array<number | undefined>(32);
+  filled.fill(3, 0, at);
+  filled[at] = 4;
+  show("fill " + at, filled);
+}
+
+// Whole-array copies far past one step, with the hole near the end.
+const long: (number | undefined)[] = [];
+for (let i = 0; i < 1000; i++) long.push(i & 15);
+const tail = long.slice();
+let total = 0;
+for (let i = 0; i < tail.length; i++) total += tail[i] ?? 0;
+punch(tail, 997);
+const again = tail.slice(990);
+show("long tail", again);
+console.log("long total", total, tail.slice(0, 997).length, 997 in tail.slice());
+
+// int32 parameters the body never writes: their reads keep the caller's
+// number, including -0 at call sites the range analysis cannot prove.
+function fib(n: number): number {
+  return n < 2 ? n : fib(n - 1) + fib(n - 2);
+}
+function half(n: number): number {
+  return n / 2 + (n | 0);
+}
+function same(n: number, sign: number): number {
+  return Object.is(n * sign, -0) ? 1 : n;
+}
+function countDown(n: number): number {
+  let steps = 0;
+  for (let i = n; i > 0; i--) steps += i;
+  return steps + n;
+}
+function rewrites(n: number): number {
+  n = n + 1;
+  return n * 2;
+}
+console.log(fib(0), fib(1), fib(20), half(7), half(-7), same(0, -1), same(5, 1));
+console.log(countDown(10), rewrites(3), Object.is(fib(-0), -0), Object.is(half(-0), -0));