From b8ef53cef1df29feeade5b607c836e1ba98e85d8 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 14 Oct 2026 22:02:48 -0700 Subject: [PATCH] Read construction-only fields of loop receivers once per loop Outside constructors a final field cannot change: the constructors that store it are finished or suspended below every other running frame. A loop reading p.f through a never-rebound parameter (usually this) still reloaded it after every store or call that might alias the object and hid bounds and strides such as y < this.h or i - this.w from loop-invariant reasoning. Such reads now go through an immutable local declared before the outermost loop containing them. analyzeFinalFields gains a scalar mode, which also treats in-place numeric updates (fieldIncDec) as writes. --- packages/compiler/src/backend/llvm/emitter.ts | 9 +- packages/compiler/src/ir/field-values.test.ts | 171 ++++++++++++++++++ packages/compiler/src/ir/field-values.ts | 129 +++++++++++++ packages/compiler/src/ir/final-fields.ts | 14 +- .../5352-final-field-loop-reads.ts.json | 6 + tests/corpus/5352-final-field-loop-reads.ts | 125 +++++++++++++ 6 files changed, 449 insertions(+), 5 deletions(-) create mode 100644 packages/compiler/src/ir/field-values.test.ts create mode 100644 packages/compiler/src/ir/field-values.ts create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/5352-final-field-loop-reads.ts.json create mode 100644 tests/corpus/5352-final-field-loop-reads.ts diff --git a/packages/compiler/src/backend/llvm/emitter.ts b/packages/compiler/src/backend/llvm/emitter.ts index e0aefe986..b5ffab3b9 100644 --- a/packages/compiler/src/backend/llvm/emitter.ts +++ b/packages/compiler/src/backend/llvm/emitter.ts @@ -94,6 +94,7 @@ import { analyzeFinalFields, constructorClass, type FinalFields } from "../../ir import { foldConstantGlobals } from "../../ir/constant-globals.js"; import { dropUnwrittenPropertyBags } from "../../ir/property-bags.js"; import { freezeInitOnlyGlobals } from "../../ir/init-only-globals.js"; +import { hoistFinalFieldReads } from "../../ir/field-values.js"; import { findIntegerViews } from "./integer-views.js"; import { findInt32Abis, @@ -467,7 +468,9 @@ export function emitLlvmModule(input: IrModule, options: LlvmTargetOptions = {}) scalarizeNumericRecords( specializeNumericCalls( foldConstantGlobals( - copyStableCaptures(freezeInitOnlyGlobals(dropUnwrittenPropertyBags(mod))), + copyStableCaptures( + hoistFinalFieldReads(freezeInitOnlyGlobals(dropUnwrittenPropertyBags(mod))), + ), ), mangleFunction, ), @@ -490,7 +493,9 @@ export function emitLlvmModuleSource( scalarizeNumericRecords( specializeNumericCalls( foldConstantGlobals( - copyStableCaptures(freezeInitOnlyGlobals(dropUnwrittenPropertyBags(mod))), + copyStableCaptures( + hoistFinalFieldReads(freezeInitOnlyGlobals(dropUnwrittenPropertyBags(mod))), + ), ), mangleFunction, ), diff --git a/packages/compiler/src/ir/field-values.test.ts b/packages/compiler/src/ir/field-values.test.ts new file mode 100644 index 000000000..1b66d1a30 --- /dev/null +++ b/packages/compiler/src/ir/field-values.test.ts @@ -0,0 +1,171 @@ +import { expect, test } from "vitest"; +import { hoistFinalFieldReads } from "./field-values.js"; +import { analyzeFinalFields } from "./final-fields.js"; +import { + BOOL, + F64, + VOID, + type IrClassDef, + type IrExpr, + type IrFunction, + type IrModule, + type IrStmt, + type IrType, +} from "./ir.js"; + +const loc = { file: "field-values.ts", start: 0, end: 0 }; +const GRID: IrType = { kind: "object", className: "Grid" }; +const num = (value: number): IrExpr => ({ loc, kind: "numLit", value, type: F64 }); +const ref = (localId: string, type: IrType = F64): IrExpr => ({ + loc, + kind: "varRef", + localId, + type, +}); +const field = (obj: string, name: string): IrExpr => ({ + loc, + kind: "fieldGet", + obj: ref(obj, GRID), + className: "Grid", + field: name, + type: F64, +}); +const lt = (left: IrExpr, right: IrExpr): IrExpr => ({ + loc, + kind: "bin", + op: "<", + left, + right, + type: BOOL, +}); +const loopReading = (...reads: IrExpr[]): IrStmt => ({ + loc, + kind: "while", + cond: lt(ref("i"), reads[0]!), + body: reads.slice(1).map((expr) => ({ loc, kind: "exprStmt", expr })), +}); +const set = (obj: string, name: string, value: IrExpr): IrStmt => ({ + loc, + kind: "fieldSet", + obj: ref(obj, GRID), + className: "Grid", + field: name, + value, +}); +const method = (name: string, body: IrStmt[], extra: Partial = {}): IrFunction => ({ + loc, + name, + params: [{ localId: "this", name: "this", type: GRID }], + locals: [ + { id: "this", name: "this", type: GRID, mutable: false }, + { id: "i", name: "i", type: F64, mutable: true }, + ], + returnType: VOID, + body, + ...extra, +}); +const grid: IrClassDef = { + name: "Grid", + fields: [ + { name: "w", type: F64 }, + { name: "count", type: F64 }, + { name: "hits", type: F64 }, + ], + methods: [], +} as unknown as IrClassDef; +const moduleOf = (functions: IrFunction[]): IrModule => ({ + irVersion: 16, + sourceFile: "field-values.ts", + entry: "%init", + classes: [grid], + functions: [ + method("%Grid.constructor", [ + set("this", "w", num(8)), + set("this", "count", num(0)), + set("this", "hits", num(0)), + ]), + ...functions, + ], +}); +const hoistedLocals = (mod: IrModule, name: string): string[] => { + const before = mod.functions.find((f) => f.name === name)!; + const after = hoistFinalFieldReads(mod).functions.find((f) => f.name === name)!; + return after.locals.slice(before.locals.length).map((l) => l.id); +}; + +test("scalar final fields exclude fields written or updated outside their constructor", () => { + const mod = moduleOf([ + method("%Grid.bump", [ + set("this", "count", num(1)), + { + loc, + kind: "exprStmt", + expr: { + loc, + kind: "fieldIncDec", + op: "+", + prefix: true, + obj: ref("this", GRID), + className: "Grid", + field: "hits", + fieldDyn: false, + type: F64, + }, + }, + ]), + ]); + const finals = analyzeFinalFields(mod, true); + expect(["w", "count", "hits"].filter((f) => finals.isFinal("Grid", f))).toEqual(["w"]); + // Reference-only analysis ignores scalar fields entirely. + expect(analyzeFinalFields(mod).isFinal("Grid", "w")).toBe(false); +}); + +test("loops read a final field of a receiver once, before the loop", () => { + const mod = moduleOf([ + method("%Grid.scan", [ + loopReading(field("this", "w"), field("this", "w"), field("this", "count")), + ]), + method("%Grid.bump", [set("this", "count", num(1))]), + ]); + const out = hoistFinalFieldReads(mod).functions.find((f) => f.name === "%Grid.scan")!; + expect(out.locals.slice(-1)).toEqual([ + { id: "this.w%final", name: "w", type: F64, mutable: false }, + ]); + const block = out.body[0]!; + expect(block.kind).toBe("block"); + if (block.kind !== "block") return; + expect(block.body[0]).toMatchObject({ + kind: "varDecl", + localId: "this.w%final", + init: { kind: "fieldGet", field: "w" }, + }); + const loop = block.body[1]!; + if (loop.kind !== "while") throw new Error("expected the loop"); + expect(loop.cond).toMatchObject({ right: { kind: "varRef", localId: "this.w%final" } }); + // The written field keeps its per-use read. + expect(loop.body[1]).toMatchObject({ + kind: "exprStmt", + expr: { kind: "fieldGet", field: "count" }, + }); +}); + +test("constructors, suspending functions and rebound receivers keep their reads", () => { + const scan = [loopReading(field("this", "w"))]; + const rebinding: IrStmt[] = [ + { loc, kind: "assign", localId: "this", value: ref("this", GRID) }, + ...scan, + ]; + const mod = moduleOf([ + method("%Grid.gen", scan, { + generator: { yieldT: F64, nextT: F64, resultType: { kind: "record", shapeId: "%gen" } }, + }), + method("%Grid.later", scan, { async: true }), + method("%Grid.rebind", rebinding), + method("%Grid.plain", [{ loc, kind: "exprStmt", expr: field("this", "w") }]), + ]); + for (const name of ["%Grid.gen", "%Grid.later", "%Grid.rebind", "%Grid.plain"]) + expect(hoistedLocals(mod, name)).toEqual([]); + const ctor = moduleOf([]); + ctor.functions[0] = method("%Grid.constructor", [set("this", "w", num(8)), ...scan]); + expect(hoistedLocals(ctor, "%Grid.constructor")).toEqual([]); +}); diff --git a/packages/compiler/src/ir/field-values.ts b/packages/compiler/src/ir/field-values.ts new file mode 100644 index 000000000..22d382c35 --- /dev/null +++ b/packages/compiler/src/ir/field-values.ts @@ -0,0 +1,129 @@ +import { analyzeFinalFields, constructorClass, type FinalFields } from "./final-fields.js"; +import type { IrExpr, IrFunction, IrLocal, IrModule, IrStmt } from "./ir.js"; +import { everyStmtList, mapExprChildren, mapStmtChildren } from "./traverse.js"; + +type FieldRead = IrExpr & { kind: "fieldGet"; obj: IrExpr & { kind: "varRef" } }; +type Loop = IrStmt & { kind: "for" | "while" | "doWhile" | "forOf" }; + +/** Final fields of parameters read once per loop instead of once per use. + * + * Outside constructors a final field (final-fields.ts: only its family's + * constructors store it, through their own `this`) cannot change: the + * constructors that do are either finished or suspended below every other + * running frame. A loop that reads `p.f` through a parameter `p` that is + * never rebound therefore reads the same value on every iteration, but + * LLVM must reload it after each store or call that might alias the + * object, and the value stays hidden from loop-invariant reasoning such as + * `y < this.h` bounds and `i - this.w` strides. Such reads are taken once, + * into an immutable local declared just before the outermost loop that + * contains them; a reference value borrows from the still-alive receiver + * exactly as the field read itself would. Async functions and generators + * are excluded: a suspension lets a still-running constructor continue. */ +export function hoistFinalFieldReads(mod: IrModule): IrModule { + if (mod.lib || !mod.classes?.length) return mod; + let finals: FinalFields | undefined; + let changed = false; + const functions = mod.functions.map((fn) => { + if (fn.async || fn.generator || constructorClass(fn.name) !== null) return fn; + const receivers = new Set(); + for (const param of fn.params) { + const local = fn.locals.find((l) => l.id === param.localId); + if (local && !local.boxed && local.type.kind === "object") receivers.add(local.id); + } + if (receivers.size === 0 || !hasLoop(fn.body)) return fn; + // A parameter the body rebinds no longer names one object. + everyStmtList(fn.body, { + stmt: (s) => { + if ("localId" in s && typeof s.localId === "string") receivers.delete(s.localId); + if (s.kind === "tryCatch" && s.catchLocalId) receivers.delete(s.catchLocalId); + return true; + }, + expr: (e) => { + if (e.kind !== "varRef" && "localId" in e && typeof e.localId === "string") + receivers.delete(e.localId); + return true; + }, + }); + if (receivers.size === 0) return fn; + finals ??= analyzeFinalFields(mod, true); + if (finals.size === 0) return fn; + const hoisted = hoistInFunction(fn, receivers, finals); + if (hoisted !== fn) changed = true; + return hoisted; + }); + return changed ? { ...mod, functions } : mod; +} + +function hoistInFunction( + fn: IrFunction, + receivers: ReadonlySet, + finals: FinalFields, +): IrFunction { + const added: IrLocal[] = []; + const taken = new Set(fn.locals.map((l) => l.id)); + const isTarget = (e: IrExpr): e is FieldRead => + e.kind === "fieldGet" && + e.obj.kind === "varRef" && + receivers.has(e.obj.localId) && + finals.isFinal(e.className, e.field); + const keyOf = (e: FieldRead): string => `${e.obj.localId}\0${e.className}\0${e.field}`; + + const hoistLoop = (loop: Loop): IrStmt | null => { + const reads = new Map(); + everyStmtList([loop], { + stmt: () => true, + expr: (e) => { + if (isTarget(e) && !reads.has(keyOf(e))) reads.set(keyOf(e), e); + return true; + }, + }); + if (reads.size === 0) return null; + const slots = new Map(); + const decls: IrStmt[] = []; + for (const [key, read] of reads) { + let id = `${read.obj.localId}.${read.field}%final`; + for (let n = 1; taken.has(id); n++) id = `${read.obj.localId}.${read.field}%final${n}`; + taken.add(id); + slots.set(key, id); + added.push({ id, name: read.field, type: read.type, mutable: false }); + decls.push({ + kind: "varDecl", + localId: id, + init: { ...read, obj: { ...read.obj } }, + loc: read.loc, + }); + } + const rewriteExpr = (e: IrExpr): IrExpr => { + if (isTarget(e)) { + const id = slots.get(keyOf(e)); + if (id !== undefined) return { kind: "varRef", localId: id, type: e.type, loc: e.loc }; + } + return mapExprChildren(e, rewriteExpr, rewriteStmt); + }; + const rewriteStmt = (s: IrStmt): IrStmt => mapStmtChildren(s, rewriteExpr, rewriteStmt); + return { kind: "block", body: [...decls, rewriteStmt(loop)], loc: loop.loc }; + }; + + let changed = false; + const visitStmt = (s: IrStmt): IrStmt => { + if (s.kind === "for" || s.kind === "while" || s.kind === "doWhile" || s.kind === "forOf") { + const hoisted = hoistLoop(s); + if (hoisted) { + changed = true; + return hoisted; + } + } + return mapStmtChildren(s, visitExpr, visitStmt); + }; + const visitExpr = (e: IrExpr): IrExpr => mapExprChildren(e, visitExpr, visitStmt); + const body = fn.body.map(visitStmt); + return changed ? { ...fn, body, locals: [...fn.locals, ...added] } : fn; +} + +function hasLoop(body: IrStmt[]): boolean { + return !everyStmtList(body, { + stmt: (s) => + !(s.kind === "for" || s.kind === "while" || s.kind === "doWhile" || s.kind === "forOf"), + expr: () => true, + }); +} diff --git a/packages/compiler/src/ir/final-fields.ts b/packages/compiler/src/ir/final-fields.ts index e37c13bd6..21572c910 100644 --- a/packages/compiler/src/ir/final-fields.ts +++ b/packages/compiler/src/ir/final-fields.ts @@ -75,7 +75,10 @@ export function constructorClass(fnName: string): string | null { : null; } -export function analyzeFinalFields(mod: IrModule): FinalFields { +/** `scalars` extends the analysis from reference-counted fields to fields of + * every type, for consumers that read a final field's value once instead of + * borrowing it (field-values.ts). */ +export function analyzeFinalFields(mod: IrModule, scalars = false): FinalFields { const classes = new Map((mod.classes ?? []).map((c) => [c.name, c])); const finals = new FinalFields(classes); if (mod.lib) return finals; @@ -104,7 +107,7 @@ export function analyzeFinalFields(mod: IrModule): FinalFields { for (const cls of classes.values()) for (const field of cls.fields) { - if (!isRefCounted(field.type) || field.name.startsWith("%")) continue; + if ((!scalars && !isRefCounted(field.type)) || field.name.startsWith("%")) continue; const family = finals.family(cls.name, field.name)!; if (finals.reasons.has(family)) continue; if (!plainClass(cls)) finals.drop(family, "runtime-provided or generic class"); @@ -131,7 +134,12 @@ export function analyzeFinalFields(mod: IrModule): FinalFields { } return everyStmtChild(s, expr, stmt); }; - const expr = (e: IrExpr): boolean => everyExprChild(e, expr, stmt); + // `o.n++` updates a numeric field in place; reference fields never see it. + const expr = (e: IrExpr): boolean => { + if (e.kind === "fieldIncDec" && finals.isFinal(e.className, e.field)) + finals.drop(finals.family(e.className, e.field)!, `updated in ${fn.name}`); + return everyExprChild(e, expr, stmt); + }; for (const s of fn.body) stmt(s); } return finals; diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/5352-final-field-loop-reads.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/5352-final-field-loop-reads.ts.json new file mode 100644 index 000000000..cfd01ca54 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/5352-final-field-loop-reads.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/5352-final-field-loop-reads.ts" + ], + "diags": [] +} diff --git a/tests/corpus/5352-final-field-loop-reads.ts b/tests/corpus/5352-final-field-loop-reads.ts new file mode 100644 index 000000000..944205b62 --- /dev/null +++ b/tests/corpus/5352-final-field-loop-reads.ts @@ -0,0 +1,125 @@ +// Loops reading construction-only fields see one value per call; fields +// anything else writes keep being re-read. + +class Grid { + readonly w: number; + readonly h: number; + readonly data: Float64Array; + readonly out: Float64Array; + constructor(w: number, h: number) { + this.w = w; + this.h = h; + this.data = new Float64Array(w * h); + this.out = new Float64Array(w * h); + for (let i = 0; i < w * h; i++) this.data[i] = (i % 7) * 0.5; + } + blur(): number { + let sum = 0; + for (let y = 1; y < this.h - 1; y++) { + for (let x = 1; x < this.w - 1; x++) { + const i = y * this.w + x; + this.out[i] = (this.data[i - 1]! + this.data[i + 1]! + this.data[i - this.w]! + this.data[i + this.w]!) / 4; + sum += this.out[i]!; + } + } + return sum; + } +} +const grid = new Grid(9, 7); +console.log("blur", grid.blur(), grid.out[10], grid.out[grid.w * 2 + 3]); + +// A base constructor runs an overridden method before the subclass has +// defined its fields: that call sees them missing, later calls see them. +class Counter { + readonly seen: number; + constructor() { + this.seen = this.count(); + } + count(): number { + return 0; + } +} +class Limited extends Counter { + readonly limit: number = 4; + readonly step: number = 2; + override count(): number { + let n = 0; + for (let i = 0; i < this.limit; i += this.step) n++; + return n; + } +} +const limited = new Limited(); +console.log("limited", limited.seen, limited.count(), limited.limit); + +// Fields a method updates are re-read on every iteration. +class Accumulator { + total = 0; + hits = 0; + readonly cap: number; + constructor(cap: number) { + this.cap = cap; + } + fill(): number { + while (this.total < this.cap) { + this.total += 3; + this.hits++; + } + return this.hits; + } +} +const acc = new Accumulator(20); +console.log("acc", acc.fill(), acc.total, acc.hits); + +// A field written through another instance is not construction-only. +class Node2 { + limit: number; + peer: Node2 | null = null; + constructor(limit: number) { + this.limit = limit; + } + walk(): number { + let steps = 0; + for (let i = 0; i < this.limit; i++) { + steps++; + if (this.peer !== null && i === 1) this.peer.limit = 2; + } + return steps; + } +} +const a = new Node2(6); +a.peer = a; +console.log("peer", a.walk(), a.limit); + +// The reference is final; the array it holds still grows during the loop. +class Bag { + readonly items: number[] = []; + readonly cap: number; + constructor(cap: number) { + this.cap = cap; + } + grow(): string { + while (this.items.length < this.cap) this.items.push(this.items.length * this.cap); + return this.items.join(","); + } +} +console.log("bag", new Bag(5).grow()); + +// Final fields read through a parameter, and from a generator. +class Range { + readonly lo: number; + readonly hi: number; + constructor(lo: number, hi: number) { + this.lo = lo; + this.hi = hi; + } + *values(): Generator { + for (let i = this.lo; i < this.hi; i++) yield i; + } +} +function span(r: Range): number { + let s = 0; + for (let i = r.lo; i < r.hi; i++) s += i; + return s; +} +const r = new Range(3, 9); +console.log("range", span(r), [...r.values()].join(" "));