bugc: keep locals located after a call

packages/bugc/CHANGELOG.md

@@ -52,6 +52,10 @@ support. Changes to the specification itself are tracked in the root
 - `push` of bytes in calldata onto an `array<bytes>` in storage now
   stores the bytes. Before, it stored the calldata reference's word
   (offset and length) ([#379]).
+- A local defined before a call keeps its pointer after the call.
+  The dominator tree that local-variable tracking uses left out a
+  call's edge to its continuation, so every local defined before a call
+  was listed with no pointer from the call on ([#378]).
 - Reading a `string` or `bytes` from storage as a whole, as in
   `let m: string = motd;`, now copies it to memory, decoded from its
   storage encoding. This also corrects an index (`data[3]`), a slice
@@ -525,4 +529,5 @@ First publication.
 [#370]: https://github.com/ethdebug/format/pull/370
 [#371]: https://github.com/ethdebug/format/pull/371
 [#377]: https://github.com/ethdebug/format/pull/377
+[#378]: https://github.com/ethdebug/format/pull/378
 [#379]: https://github.com/ethdebug/format/pull/379

packages/bugc/src/evmgen/debug/local-variables.soundness.test.ts

@@ -39,6 +39,24 @@ function codeOffset(context: unknown): number | undefined {
     ?.code?.range?.offset;
 }
 
+/** A struct local, a call, then the local again */
+const acrossCall = `name AcrossCall;
+define {
+  struct P { a: uint64; b: uint32; };
+  function f() -> uint256 { return r + 3; };
+}
+storage { [0] r: uint256; [1] ps: mapping<address, P>; }
+create {}
+code {
+  let p: P = ps[msg.sender];
+  p.a = p.a + 5;
+  let h = f();
+  if (h == 0) { return; }
+  p.b = p.b + 1;
+  ps[msg.sender] = p;
+  r = h;
+}`;
+
 describe("which locals are listed", () => {
   it("(a) does not list a local before its declaration", async () => {
     const source = `name PreDef;
@@ -157,6 +175,22 @@ code {
     expect(rows.some(({ names }) => names.includes("i"))).toBe(true);
   });
 
+  it("(g) keeps a local located after a call", async () => {
+    // A call's continuation is dominated by the calling block: wherever
+    // the call's result `h` is located, so is `p`, defined before it
+    for (const level of levels) {
+      const { program } = await traceLocals(acrossCall, level);
+      const located = program.instructions
+        .map((instruction) => localsOf(instruction.context))
+        .filter((vs) => vs.some((v) => v.identifier === "h" && v.pointer));
+      expect(located.length, `O${level}`).toBeGreaterThan(0);
+      for (const vs of located) {
+        const p = vs.find((v) => v.identifier === "p");
+        expect(p?.pointer, `O${level}`).toBeDefined();
+      }
+    }
+  });
+
   it("(f) gives a stack-resident local a stack pointer", async () => {
     const { program } = await traceLocals(programs[0].source);
     const pointers = program.instructions.flatMap((i) =>
@@ -852,6 +886,28 @@ code {
       k: { values: [7n] },
     },
   },
+  {
+    name: "a struct local across a call",
+    source: acrossCall,
+    locals: {
+      p: {
+        shape: {
+          kind: "struct",
+          fields: {
+            a: { kind: "scalar", size: 8 },
+            b: { kind: "scalar", size: 4 },
+          },
+        },
+        everyLevel: true,
+        values: [
+          { a: 0n, b: 0n },
+          { value: { a: 5n, b: 0n }, after: "p.a = p.a + 5" },
+          { value: { a: 5n, b: 1n }, after: "p.b = p.b + 1" },
+        ],
+      },
+      h: { values: [3n] },
+    },
+  },
   {
     name: "a void call",
     source: `name VoidCall;

packages/bugc/src/ir/analysis/stats.test.ts

@@ -173,6 +173,52 @@ describe("Statistics", () => {
     });
   });
 
+  describe("calls", () => {
+    const source = `
+      name Test;
+      define {
+        function f(n: uint256) -> uint256 { return n + 1; };
+      }
+      storage { [0] x: uint256; }
+      code {
+        for (let i = 0; i < 3; i = i + 1) {
+          let y = f(i);
+          x = x + y;
+        }
+        return;
+      }
+    `;
+
+    it("follows a call to its continuation", async () => {
+      const ir = await compileToIr(source);
+      const call = [...ir.main.blocks.entries()].find(
+        ([, block]) => block.terminator.kind === "call",
+      )!;
+      const terminator = call[1].terminator;
+      if (terminator.kind !== "call") throw new Error("no call");
+      const { continuation } = terminator;
+
+      const dom = new Statistics.Analyzer().analyze(ir).dominatorTree;
+      expect(dom[continuation]).toBe(call[0]);
+
+      // without call edges, no block after a call has a dominator
+      const without = new Statistics.Analyzer({ callEdges: false }).analyze(
+        ir,
+      ).dominatorTree;
+      expect(without[continuation]).toBeUndefined();
+    });
+
+    it("finds a loop's blocks through a call", async () => {
+      const ir = await compileToIr(source);
+      const { loopInfo } = new Statistics.Analyzer().analyze(ir);
+      expect(loopInfo).toHaveLength(1);
+      const body = [...ir.main.blocks.keys()].find((id) =>
+        id.startsWith("for_body"),
+      )!;
+      expect(loopInfo[0].blocks).toContain(body);
+    });
+  });
+
   describe("detectLoops", () => {
     it("should detect nested loops", async () => {
       const source = `

packages/bugc/src/ir/analysis/stats.ts

@@ -32,7 +32,23 @@ export namespace Statistics {
     depth: number;
   }
 
+  export interface Options {
+    /**
+     * Whether a call's continuation is a successor of the calling
+     * block (default true). Without call edges, no block after a call
+     * has a dominator: common-subexpression elimination relies on this
+     * to reuse no value across a call.
+     */
+    callEdges?: boolean;
+  }
+
   export class Analyzer {
+    private readonly callEdges: boolean;
+
+    constructor({ callEdges = true }: Options = {}) {
+      this.callEdges = callEdges;
+    }
+
     analyze(module: Ir.Module): Statistics {
       const func = module.main;
       const blocks = Array.from(func.blocks.values());
@@ -160,6 +176,9 @@ export namespace Statistics {
           case "branch":
             edges += 2;
             break;
+          case "call":
+            if (this.callEdges) edges += 1;
+            break;
           // return has no edges
         }
       }
@@ -174,13 +193,7 @@ export namespace Statistics {
       const dominators: Record<string, string | null> = {};
       const blockIds = Array.from(func.blocks.keys());
 
-      // Build predecessor map for efficiency
-      const predecessors: Record<string, string[]> = {};
-      for (const blockId of blockIds) {
-        predecessors[blockId] = Array.from(
-          func.blocks.get(blockId)?.predecessors || [],
-        );
-      }
+      const predecessors = this.predecessorsOf(func);
 
       // Entry block dominates itself (has no dominator)
       dominators[func.entry] = null;
@@ -395,6 +408,7 @@ export namespace Statistics {
       func: Ir.Function,
     ): string[] {
       // Find all blocks in the loop (simplified)
+      const predecessors = this.predecessorsOf(func);
       const loopBlocks = new Set<string>([header, tail]);
       const worklist = [tail];
 
@@ -403,7 +417,7 @@ export namespace Statistics {
         const block = func.blocks.get(blockId);
         if (!block) continue;
 
-        for (const pred of block.predecessors) {
+        for (const pred of predecessors[blockId] ?? []) {
           if (!loopBlocks.has(pred)) {
             loopBlocks.add(pred);
             if (pred !== header) {
@@ -416,12 +430,37 @@ export namespace Statistics {
       return Array.from(loopBlocks);
     }
 
+    /**
+     * Each block's predecessors. With call edges, from the terminators:
+     * `Block.predecessors` omits a call's edge to its continuation.
+     * Without, `Block.predecessors` as it is.
+     */
+    private predecessorsOf(func: Ir.Function): Record<string, string[]> {
+      const predecessors: Record<string, string[]> = {};
+      for (const [blockId, block] of func.blocks) {
+        predecessors[blockId] = this.callEdges
+          ? []
+          : Array.from(block.predecessors || []);
+      }
+      if (!this.callEdges) return predecessors;
+      for (const [blockId, block] of func.blocks) {
+        for (const succ of this.getSuccessors(block)) {
+          // (a successor that is not one of the function's blocks has
+          // no entry, and is skipped)
+          predecessors[succ]?.push(blockId);
+        }
+      }
+      return predecessors;
+    }
+
     private getSuccessors(block: Ir.Block): string[] {
       switch (block.terminator.kind) {
         case "jump":
           return [block.terminator.target];
         case "branch":
           return [block.terminator.trueTarget, block.terminator.falseTarget];
+        case "call":
+          return this.callEdges ? [block.terminator.continuation] : [];
         case "return":
           return [];
         default:

packages/bugc/src/irgen/generate/function.ts

@@ -135,7 +135,12 @@ function computePredecessors(
         }
         break;
       }
-      // "return" and "unreachable" have no successors
+      // "return" and "unreachable" have no successors. A "call"'s edge
+      // to its continuation is left out: the passes that read
+      // `predecessors` (block merging, jump optimization, return
+      // merging, tail calls) see the CFG without it, and adding it here
+      // can change the code they make. Dominators and loops
+      // (Statistics.Analyzer) derive their own, with call edges.
     }
   }
 

packages/bugc/src/optimizer/steps/common-subexpression-elimination.test.ts

@@ -0,0 +1,41 @@
+import { describe, it, expect } from "vitest";
+
+import { compile } from "#compiler";
+import type * as Ir from "#ir";
+
+describe("CommonSubexpressionEliminationStep", () => {
+  // `msg.value` before and after a call that is not inlined (it
+  // recurses): computing it again is cheaper than keeping it live
+  // across the call, so the value is not reused
+  const source = `name Rec;
+define {
+  function g(n: uint256) -> uint256 {
+    let a = msg.value + n;
+    if (n == 0) { return a; }
+    let b = g(n - 1);
+    return a + b + msg.value;
+  };
+}
+storage { [0] r: uint256; }
+create {}
+code { r = g(3); }`;
+
+  for (const level of [2, 3] as const) {
+    it(`reuses no value across a call (level ${level})`, async () => {
+      const result = await compile({
+        to: "ir",
+        source,
+        optimizer: { level },
+      });
+      if (!result.success) throw new Error("compile failed");
+      const g = result.value.ir.functions.get("g")!;
+      const values = [...g.blocks.values()]
+        .flatMap((block) => block.instructions)
+        .filter(
+          (inst): inst is Ir.Instruction.Env =>
+            inst.kind === "env" && inst.op === "msg_value",
+        );
+      expect(values).toHaveLength(2);
+    });
+  }
+});

packages/bugc/src/optimizer/steps/common-subexpression-elimination.ts

@@ -13,8 +13,12 @@ export class CommonSubexpressionEliminationStep extends BaseOptimizationStep {
 
     // Process each function separately
     this.processAllFunctions(optimized, (func) => {
-      // Compute dominator tree for this function
-      const analyzer = new Ir.Analysis.Statistics.Analyzer();
+      // Compute dominator tree for this function, without call edges:
+      // no value is reused across a call (keeping one live across a
+      // call can cost more than computing it again)
+      const analyzer = new Ir.Analysis.Statistics.Analyzer({
+        callEdges: false,
+      });
       const analysis = analyzer.analyze({ ...module, main: func });
       const dominators = analysis.dominatorTree;