diff --git a/packages/bugc/CHANGELOG.md b/packages/bugc/CHANGELOG.md index 1caacd17bb..e0eeeef664 100644 --- a/packages/bugc/CHANGELOG.md +++ b/packages/bugc/CHANGELOG.md @@ -21,7 +21,10 @@ support. Changes to the specification itself are tracked in the root elements or data it refers to. At optimization levels 1 to 3, a local whose current value the optimizer folded to a constant or removed is listed without a pointer, and an inlined function's locals are listed - in its inlined code ([#328]). + in its inlined code, with the storage variables, in one flat + `variables` list: from the instruction that invokes it up to the one + that returns from it, which lists the caller's variables ([#328], + [#341]). ### Changed @@ -50,6 +53,10 @@ support. Changes to the specification itself are tracked in the root ### Fixed +- At optimization levels 2 and 3, an inlined function that starts with an + inlined call, or returns one's result, now has its own `invoke` and + `return` contexts, around the inner call's. Before, its `invoke` (or its + `return`) was lost, so the call stack did not balance ([#341]). - A cast from dynamic `bytes` to a fixed-size type now reads the bytes. `msg.data[0:4] as bytes4` is the first four bytes of calldata; bytes past the slice's length are zero. Before, the cast gave the slice's @@ -209,3 +216,4 @@ First publication. [#337]: https://github.com/ethdebug/format/pull/337 [#339]: https://github.com/ethdebug/format/pull/339 [#340]: https://github.com/ethdebug/format/pull/340 +[#341]: https://github.com/ethdebug/format/pull/341 diff --git a/packages/bugc/src/evmgen/debug/local-variables.soundness.test.ts b/packages/bugc/src/evmgen/debug/local-variables.soundness.test.ts index 6e4a1e23fe..58db1a693a 100644 --- a/packages/bugc/src/evmgen/debug/local-variables.soundness.test.ts +++ b/packages/bugc/src/evmgen/debug/local-variables.soundness.test.ts @@ -198,6 +198,174 @@ code { r = add(s, 2); }`; } }, ); + + // `dbl` is inlined into `weight` (and `weight` into the loop at O3) + const inlinedSource = `name InlineScope; +define { + function dbl(x: uint256) -> uint256 { + return x + x; + }; + function weight(i: uint256, n: uint256) -> uint256 { + let w = dbl(i) + n; + return w; + }; +} +storage { [0] total: uint256; [1] calls: uint256; } +create {} +code { + let sum = 0; + for (let i = 0; i < 2; i = i + 1) { + sum = sum + weight(i, 4); + } + total = sum; + calls = calls + 1; +}`; + + /** The activation markers (`invoke`/`return`) of a context */ + const activations = ( + context: unknown, + key: "invoke" | "return", + ): string[] => { + if (!context || typeof context !== "object") return []; + const { gather } = context as { gather?: unknown[] }; + const own = (context as Record)[key]; + return [ + ...(own ? [own.identifier as string] : []), + ...(Array.isArray(gather) + ? gather.flatMap((c) => activations(c, key)) + : []), + ]; + }; + + it.each([2, 3] as const)( + "(h) lists an inlined function's locals only within its body at O%i", + async (level) => { + const run = await traceLocals(inlinedSource, level); + // Contexts are postconditions: the callee's locals are in scope + // from the step that invokes it up to, not including, the step + // that returns from it + let inside = false; + let seen = 0; + for (const step of run.steps) { + const context = run.instructionAt(step)?.context; + if (activations(context, "invoke").includes("dbl")) inside = true; + if (activations(context, "return").includes("dbl")) inside = false; + const names = localsOf(context).map((v) => v.identifier); + if (!inside) { + expect(names, `x listed outside dbl, pc ${step.pc}`).not.toContain( + "x", + ); + } else if (names.includes("x")) { + seen++; + } + } + expect(seen).toBeGreaterThan(0); + }, + ); + + it.each([2, 3] as const)( + "(j) lists the caller's locals where an inlined function returns at O%i", + async (level) => { + // The callee's locals have the caller's names + const source = `name Collide; +define { + function f(x: uint256) -> uint256 { + let y = x + 1; + let total = y * 2; + return total + y; + }; +} +storage { [0] total: uint256; [1] s: uint256; } +create { s = 4; } +code { let y = s + 3; let x = s + 10; total = f(x); total = total + y; }`; + const main = source.indexOf("code {"); + const run = await traceLocals(source, level); + let returns = 0; + for (const step of run.steps) { + const context = run.instructionAt(step)?.context; + if (!activations(context, "return").includes("f")) continue; + returns++; + const locals = localsOf(context).filter((v) => { + const pointer = JSON.stringify(v.pointer ?? {}); + return !pointer.includes('"storage"'); + }); + expect(locals.map((v) => v.identifier).sort()).toEqual(["x", "y"]); + for (const local of locals) { + const { range } = local.declaration as { range: { offset: number } }; + expect(range.offset, `${local.identifier}`).toBeGreaterThan(main); + } + } + expect(returns).toBe(1); + }, + ); + + it.each([2, 3] as const)( + "(k) brackets each inlined function that starts or ends with an " + + "inlined call at O%i", + async (level) => { + const bodies = { + "starts with": "let s = inner(r); return s + 1;", + "ends with": "return inner(r);", + }; + for (const [what, body] of Object.entries(bodies)) { + const source = `name Nested; +define { + function inner(p: uint256) -> uint256 { let q = p * 3; return q; }; + function outer(r: uint256) -> uint256 { ${body} }; +} +storage { [0] total: uint256; [1] calls: uint256; } +create { calls = 2; } +code { let v = calls + 5; total = outer(v); total = total + v; }`; + const run = await traceLocals(source, level); + const events: string[] = []; + for (const step of run.steps) { + const context = run.instructionAt(step)?.context; + const names = localsOf(context).map((v) => v.identifier); + const invokes = activations(context, "invoke"); + const returns = activations(context, "return"); + events.push( + ...invokes.map((f) => `invoke ${f}`), + ...returns.map((f) => `return ${f}`), + ); + // Back in main once outer returns + if (returns.includes("outer")) { + expect(names, `${what}, pc ${step.pc}`).toContain("v"); + } + } + expect(events, what).toEqual([ + "invoke outer", + "invoke inner", + "return inner", + "return outer", + ]); + } + }, + ); + + it.each([2, 3] as const)( + "(i) lists storage variables in inlined code at O%i", + async (level) => { + const { program } = await traceLocals(inlinedSource, level); + const rows = await listed(inlinedSource, level); + const inlined = rows.filter(({ names }) => names.includes("x")); + expect(inlined.length).toBeGreaterThan(0); + for (const { instruction, names } of inlined) { + const where = `${instruction.operation?.mnemonic} at ${instruction.offset}`; + expect(names, where).toContain("total"); + expect(names, where).toContain("calls"); + } + // ... and nowhere apart from the flat list + const nested = (context: unknown): boolean => { + const { gather } = (context ?? {}) as { gather?: unknown[] }; + return (gather ?? []).some((c) => localsOf(c).length > 0 || nested(c)); + }; + for (const instruction of program.instructions) { + expect(nested(instruction.context), `${instruction.offset}`).toBe( + false, + ); + } + }, + ); }); const words: Shape = { kind: "array", element: { kind: "scalar", size: 32 } }; diff --git a/packages/bugc/src/evmgen/debug/local-variables.test.ts b/packages/bugc/src/evmgen/debug/local-variables.test.ts index 8cad2b0ec9..9d7dba1605 100644 --- a/packages/bugc/src/evmgen/debug/local-variables.test.ts +++ b/packages/bugc/src/evmgen/debug/local-variables.test.ts @@ -11,6 +11,7 @@ import { describe, it, expect } from "vitest"; import { compile } from "#compiler"; import { executeProgram } from "#test/evm/behavioral"; import type * as Format from "@ethdebug/format"; +import { liftVariables } from "./local-variables.js"; async function compileProgram( source: string, @@ -185,3 +186,56 @@ code { r = r + 1; }`; }); }); }); + +describe("liftVariables", () => { + const total = { identifier: "total", pointer: { location: "storage" } }; + const x = { identifier: "x" }; + const code = (offset: number) => ({ code: { range: { offset } } }); + /** Every `variables` entry in a context, nested or not */ + const all = (context: unknown): unknown[] => { + const { variables, gather } = (context ?? {}) as { + variables?: unknown[]; + gather?: unknown[]; + }; + return [...(variables ?? []), ...(gather ?? []).flatMap(all)]; + }; + + it("lifts variables out of a gather, once each", () => { + const { variables, context } = liftVariables({ + variables: [x], + gather: [code(1), { ...code(2), variables: [total] }], + }); + expect(variables).toEqual([x, total]); + expect(context).toEqual({ gather: [code(1), code(2)] }); + expect(all(context)).toEqual([]); + }); + + it("drops a gathered context left empty, and composes one flat", () => { + const { variables, context } = liftVariables({ + gather: [code(1), { variables: [total] }], + }); + expect(variables).toEqual([total]); + expect(context).toEqual(code(1)); + }); + + it("keeps the variables of a gathered context with a frame", () => { + const framed = { frame: "ir", ...code(2), variables: [total] }; + const { variables, context } = liftVariables({ + gather: [{ frame: "source", ...code(1) }, framed], + }); + expect(variables).toEqual([]); + expect(context).toEqual({ + gather: [{ frame: "source", ...code(1) }, framed], + }); + }); + + it("keeps variables where one context is left that collides", () => { + const input = { + ...code(1), + gather: [{ variables: [total] }, { ...code(2), variables: [x] }], + }; + const { variables, context } = liftVariables(input); + expect([...variables, ...all(context)]).toEqual([total, x]); + expect(variables).toEqual([]); + }); +}); diff --git a/packages/bugc/src/evmgen/debug/local-variables.ts b/packages/bugc/src/evmgen/debug/local-variables.ts index 7712dec07d..a8376c2195 100644 --- a/packages/bugc/src/evmgen/debug/local-variables.ts +++ b/packages/bugc/src/evmgen/debug/local-variables.ts @@ -280,10 +280,14 @@ function buildEntry( } /** - * Merge variable entries into an instruction's debug context as a - * flat `variables` sibling, alongside any existing `variables` (the - * storage variables from irgen) and preserving all other keys. A local - * in scope hides a storage variable of the same name. + * Merge variable entries into an instruction's debug context as one + * flat `variables` list, with the existing `variables` (the storage + * variables from irgen), and preserving all other keys. Storage + * variables in a `gather` (as when inlined code gathers the call + * site's `code` with the callee's) move to the flat list too: all of a + * gather's contexts apply, so this does not change what it says, and + * one list keeps each name once. A local in scope hides a storage + * variable of the same name. */ function withVariables( debug: Ir.Instruction.Debug | undefined, @@ -291,26 +295,69 @@ function withVariables( stackLocals: StackLocal[], ): Ir.Instruction.Debug { const { stored: _, stackLocals: __, ...rest } = debug ?? {}; - if (entries.length === 0) return rest; + if (!rest.context) { + if (entries.length === 0) return rest; + rest.context = {} as Format.Program.Context; + } - const context = (rest.context ?? {}) as Record; - const locals = new Set(entries.map((e) => e.identifier)); - const storage = ( - Array.isArray(context.variables) - ? (context.variables as VariableEntry[]) - : [] - ).filter((v) => !locals.has(v.identifier)); + const { variables: existing, context } = liftVariables( + rest.context as Record, + ); + const names = new Set(entries.map((e) => e.identifier)); + const storage = existing.filter((v) => { + if (names.has(v.identifier)) return false; + names.add(v.identifier); + return true; + }); + const variables = [...storage, ...entries]; return { ...rest, - context: { - ...context, - variables: [...storage, ...entries], - } as Format.Program.Context, + context: (variables.length > 0 + ? { ...context, variables } + : context) as Format.Program.Context, ...(stackLocals.length > 0 ? { stackLocals } : {}), }; } +/** + * A context's `variables`, its own and those of its `gather`'s + * contexts, and the context without them. A gathered context with a + * `frame` keeps its `variables`, which hold only in that frame. A + * gathered context left empty is dropped; a gather left with one + * context composes flat (or, if a key of that context collides, keeps + * its contexts' `variables`). + */ +export function liftVariables(context: Record): { + variables: VariableEntry[]; + context: Record; +} { + const { variables, gather, ...rest } = context; + const own = Array.isArray(variables) ? (variables as VariableEntry[]) : []; + if (!Array.isArray(gather)) return { variables: own, context: rest }; + + const lifted = (gather as Record[]).map((member) => + "frame" in member + ? { variables: [], context: member } + : liftVariables(member), + ); + const kept = lifted + .map((l) => l.context) + .filter((c) => Object.keys(c).length > 0); + if (kept.length === 1 && Object.keys(kept[0]).some((key) => key in rest)) { + return { variables: own, context: { ...rest, gather } }; + } + return { + variables: [...own, ...lifted.flatMap((l) => l.variables)], + context: + kept.length === 1 + ? { ...rest, ...kept[0] } + : kept.length > 0 + ? { ...rest, gather: kept } + : rest, + }; +} + /** * Give each listed local whose temp is on the stack a stack pointer, * for an instruction just emitted with the stack as it is after it @@ -349,6 +396,35 @@ export function withStackLocals( }; } +/** + * The op that returns from an inlined function, listing the variables + * once returned: the caller's (`returned`, as `enrich` stamps it), for + * the stack as it is after the op. Contexts are postconditions. + */ +export function withReturned( + instruction: I, + returned: Ir.Instruction.Debug, + stack: ReadonlyArray<{ irValue?: string }>, +): I { + const { variables } = (returned.context ?? {}) as { variables?: unknown }; + const { variables: _, ...context } = (instruction.debug?.context ?? + {}) as Record; + const { stackLocals: __, returned: ___, ...debug } = instruction.debug ?? {}; + return withStackLocals( + { + ...instruction, + debug: { + ...debug, + context: (variables + ? { ...context, variables } + : context) as Format.Program.Context, + ...(returned.stackLocals ? { stackLocals: returned.stackLocals } : {}), + }, + }, + stack, + ); +} + /** * Drop a function's locals from a debug context, for the instructions * that tear its frame down. Once the caller's frame pointer is @@ -635,11 +711,22 @@ function enrichFunction( for (const [blockId, block] of func.blocks) { // The block's entry: just before its first instruction (or its - // terminator), with that instruction's scope and inline site. + // terminator), with that instruction's scope and inline site. Just + // before an inlined function's invoke, that is the caller's. const first = block.instructions[0] ?? block.terminator; const result = spilled.get(blockId); + const sites = first.operationDebug?.inlineSites ?? []; + const invokes = Ir.Utils.activationsOf( + first.operationDebug?.context, + "invoke", + ).length; const entry = stamp( - first.operationDebug, + invokes > 0 && sites.length > 0 + ? { + ...first.operationDebug, + inlineSites: sites.slice(0, -Math.min(invokes, sites.length)), + } + : first.operationDebug, blockId, 0, -1, @@ -662,10 +749,17 @@ function enrichFunction( // that locates the new value. if (!("dest" in inst) || !homed.has(inst.dest as string)) { inst.operationDebug = stamp(debug, blockId, i); - return; + } else { + const stored = stamp(debug, blockId, i); + inst.operationDebug = { ...stamp(debug, blockId, i, -1), stored }; + } + // An instruction that returns from an inlined function: its last + // op lists the caller's variables + if (debug?.returned) { + const offsets = lastOffsets; + inst.operationDebug.returned = stamp(debug.returned, blockId, i); + lastOffsets = offsets; } - const stored = stamp(debug, blockId, i); - inst.operationDebug = { ...stamp(debug, blockId, i, -1), stored }; }); // The terminator's position is after all instructions in the block. const term = block.terminator; diff --git a/packages/bugc/src/evmgen/generation/block.ts b/packages/bugc/src/evmgen/generation/block.ts index a6782f1bfe..6f489cd8c9 100644 --- a/packages/bugc/src/evmgen/generation/block.ts +++ b/packages/bugc/src/evmgen/generation/block.ts @@ -21,7 +21,7 @@ import { generateCallTerminator, } from "./control-flow/index.js"; import { annotateTop } from "./values/identify.js"; -import { withStackLocals } from "../debug/local-variables.js"; +import { withReturned, withStackLocals } from "../debug/local-variables.js"; /** * Generate code for a basic block @@ -201,16 +201,24 @@ export function generate( result = result.then(gen); continue; } + const returned = inst.operationDebug?.returned; result = result.peek((state, builder) => { const start = state.instructions.length; - return builder.then(gen).then((s) => ({ - ...s, - instructions: bracketActivation( - s.instructions, - start, - operationCtx, - ), - })); + return builder.then(gen).then((s) => { + const instructions = [ + ...bracketActivation(s.instructions, start, operationCtx), + ]; + // The op that returns from an inlined function lists the + // caller's variables + if (returned && instructions.length > start) { + instructions[instructions.length - 1] = withReturned( + instructions[instructions.length - 1], + returned, + s.stack, + ); + } + return { ...s, instructions }; + }); }); } @@ -248,14 +256,36 @@ export function generate( // Process terminator // Handle call terminators specially // (they cross function boundaries) - if (block.terminator.kind === "call") { - result = result.then( - generateCallTerminator(block.terminator, functions), - ); + const term = block.terminator; + const terminate = + term.kind === "call" + ? generateCallTerminator(term, functions) + : generateTerminator(term, isLastBlock, isUserFunction); + // A terminator carries an inlined function's invoke or return + // only when its block has no instruction to carry it (an + // inlined function that starts with a call, or returns a call's + // result): bracket it as for an instruction. A call's own JUMP + // carries the call's invoke, so it is left out. + const termCtx = term.operationDebug?.context; + if ( + (term.kind === "call" || (term.kind === "jump" && !term.tailCall)) && + (carriesActivation(termCtx, "invoke") || + carriesActivation(termCtx, "return")) + ) { + result = result.peek((state, builder) => { + const start = state.instructions.length; + return builder.then(terminate).then((s) => ({ + ...s, + instructions: bracketActivation( + s.instructions, + start, + termCtx, + s.instructions.length - (term.kind === "call" ? 1 : 0), + ), + })); + }); } else { - result = result.then( - generateTerminator(block.terminator, isLastBlock, isUserFunction), - ); + result = result.then(terminate); } return result; diff --git a/packages/bugc/src/evmgen/generation/bracket-activation.ts b/packages/bugc/src/evmgen/generation/bracket-activation.ts index 88a2886a78..958303b907 100644 --- a/packages/bugc/src/evmgen/generation/bracket-activation.ts +++ b/packages/bugc/src/evmgen/generation/bracket-activation.ts @@ -26,6 +26,7 @@ */ import type * as Format from "@ethdebug/format"; import type * as Evm from "#evm"; +import * as Ir from "#ir"; type Ctx = Format.Program.Context; type Activation = "invoke" | "return"; @@ -60,27 +61,6 @@ export function carriesActivation( return key in ctx; } -/** The first activation value found for the given key, reaching into - * pick/gather composites. */ -function findActivation(ctx: Ctx | undefined, key: Activation): unknown { - if (!ctx || typeof ctx !== "object") return undefined; - if (isPick(ctx)) { - for (const c of ctx.pick) { - const v = findActivation(c, key); - if (v !== undefined) return v; - } - return undefined; - } - if (isGather(ctx)) { - for (const c of ctx.gather) { - const v = findActivation(c, key); - if (v !== undefined) return v; - } - return undefined; - } - return (ctx as Record)[key]; -} - /** Remove invoke and return discriminators anywhere in ctx, reaching * into pick/gather composites. Returns undefined if nothing remains. */ export function stripActivation(ctx: Ctx | undefined): Ctx | undefined { @@ -119,57 +99,34 @@ export function stripActivation(ctx: Ctx | undefined): Ctx | undefined { return Object.keys(rest).length > 0 ? (rest as Ctx) : undefined; } -/** Attach an activation discriminator, composing it as a flat sibling - * key on a leaf context (per the flat-composition convention), or - * appending it to a pick/gather composite. */ -function attachActivation( - ctx: Ctx | undefined, - key: Activation, - value: unknown, -): Ctx { - const marker = { [key]: value } as Ctx; - if (!ctx || typeof ctx !== "object") return marker; - if (isPick(ctx)) return { pick: [...ctx.pick, marker] } as Ctx; - if (isGather(ctx)) return { ...ctx, gather: [...ctx.gather, marker] } as Ctx; - return { ...(ctx as Record), [key]: value } as Ctx; -} - /** - * Rewrite the ops emitted by one IR instruction (the tail slice - * `instructions[start..]`) so invoke rides only the first op and - * return only the last op, using the discriminators found on the - * instruction's `operationDebug` context. No-op unless that context - * carries invoke and/or return, so it never touches ordinary code. + * Rewrite the ops emitted by one IR instruction (the slice + * `instructions[start..end]`, by default to the end) so invokes ride + * only the first op and returns only the last op, using the + * discriminators found on the instruction's `operationDebug` context. + * Several of one kind (an inlined function that starts or ends with + * an inlined call) keep their order: outermost invoke first, innermost + * return first. No-op unless that context carries invoke and/or + * return, so it never touches ordinary code. */ export function bracketActivation( instructions: Evm.Instruction[], start: number, operationCtx: Ctx | undefined, + end: number = instructions.length, ): Evm.Instruction[] { - const end = instructions.length; // exclusive if (end <= start) return instructions; - const hasInvoke = carriesActivation(operationCtx, "invoke"); - const hasReturn = carriesActivation(operationCtx, "return"); - if (!hasInvoke && !hasReturn) return instructions; - - const invokeValue = hasInvoke - ? findActivation(operationCtx, "invoke") - : undefined; - const returnValue = hasReturn - ? findActivation(operationCtx, "return") - : undefined; + const invokes = Ir.Utils.activationsOf(operationCtx, "invoke"); + const returns = Ir.Utils.activationsOf(operationCtx, "return"); + if (invokes.length === 0 && returns.length === 0) return instructions; const out = instructions.slice(); for (let i = start; i < end; i++) { const op = out[i]; - let ctx = stripActivation(op.debug?.context); - if (hasInvoke && i === start) { - ctx = attachActivation(ctx, "invoke", invokeValue); - } - if (hasReturn && i === end - 1) { - ctx = attachActivation(ctx, "return", returnValue); - } + let ctx = Ir.Utils.withoutActivations(op.debug?.context); + if (i === start) ctx = Ir.Utils.withActivations(ctx, { invokes }); + if (i === end - 1) ctx = Ir.Utils.withActivations(ctx, { returns }); out[i] = { ...op, debug: { ...op.debug, context: ctx } }; } return out; diff --git a/packages/bugc/src/ir/spec/instruction.ts b/packages/bugc/src/ir/spec/instruction.ts index 1d072c020c..d12ecaa5af 100644 --- a/packages/bugc/src/ir/spec/instruction.ts +++ b/packages/bugc/src/ir/spec/instruction.ts @@ -46,6 +46,13 @@ export namespace Instruction { * local whose value the result is can be listed with a pointer. */ stored?: Debug; + /** + * For an instruction that returns from an inlined function: the + * debug once it has returned. The inliner gives it the caller's + * source range and placement; code generation gives it the + * caller's variables, for the op that carries the `return`. + */ + returned?: Debug; /** * Inline sites this instruction was inlined through, outermost * first. Only the innermost site's locals are listed here. diff --git a/packages/bugc/src/ir/utils/debug.ts b/packages/bugc/src/ir/utils/debug.ts index 7a0e0ed0e0..042fb9c880 100644 --- a/packages/bugc/src/ir/utils/debug.ts +++ b/packages/bugc/src/ir/utils/debug.ts @@ -16,7 +16,8 @@ export function combineDebugContexts( return withPlacement(combineContexts(debugs), debugs[0]); } -/** Keep an instruction's inline sites and origin on a new debug. */ +/** Keep an instruction's inline sites, origin and (if it returns from + * an inlined function) its debug once returned on a new debug. */ function withPlacement( debug: Ir.Instruction.Debug, from: Ir.Instruction.Debug | undefined, @@ -25,6 +26,7 @@ function withPlacement( ...debug, ...(from?.inlineSites ? { inlineSites: from.inlineSites } : {}), ...(from?.origin ? { origin: from.origin } : {}), + ...(from?.returned ? { returned: from.returned } : {}), }; } @@ -422,3 +424,104 @@ export function addTransform( debug, ); } + +/** An activation discriminator: a call's `invoke` or its `return` */ +export type Activation = "invoke" | "return"; + +/** + * The values of a context's `invoke` (or `return`) discriminators, its + * own and its `gather`'s, in document order. + */ +export function activationsOf( + context: Format.Program.Context | undefined, + key: Activation, +): unknown[] { + if (!context || typeof context !== "object") return []; + const { gather } = context as { gather?: unknown }; + const own = (context as Record)[key]; + return [ + ...(own === undefined ? [] : [own]), + ...(Array.isArray(gather) + ? (gather as Format.Program.Context[]).flatMap((c) => + activationsOf(c, key), + ) + : []), + ]; +} + +/** + * A context without its `invoke` and `return` discriminators. + * Undefined if nothing else remains. A `gather` left with one context + * composes flat, if its keys do not collide. + */ +export function withoutActivations( + context: Format.Program.Context | undefined, +): Format.Program.Context | undefined { + if (!context || typeof context !== "object") return context; + const { + invoke: _, + return: __, + gather, + ...rest + } = context as Record; + if (!Array.isArray(gather)) { + return Object.keys(rest).length > 0 + ? (rest as Format.Program.Context) + : undefined; + } + const kept = (gather as Format.Program.Context[]) + .map(withoutActivations) + .filter((c): c is Format.Program.Context => c !== undefined); + if (kept.length === 0) { + return Object.keys(rest).length > 0 + ? (rest as Format.Program.Context) + : undefined; + } + if ( + kept.length === 1 && + Object.keys(kept[0]).every((key) => !(key in rest)) + ) { + return { ...rest, ...kept[0] } as Format.Program.Context; + } + return { ...rest, gather: kept } as Format.Program.Context; +} + +/** + * A context with activation discriminators added: `invokes` before + * the ones it has, `returns` after them, so that nested activations + * on one instruction open outermost first and close innermost first. + */ +export function withActivations( + context: Format.Program.Context | undefined, + { invokes = [], returns = [] }: { invokes?: unknown[]; returns?: unknown[] }, +): Format.Program.Context | undefined { + if (invokes.length === 0 && returns.length === 0) return context; + const markers = [ + ...[...invokes, ...activationsOf(context, "invoke")].map((invoke) => ({ + invoke, + })), + ...[...activationsOf(context, "return"), ...returns].map((value) => ({ + return: value, + })), + ] as Format.Program.Context[]; + const rest = withoutActivations(context) as + | Record + | undefined; + if (!rest) { + return ( + markers.length === 1 ? markers[0] : { gather: markers } + ) as Format.Program.Context; + } + if (Array.isArray(rest.gather)) { + return { + ...rest, + gather: [...rest.gather, ...markers], + } as Format.Program.Context; + } + // Without its activations, `rest` has no `invoke` or `return` key + return ( + markers.length === 1 + ? { ...rest, ...markers[0] } + : { ...rest, gather: markers } + ) as Format.Program.Context; +} diff --git a/packages/bugc/src/optimizer/steps/inlining.ts b/packages/bugc/src/optimizer/steps/inlining.ts index 42fb77801f..daca9f2b62 100644 --- a/packages/bugc/src/optimizer/steps/inlining.ts +++ b/packages/bugc/src/optimizer/steps/inlining.ts @@ -184,9 +184,46 @@ export class InliningStep extends BaseOptimizationStep { // maps to no instruction. Preserve it by gathering it onto the // entry instruction alongside the callee-body range it collides // with (two source ranges that both apply → gather). - const callSiteCode = ( - call.operationDebug?.context as { code?: unknown } | undefined - )?.code; + // + // An inlined function whose entry block is empty (it starts with + // a call) leaves its invoke pending on that call: inlining the + // call puts it on the callee's first instruction, before the + // callee's own. Likewise, an inlined function whose return block + // is empty (it returns a call's result) leaves its return pending + // on the jump from that block, which is this call's continuation: + // inlining the call puts it after the callee's own return. + const pendingInvokes = Ir.Utils.activationsOf( + call.operationDebug?.context, + "invoke", + ); + const callDebug = withoutActivationsDebug(call.operationDebug); + const callSiteCode = (callDebug.context as { code?: unknown } | undefined) + ?.code; + + const continuation = caller.blocks.get(call.continuation); + const pendingReturns = + continuation && + continuation.instructions.length === 0 && + continuation.terminator.kind === "jump" && + !continuation.terminator.tailCall + ? Ir.Utils.activationsOf( + continuation.terminator.operationDebug?.context, + "return", + ) + : []; + // The placement once the callee (and any pending returns) has + // returned: the caller's, at the call or past those returns + let after = callDebug; + if (pendingReturns.length > 0) { + after = withoutActivationsDebug(continuation!.terminator.operationDebug); + continuation!.terminator.operationDebug = after; + } + const invokes = [...pendingInvokes, inlineInvoke]; + const returns = [inlineReturn, ...pendingReturns]; + const returnedJump: Ir.Instruction.Debug = { + ...Ir.Utils.addTransform(after, "inline"), + ...placement(after), + }; const entryBlockId = blockRename.get(callee.entry)!; const returnBlockIds: string[] = []; @@ -194,10 +231,9 @@ export class InliningStep extends BaseOptimizationStep { // --- clone + remap callee blocks --- for (const [origId, origBlock] of callee.blocks) { const newId = blockRename.get(origId)!; - const isEntry = origId === callee.entry; const instructions: Ir.Instruction[] = origBlock.instructions.map( - (inst, idx) => { + (inst) => { const cloned = remapInstruction(inst, remapValue, idRename); const sites = inlineSites(inst.operationDebug); // Mark every inlined instruction for membership. @@ -205,23 +241,14 @@ export class InliningStep extends BaseOptimizationStep { cloned.operationDebug, "inline", ); - // Virtual invoke on the first instruction of the entry, - // plus the call site's source range (gathered with the - // callee-body range it collides with). - if (isEntry && idx === 0) { - cloned.operationDebug = mergeDiscriminator( - cloned.operationDebug, - "invoke", - inlineInvoke, - ); - if (callSiteCode !== undefined) { - cloned.operationDebug = gatherCallSite( - cloned.operationDebug, - callSiteCode, - ); - } - } cloned.operationDebug.inlineSites = sites; + const returned = cloned.operationDebug.returned; + if (returned) { + cloned.operationDebug.returned = { + ...returned, + inlineSites: inlineSites(returned), + }; + } return cloned; }, ); @@ -234,39 +261,25 @@ export class InliningStep extends BaseOptimizationStep { const t = origBlock.terminator; if (t.kind === "return") { returnBlockIds.push(newId); - // Virtual return marker on the last body instruction of - // this block (or a synthetic carrier if the block is empty - // is not needed — return blocks always have ≥1 emitted - // instruction in practice; if empty, the marker rides the - // jump's debug below). - if (instructions.length > 0) { - const last = instructions[instructions.length - 1]; - last.operationDebug = mergeDiscriminator( - last.operationDebug, - "return", - inlineReturn, - ); - } - // return -> jump to the caller's continuation + // return -> jump to the caller's continuation. Once the + // callee has returned, this jump is the caller's code: it + // keeps the caller's source range, inline sites and origin, + // so it lists the caller's variables, not the callee's locals. terminator = { kind: "jump", target: call.continuation, - operationDebug: Ir.Utils.addTransform( - mergeDiscriminator({}, "return", inlineReturn), - "inline", - ), + operationDebug: { ...returnedJump }, }; } else { terminator = remapTerminator(t, remapValue, blockRename); + terminator.operationDebug = { + ...terminator.operationDebug, + inlineSites: inlineSites(t.operationDebug), + ...(t.operationDebug?.origin + ? { origin: t.operationDebug.origin } + : {}), + }; } - terminator.operationDebug = { - ...terminator.operationDebug, - inlineSites: inlineSites(t.operationDebug), - ...(t.operationDebug?.origin - ? { origin: t.operationDebug.origin } - : {}), - }; - caller.blocks.set(newId, { id: newId, phis, @@ -277,6 +290,79 @@ export class InliningStep extends BaseOptimizationStep { }); } + // --- the virtual invoke and return --- + // The invoke rides the body's first instruction, plus the call + // site's source range (gathered with the callee-body range it + // collides with); the return rides its last instruction, which + // lists the caller's variables once returned (`returned`). Empty + // blocks that only jump on (as when the body starts or ends with + // an inlined call) are passed over; the jumps past the return are + // the caller's code. With no instruction to carry it, a marker + // rides a terminator. + const inlinedIds = new Set(blockRename.values()); + const jumpsInto = new Map(); + for (const id of inlinedIds) { + for (const target of successors(caller.blocks.get(id)!.terminator)) { + if (inlinedIds.has(target)) { + jumpsInto.set(target, [...(jumpsInto.get(target) ?? []), id]); + } + } + } + const isPlainJump = (t: Ir.Block.Terminator) => + t.kind === "jump" && !t.tailCall; + + let entryId = entryBlockId; + for (;;) { + const block = caller.blocks.get(entryId)!; + const first = block.instructions[0]; + const t = block.terminator; + if (first) { + first.operationDebug = withActivationsDebug( + callSiteCode === undefined + ? first.operationDebug + : gatherCallSite(first.operationDebug, callSiteCode), + { invokes }, + ); + break; + } + if ( + t.kind === "jump" && + isPlainJump(t) && + jumpsInto.get(t.target)?.length === 1 + ) { + entryId = t.target; + continue; + } + t.operationDebug = withActivationsDebug(t.operationDebug, { invokes }); + break; + } + + for (let returnId of returnBlockIds) { + for (;;) { + const block = caller.blocks.get(returnId)!; + const last = block.instructions.at(-1); + if (last) { + last.operationDebug = { + ...withActivationsDebug(last.operationDebug, { returns }), + returned: after, + }; + break; + } + const from = jumpsInto.get(returnId) ?? []; + const pred = from.length === 1 ? caller.blocks.get(from[0]) : undefined; + if (pred && isPlainJump(pred.terminator)) { + pred.terminator.operationDebug = { ...returnedJump }; + returnId = from[0]; + continue; + } + block.terminator.operationDebug = withActivationsDebug( + block.terminator.operationDebug, + { returns }, + ); + break; + } + } + // --- carry the callee's locals into the caller --- // Each keeps the temp that now holds its value (a parameter's is // the call's argument; none if that is a constant) and belongs to @@ -328,7 +414,7 @@ export class InliningStep extends BaseOptimizationStep { callBlock.terminator = { kind: "jump", target: entryBlockId, - operationDebug: call.operationDebug, + operationDebug: callDebug, }; void callBlockId; @@ -590,19 +676,21 @@ function substituteTemp(fn: Ir.Function, id: string, value: Ir.Value): void { } } +/** The blocks a terminator goes on to */ +function successors(t: Ir.Block.Terminator): string[] { + return t.kind === "jump" + ? [t.target] + : t.kind === "branch" + ? [t.trueTarget, t.falseTarget] + : t.kind === "call" + ? [t.continuation] + : []; +} + function recomputePredecessors(fn: Ir.Function): void { for (const b of fn.blocks.values()) b.predecessors = new Set(); for (const [id, b] of fn.blocks) { - const t = b.terminator; - const targets: string[] = - t.kind === "jump" - ? [t.target] - : t.kind === "branch" - ? [t.trueTarget, t.falseTarget] - : t.kind === "call" - ? [t.continuation] - : []; - for (const tgt of targets) { + for (const tgt of successors(b.terminator)) { fn.blocks.get(tgt)?.predecessors.add(id); } } @@ -610,33 +698,30 @@ function recomputePredecessors(fn: Ir.Function): void { // ---- debug-context composition ---- -/** - * Attach a discriminator (invoke/return) as a flat sibling key on - * a debug context, threading into a gather leaf if present so the - * marker never sits as a sibling of `gather`. - */ -function mergeDiscriminator( +/** A debug with activation discriminators added (see + * `Ir.Utils.withActivations`) */ +function withActivationsDebug( debug: Ir.Instruction.Debug, - key: "invoke" | "return", - value: unknown, + activations: { invokes?: unknown[]; returns?: unknown[] }, ): Ir.Instruction.Debug { - const existing = debug.context as Record | undefined; - if (existing && "gather" in existing && Array.isArray(existing.gather)) { - // Add as a new gather child rather than a sibling of gather. - return { - ...debug, - context: { - ...existing, - gather: [...(existing.gather as unknown[]), { [key]: value }], - } as Format.Program.Context, - }; - } + const context = Ir.Utils.withActivations(debug.context, activations); + return { ...debug, ...(context ? { context } : {}) }; +} + +/** A debug without activation discriminators */ +function withoutActivationsDebug( + debug: Ir.Instruction.Debug | undefined, +): Ir.Instruction.Debug { + const { context: _, ...rest } = debug ?? {}; + const context = Ir.Utils.withoutActivations(debug?.context); + return { ...rest, ...(context ? { context } : {}) }; +} + +/** A debug's inline sites and origin, which say where it is */ +function placement(debug: Ir.Instruction.Debug): Ir.Instruction.Debug { return { - ...debug, - context: { - ...(existing ?? {}), - [key]: value, - } as Format.Program.Context, + ...(debug.inlineSites ? { inlineSites: debug.inlineSites } : {}), + ...(debug.origin ? { origin: debug.origin } : {}), }; } @@ -660,6 +745,7 @@ function gatherCallSite( return { ...debug, context: { + ...existing, gather: [callSite, ...(existing.gather as unknown[])], } as Format.Program.Context, };