From 8bef13d2459291948a32b69a8b3940eb15d0ebbb Mon Sep 17 00:00:00 2001 From: "g. nicholas d'andrea" Date: Thu, 2 Apr 2026 08:26:04 -0400 Subject: [PATCH] bugc: fix optimizer for nested call arguments in recursion Move phi resolution from target blocks to predecessor blocks (standard phi deconstruction). The previous approach resolved phis at the target using the layout-order predecessor, which broke for back-edges where the runtime predecessor differs. Also fixes jump optimization eliminating phi-bearing blocks and rewrites TCO to use a pre_entry trampoline with phis on the original entry block. --- packages/bugc/src/evmgen/behavioral.test.ts | 57 ++++++++++ packages/bugc/src/evmgen/generation/block.ts | 29 ++++- .../src/optimizer/steps/jump-optimization.ts | 1 + .../steps/tail-call-optimization.test.ts | 26 +++-- .../optimizer/steps/tail-call-optimization.ts | 101 ++++++++++-------- 5 files changed, 154 insertions(+), 60 deletions(-) diff --git a/packages/bugc/src/evmgen/behavioral.test.ts b/packages/bugc/src/evmgen/behavioral.test.ts index baf774dbc..d2f32b3e8 100644 --- a/packages/bugc/src/evmgen/behavioral.test.ts +++ b/packages/bugc/src/evmgen/behavioral.test.ts @@ -387,6 +387,63 @@ code { result = factorial(5); }`; expect(result.callSuccess).toBe(true); expect(await result.getStorage(0n)).toBe(120n); }); + + it("should support recursion at optimization level 2", async () => { + const source = `name RecursionOpt; + +define { + function succ(n: uint256) -> uint256 { + return n + 1; + }; + function count( + n: uint256, target: uint256 + ) -> uint256 { + if (n < target) { + return count(succ(n), target); + } else { + return n; + } + }; +} + +storage { [0] result: uint256; } +create { result = 0; } +code { result = count(0, 5); }`; + + const result = await executeProgram(source, { + calldata: "", + optimizationLevel: 2, + }); + + expect(result.callSuccess).toBe(true); + expect(await result.getStorage(0n)).toBe(5n); + }); + + it("should support factorial at optimization level 3", async () => { + const source = `name FactorialOpt; + +define { + function factorial(n: uint256) -> uint256 { + if (n < 2) { + return 1; + } else { + return n * factorial(n - 1); + } + }; +} + +storage { [0] result: uint256; } +create { result = 0; } +code { result = factorial(5); }`; + + const result = await executeProgram(source, { + calldata: "", + optimizationLevel: 3, + }); + + expect(result.callSuccess).toBe(true); + expect(await result.getStorage(0n)).toBe(120n); + }); }); describe("loops", () => { diff --git a/packages/bugc/src/evmgen/generation/block.ts b/packages/bugc/src/evmgen/generation/block.ts index 7c9f71247..e23460dfb 100644 --- a/packages/bugc/src/evmgen/generation/block.ts +++ b/packages/bugc/src/evmgen/generation/block.ts @@ -154,18 +154,37 @@ export function generate( } } - // Process phi nodes if we have a predecessor - if (predecessor && block.phis.length > 0) { - result = result.then(generatePhis(block.phis, predecessor)); - } + // Phi resolution happens at predecessors, not at the + // target. Each predecessor stores its phi source values + // into the phi destination memory slots before jumping. + // This is necessary for back-edges (loops, TCO) where + // the runtime predecessor differs from the layout-order + // predecessor. // Process regular instructions for (const inst of block.instructions) { result = result.then(Instruction.generate(inst)); } + // Emit phi copies for successor blocks before the + // terminator. For jump terminators, check if the + // target has phis and store the source values for + // this block. + if (func && block.terminator.kind === "jump") { + const target = func.blocks.get(block.terminator.target); + if (target && target.phis.length > 0) { + const relevant = target.phis.filter((phi) => + phi.sources.has(block.id), + ); + if (relevant.length > 0) { + result = result.then(generatePhis(relevant, block.id)); + } + } + } + // Process terminator - // Handle call terminators specially (they cross function boundaries) + // Handle call terminators specially + // (they cross function boundaries) if (block.terminator.kind === "call") { result = result.then( generateCallTerminator(block.terminator, functions), diff --git a/packages/bugc/src/optimizer/steps/jump-optimization.ts b/packages/bugc/src/optimizer/steps/jump-optimization.ts index ccb4e0586..a8d657d65 100644 --- a/packages/bugc/src/optimizer/steps/jump-optimization.ts +++ b/packages/bugc/src/optimizer/steps/jump-optimization.ts @@ -18,6 +18,7 @@ export class JumpOptimizationStep extends BaseOptimizationStep { for (const [blockId, block] of func.blocks) { if ( block.instructions.length === 0 && + block.phis.length === 0 && block.terminator.kind === "jump" ) { jumpTargets.set(blockId, block.terminator.target); diff --git a/packages/bugc/src/optimizer/steps/tail-call-optimization.test.ts b/packages/bugc/src/optimizer/steps/tail-call-optimization.test.ts index ce888a246..18ac89102 100644 --- a/packages/bugc/src/optimizer/steps/tail-call-optimization.test.ts +++ b/packages/bugc/src/optimizer/steps/tail-call-optimization.test.ts @@ -78,25 +78,35 @@ describe("TailCallOptimizationStep", () => { // Count call terminators after optimization let callsAfterCount = 0; - let hasLoopHeader = false; + let hasPreEntry = false; for (const [blockId, block] of optimizedFunc.blocks) { if (block.terminator.kind === "call") { callsAfterCount++; } - // Look for the loop header block - if (blockId.includes("_loop")) { - hasLoopHeader = true; - // Loop header should have phi nodes for parameters - expect(block.phis.length).toBe(factorialFunc.parameters.length); + // Look for the pre_entry trampoline block + if (blockId.includes("_pre")) { + hasPreEntry = true; } } // Tail-recursive calls should be eliminated expect(callsAfterCount).toBe(0); - // Should have created a loop header - expect(hasLoopHeader).toBe(true); + // Should have created a pre_entry trampoline + expect(hasPreEntry).toBe(true); + + // Original entry block should have phi nodes + // for parameters + const origEntry = optimizedFunc.blocks.get(optimizedFunc.entry); + const origEntryTarget = + origEntry?.terminator.kind === "jump" + ? origEntry.terminator.target + : undefined; + const entryBlock = origEntryTarget + ? optimizedFunc.blocks.get(origEntryTarget) + : undefined; + expect(entryBlock?.phis.length).toBe(factorialFunc.parameters.length); // Should have recorded transformations const transformations = context.getTransformations(); diff --git a/packages/bugc/src/optimizer/steps/tail-call-optimization.ts b/packages/bugc/src/optimizer/steps/tail-call-optimization.ts index 88961ddef..a5273a19a 100644 --- a/packages/bugc/src/optimizer/steps/tail-call-optimization.ts +++ b/packages/bugc/src/optimizer/steps/tail-call-optimization.ts @@ -80,55 +80,59 @@ export class TailCallOptimizationStep extends BaseOptimizationStep { tailCallBlocks.push(blockId); } - // If we found tail calls, create a loop structure + // If we found tail calls, transform into a loop. + // + // Strategy: create a trampoline "pre_entry" block as + // the new function entry. Add phi nodes to the original + // entry block that select between the initial param + // values (from pre_entry) and the tail-call arguments + // (from each tail-call site). Tail-call blocks jump + // directly to the original entry. if (tailCallBlocks.length > 0) { - // Create a new loop header block that will contain phis for parameters - const loopHeaderId = `${func.entry}_loop`; - const originalEntry = func.blocks.get(func.entry); - - if (!originalEntry) { - return; // Should not happen - } + const origEntryId = func.entry; + const origEntry = func.blocks.get(origEntryId); + if (!origEntry) return; + + // Create trampoline that becomes the new func.entry + const preEntryId = `${origEntryId}_pre`; + const preEntry: Ir.Block = { + id: preEntryId, + phis: [], + instructions: [], + terminator: { + kind: "jump", + target: origEntryId, + operationDebug: {}, + }, + predecessors: new Set(), + debug: {}, + }; + func.blocks.set(preEntryId, preEntry); + func.entry = preEntryId; - // Create phi nodes for each parameter + // Build phi nodes on the original entry block. + // Sources: preEntry → original param, each tail + // call block → the call's corresponding argument. const paramPhis: Ir.Block.Phi[] = []; for (let i = 0; i < func.parameters.length; i++) { const param = func.parameters[i]; - const phiSources = new Map(); - - // Initial value from function entry - phiSources.set(func.entry, { + const sources = new Map(); + sources.set(preEntryId, { kind: "temp", id: param.tempId, type: param.type, }); - paramPhis.push({ kind: "phi", - sources: phiSources, - dest: `${param.tempId}_loop`, + sources, + dest: param.tempId, type: param.type, - operationDebug: { context: param.loc ? undefined : undefined }, + operationDebug: {}, }); } - // Create the loop header block - const loopHeader: Ir.Block = { - id: loopHeaderId, - phis: paramPhis, - instructions: [], - terminator: { - kind: "jump", - target: func.entry, - operationDebug: {}, - }, - predecessors: new Set([func.entry, ...tailCallBlocks]), - debug: {}, - }; - - func.blocks.set(loopHeaderId, loopHeader); - - // Transform each tail call + // Transform each tail call: replace call with jump + // to origEntry, add phi sources for arguments. for (const blockId of tailCallBlocks) { const block = func.blocks.get(blockId)!; const callTerm = block.terminator as Ir.Block.Terminator & { @@ -136,21 +140,18 @@ export class TailCallOptimizationStep extends BaseOptimizationStep { }; const contBlock = func.blocks.get(callTerm.continuation)!; - // Update phi sources with arguments from this tail call for (let i = 0; i < func.parameters.length; i++) { if (i < callTerm.arguments.length) { paramPhis[i].sources.set(blockId, callTerm.arguments[i]); } } - // Replace call with jump to loop header block.terminator = { kind: "jump", - target: loopHeaderId, + target: origEntryId, operationDebug: callTerm.operationDebug, }; - // Track the transformation context.trackTransformation({ type: "replace", pass: this.name, @@ -159,30 +160,36 @@ export class TailCallOptimizationStep extends BaseOptimizationStep { ...Ir.Utils.extractContexts(contBlock), ], result: Ir.Utils.extractContexts(block), - reason: `Optimized tail-recursive call to ${funcName} into loop`, + reason: + `Optimized tail-recursive call to ` + `${funcName} into loop`, }); - // Mark continuation block for removal if it has no other - // predecessors - const otherPredecessors = Array.from(contBlock.predecessors).filter( - (pred) => pred !== blockId, + // Remove continuation if no other predecessors + const otherPreds = Array.from(contBlock.predecessors).filter( + (p) => p !== blockId, ); - if (otherPredecessors.length === 0) { + if (otherPreds.length === 0) { blocksToRemove.add(callTerm.continuation); - context.trackTransformation({ type: "delete", pass: this.name, original: Ir.Utils.extractContexts(contBlock), result: [], - reason: `Removed unused continuation block ${callTerm.continuation}`, + reason: + `Removed unused continuation block ` + callTerm.continuation, }); } else { - // Update predecessors contBlock.predecessors.delete(blockId); } } + + // Install phis and update predecessors + origEntry.phis = [...paramPhis, ...origEntry.phis]; + origEntry.predecessors.add(preEntryId); + for (const blockId of tailCallBlocks) { + origEntry.predecessors.add(blockId); + } } // Remove marked blocks