[ssa] Potential improvements to ssa construction

Redo of #711 

Note that one test (switch.js) is failing, i'm working on a fix.

compiler/forget/src/HIR/SSAify.ts

@@ -22,11 +22,10 @@ type State = {
   incompletePhis: IncompletePhi[];
 };
 
-const unsealedPreds: Map<BasicBlock, number> = new Map();
-
 class SSABuilder {
   #states: Map<BasicBlock, State> = new Map();
   #current: BasicBlock | null = null;
+  unsealedPreds: Map<BasicBlock, number> = new Map();
   #env: Environment;
 
   constructor(env: Environment) {
@@ -73,13 +72,13 @@ class SSABuilder {
 
     if (block.preds.size == 0) {
       // We're at the entry block and haven't found our defintion yet.
-      console.log(
-        `Unable to find "${printIdentifier(oldId)}", assuming it's a global`
-      );
+      // console.log(
+      //   `Unable to find "${printIdentifier(oldId)}", assuming it's a global`
+      // );
       return oldId;
     }
 
-    if (unsealedPreds.get(block)! > 0) {
+    if (this.unsealedPreds.get(block)! > 0) {
       // We haven't visited all our predecessors, let's place an incomplete phi
       // for now.
       const newId = { ...oldId, id: this.nextIdentifierId };
@@ -96,17 +95,45 @@ class SSABuilder {
       return newId;
     }
 
-    // There are multiple predecessors, we need a phi.
-    const newId = { ...oldId, id: this.nextIdentifierId };
+    // There are multiple predecessors, we may need a phi.
+    return this.maybeAddPhi(block, oldId, state);
+  }
+
+  maybeAddPhi(block: BasicBlock, oldId: Identifier, state: State): Identifier {
     // Adding a phi may loop back to our block if there is a loop in the CFG.  We
     // update our defs before adding the phi to terminate the recursion rather than
     // looping infinitely.
+    const newId = { ...oldId, id: this.nextIdentifierId };
     state.defs.set(oldId, newId);
-    this.addPhi(block, oldId, newId);
 
-    // TODO(gsn): Can we just return `newPlace` rather than looking it up?
-    // `addPhi` _can_ mutate it, but _will_ it?
-    return state.defs.get(oldId)!;
+    const predDefs: Map<BasicBlock, Identifier> = new Map();
+    const predIds: Set<Identifier> = new Set();
+    for (const predBlock of block.preds) {
+      const predId = this.getIdAt(oldId, predBlock);
+      predDefs.set(predBlock, predId);
+      predIds.add(predId);
+    }
+
+    // if all predecessors have the same id, then there is no need for a phi node.
+    // note that in the case of a loop there are guaranteed to be multiple values,
+    // since we have already updated this block with a new identifier to terminate
+    // the recursion
+    if (predIds.size === 1) {
+      // there was only a single incoming id so we don't need a phi node,
+      // replace with that incoming id instead
+      const predId = [...predIds][0]!;
+      state.defs.set(oldId, predId);
+      return predId;
+    }
+
+    const phi: Phi = {
+      kind: "Phi",
+      id: newId,
+      operands: predDefs,
+    };
+
+    block.phis.add(phi);
+    return newId;
   }
 
   addPhi(block: BasicBlock, oldId: Identifier, newId: Identifier) {
@@ -197,14 +224,14 @@ export default function buildSSA(func: HIRFunction, env: Environment) {
     const outputBlocks = outputs.map((id) => func.body.blocks.get(id)!);
     for (const output of outputBlocks) {
       let count;
-      if (unsealedPreds.has(output)) {
-        count = unsealedPreds.get(output)! - 1;
+      if (builder.unsealedPreds.has(output)) {
+        count = builder.unsealedPreds.get(output)! - 1;
       } else {
         count = output.preds.size - 1;
       }
-      unsealedPreds.set(output, count);
+      builder.unsealedPreds.set(output, count);
 
-      if (count == 0 && visitedBlocks.has(output)) {
+      if (count === 0 && visitedBlocks.has(output)) {
         builder.fixIncompletePhis(output);
       }
     }

compiler/forget/src/__tests__/fixtures/hir/component.expect.md

@@ -44,11 +44,11 @@ bb0:
   Goto bb1
 bb1:
   predecessor blocks: bb0 bb4 bb7
-  items$33: phi(bb0: items$27, bb4: items$50, bb7: items$50)
-  item$35: phi(bb0: item$10, bb4: item$42, bb7: item$42)
-  seen$38: phi(bb0: seen$30, bb4: seen$41, bb7: seen$41)
-  renderedItems$46: phi(bb0: renderedItems$29, bb4: renderedItems$45, bb7: renderedItems$45)
-  max$48: phi(bb0: max$32, bb4: max$47, bb7: max$47)
+  items$33: phi(bb0: items$27, bb4: items$33, bb7: items$33)
+  item$35: phi(bb0: item$10, bb4: item$35, bb7: item$35)
+  seen$38: phi(bb0: seen$30, bb4: seen$38, bb7: seen$38)
+  renderedItems$46: phi(bb0: renderedItems$29, bb4: renderedItems$46, bb7: renderedItems$46)
+  max$48: phi(bb0: max$32, bb4: max$48, bb7: max$48)
   If (read items$33) then:bb3 else:bb2
 bb3:
   predecessor blocks: bb1
@@ -66,24 +66,18 @@ bb9:
 bb7:
   predecessor blocks: bb9 bb8
   $40: phi(bb9: $39, bb8: $37)
-  seen$41: phi(bb9: seen$38, bb8: seen$38)
-  item$42: phi(bb9: item$35, bb8: item$35)
-  renderedItems$45: phi(bb9: renderedItems$46, bb8: renderedItems$46)
-  max$47: phi(bb9: max$48, bb8: max$48)
-  items$50: phi(bb9: items$33, bb8: items$33)
   If (read $40) then:bb1 else:bb4
 bb4:
   predecessor blocks: bb7
-  Call mutate seen$41.add(mutate item$42)
+  Call mutate seen$38.add(mutate item$35)
   Const mutate $43 = "div"
-  Const mutate $44 = JSX <read $43>{read item$42}</read $43>
-  Call mutate renderedItems$45.push(read $44)
-  Const mutate $49 = Binary read renderedItems$45.length >= read max$47
+  Const mutate $44 = JSX <read $43>{read item$35}</read $43>
+  Call mutate renderedItems$46.push(read $44)
+  Const mutate $49 = Binary read renderedItems$46.length >= read max$48
   If (read $49) then:bb2 else:bb1
 bb2:
   predecessor blocks: bb1 bb4
-  renderedItems$51: phi(bb1: renderedItems$46, bb4: renderedItems$45)
-  Const mutate count$52 = read renderedItems$51.length
+  Const mutate count$52 = read renderedItems$46.length
   Const mutate $53 = "div"
   Const mutate $54 = "\n      "
   Const mutate $55 = "h1"
@@ -91,7 +85,7 @@ bb2:
   Const mutate $57 = JSX <read $55>{freeze count$52}{read $56}</read $55>
   Const mutate $58 = "\n      "
   Const mutate $59 = "\n    "
-  Const mutate $60 = JSX <read $53>{read $54}{read $57}{read $58}{freeze renderedItems$51}{read $59}</read $53>
+  Const mutate $60 = JSX <read $53>{read $54}{read $57}{read $58}{freeze renderedItems$46}{read $59}</read $53>
   Return read $60
 ```
 

compiler/forget/src/__tests__/fixtures/hir/conditional-break.expect.md

@@ -119,10 +119,8 @@ bb2:
   Goto bb1
 bb1:
   predecessor blocks: bb0 bb2
-  a$5: phi(bb0: a$4, bb2: a$4)
-  props$6: phi(bb0: props$3, bb2: props$3)
-  Call mutate a$5.push(read props$6.d)
-  Return freeze a$5
+  Call mutate a$4.push(read props$3.d)
+  Return freeze a$4
 ```
 
 ## Code
@@ -136,8 +134,8 @@ function Component$0(props$3) {
     ("<<TODO: handle complex control flow in codegen>>");
   }
 
-  a$5.push(props$6.d);
-  return a$5;
+  a$4.push(props$3.d);
+  return a$4;
 }
 
 ```
@@ -221,10 +219,8 @@ bb2:
   Goto bb1
 bb1:
   predecessor blocks: bb2 bb0
-  a$5: phi(bb2: a$4, bb0: a$4)
-  props$6: phi(bb2: props$3, bb0: props$3)
-  Call mutate a$5.push(read props$6.d)
-  Return freeze a$5
+  Call mutate a$4.push(read props$3.d)
+  Return freeze a$4
 ```
 
 ## Code
@@ -234,8 +230,8 @@ function Component$0(props$3) {
   const a$4 = [];
   a$4.push(props$3.a);
   if (props$3.b) {
-    a$5.push(props$6.d);
-    return a$5;
+    a$4.push(props$3.d);
+    return a$4;
   }
 
   a$4.push(props$3.c);

compiler/forget/src/__tests__/fixtures/hir/conditional-on-mutable.expect.md

@@ -76,21 +76,14 @@ bb2:
   Goto bb1
 bb1:
   predecessor blocks: bb0 bb2
-  props$9: phi(bb0: props$6, bb2: props$6)
-  b$10: phi(bb0: b$8, bb2: b$8)
-  Foo$12: phi(bb0: Foo$4, bb2: Foo$4)
-  a$14: phi(bb0: a$7, bb2: a$7)
-  If (read props$9.p1) then:bb4 else:bb3
+  If (read props$6.p1) then:bb4 else:bb3
 bb4:
   predecessor blocks: bb1
-  Call mutate b$10.push(read props$9.p2)
+  Call mutate b$8.push(read props$6.p2)
   Goto bb3
 bb3:
   predecessor blocks: bb1 bb4
-  Foo$11: phi(bb1: Foo$12, bb4: Foo$12)
-  a$13: phi(bb1: a$14, bb4: a$14)
-  b$15: phi(bb1: b$10, bb4: b$10)
-  Const mutate $16 = JSX <read Foo$11 a={freeze a$13} b={freeze b$15} ></read Foo$11>
+  Const mutate $16 = JSX <read Foo$4 a={freeze a$7} b={freeze b$8} ></read Foo$4>
   Return read $16
 ```
 
@@ -105,12 +98,12 @@ function Component$0(props$6) {
     ("<<TODO: handle complex control flow in codegen>>");
   }
 
-  if (props$9.p1) {
-    b$10.push(props$9.p2);
+  if (props$6.p1) {
+    b$8.push(props$6.p2);
     ("<<TODO: handle complex control flow in codegen>>");
   }
 
-  return <Foo$11 a={a$13} b={b$15}></Foo$11>;
+  return <Foo$4 a={a$7} b={b$8}></Foo$4>;
 }
 
 ```
@@ -128,21 +121,14 @@ bb2:
   Goto bb1
 bb1:
   predecessor blocks: bb0 bb2
-  props$12: phi(bb0: props$8, bb2: props$8)
-  b$13: phi(bb0: b$10, bb2: b$10)
-  Foo$15: phi(bb0: Foo$6, bb2: Foo$6)
-  a$17: phi(bb0: a$9, bb2: a$9)
-  If (read props$12.p1) then:bb4 else:bb3
+  If (read props$8.p1) then:bb4 else:bb3
 bb4:
   predecessor blocks: bb1
-  Call mutate b$13.push(read props$12.p2)
+  Call mutate b$10.push(read props$8.p2)
   Goto bb3
 bb3:
   predecessor blocks: bb1 bb4
-  Foo$14: phi(bb1: Foo$15, bb4: Foo$15)
-  a$16: phi(bb1: a$17, bb4: a$17)
-  b$18: phi(bb1: b$13, bb4: b$13)
-  Const mutate $19 = JSX <read Foo$14 a={freeze a$16} b={freeze b$18} ></read Foo$14>
+  Const mutate $19 = JSX <read Foo$6 a={freeze a$9} b={freeze b$10} ></read Foo$6>
   Return read $19
 ```
 
@@ -157,12 +143,12 @@ function Component$0(props$8) {
     ("<<TODO: handle complex control flow in codegen>>");
   }
 
-  if (props$12.p1) {
-    b$13.push(props$12.p2);
+  if (props$8.p1) {
+    b$10.push(props$8.p2);
     ("<<TODO: handle complex control flow in codegen>>");
   }
 
-  return <Foo$14 a={a$16} b={b$18}></Foo$14>;
+  return <Foo$6 a={a$9} b={b$10}></Foo$6>;
 }
 
 ```

compiler/forget/src/__tests__/fixtures/hir/hooks-freeze-possibly-mutable-arguments.expect.md

@@ -40,12 +40,10 @@ bb3:
   Goto bb1
 bb1:
   predecessor blocks: bb3 bb2
-  useFreeze$13: phi(bb3: useFreeze$5, bb2: useFreeze$5)
   a$14: phi(bb3: a$12, bb2: a$11)
-  call$15: phi(bb3: call$6, bb2: call$6)
-  Call read useFreeze$13(freeze a$14)
-  Call read useFreeze$13(read a$14)
-  Call mutate call$15(read a$14)
+  Call read useFreeze$5(freeze a$14)
+  Call read useFreeze$5(read a$14)
+  Call mutate call$6(read a$14)
   Return read a$14
 ```
 
@@ -64,9 +62,9 @@ function Component$0(props$7) {
     ("<<TODO: handle complex control flow in codegen>>");
   }
 
-  useFreeze$13(a$14);
-  useFreeze$13(a$14);
-  call$15(a$14);
+  useFreeze$5(a$14);
+  useFreeze$5(a$14);
+  call$6(a$14);
   return a$14;
 }
 

compiler/forget/src/__tests__/fixtures/hir/independent-across-if.expect.md

@@ -107,10 +107,7 @@ bb2:
   Goto bb1
 bb1:
   predecessor blocks: bb0 bb2
-  Foo$11: phi(bb0: Foo$6, bb2: Foo$6)
-  a$12: phi(bb0: a$9, bb2: a$9)
-  b$13: phi(bb0: b$10, bb2: b$10)
-  Const mutate $14 = JSX <read Foo$11 a={freeze a$12} b={freeze b$13} ></read Foo$11>
+  Const mutate $14 = JSX <read Foo$6 a={freeze a$9} b={freeze b$10} ></read Foo$6>
   Return read $14
 ```
 
@@ -126,7 +123,7 @@ function Component$0(props$8) {
     ("<<TODO: handle complex control flow in codegen>>");
   }
 
-  return <Foo$11 a={a$12} b={b$13}></Foo$11>;
+  return <Foo$6 a={a$9} b={b$10}></Foo$6>;
 }
 
 ```

compiler/forget/src/__tests__/fixtures/hir/interdependent-across-if.expect.md

@@ -85,10 +85,7 @@ bb2:
   Goto bb1
 bb1:
   predecessor blocks: bb0 bb2
-  Foo$11: phi(bb0: Foo$6, bb2: Foo$6)
-  a$12: phi(bb0: a$9, bb2: a$9)
-  b$13: phi(bb0: b$10, bb2: b$10)
-  Const mutate $14 = JSX <read Foo$11 a={freeze a$12} b={freeze b$13} ></read Foo$11>
+  Const mutate $14 = JSX <read Foo$6 a={freeze a$9} b={freeze b$10} ></read Foo$6>
   Return read $14
 ```
 
@@ -103,7 +100,7 @@ function Component$0(props$8) {
     ("<<TODO: handle complex control flow in codegen>>");
   }
 
-  return <Foo$11 a={a$12} b={b$13}></Foo$11>;
+  return <Foo$6 a={a$9} b={b$10}></Foo$6>;
 }
 
 ```

compiler/forget/src/__tests__/fixtures/hir/mutable-lifetime-loops.expect.md

@@ -104,7 +104,6 @@ bb2:
   b$31: phi(bb1: b$20, bb3: b$23)
   c$32: phi(bb1: c$22, bb3: c$25)
   d$33: phi(bb1: d$24, bb3: d$26)
-  mutate$35: phi(bb1: mutate$27, bb3: mutate$27)
   If (read a$30) then:bb7 else:bb7
 bb7:
   predecessor blocks: bb2
@@ -118,7 +117,7 @@ bb11:
 bb13:
   predecessor blocks: bb11
   Const mutate $34 = null
-  Call mutate mutate$35(mutate d$33, read $34)
+  Call mutate mutate$27(mutate d$33, read $34)
   Return
 ```
 

compiler/forget/src/__tests__/fixtures/hir/reassignment-conditional.expect.md

@@ -33,13 +33,10 @@ bb2:
   Goto bb1
 bb1:
   predecessor blocks: bb0 bb2
-  Component$10: phi(bb0: Component$0, bb2: Component$0)
   x$11: phi(bb0: x$7, bb2: x$9)
-  y$13: phi(bb0: y$8, bb2: y$8)
-  props$14: phi(bb0: props$6, bb2: props$6)
-  Let mutate _$12 = JSX <read Component$10 x={freeze x$11} ></read Component$10>
-  Call read y$13.push(read props$14.p2)
-  Const mutate $15 = JSX <read Component$10 x={read x$11} y={read y$13} ></read Component$10>
+  Let mutate _$12 = JSX <read Component$0 x={freeze x$11} ></read Component$0>
+  Call read y$8.push(read props$6.p2)
+  Const mutate $15 = JSX <read Component$0 x={read x$11} y={read y$8} ></read Component$0>
   Return read $15
 ```
 
@@ -55,10 +52,10 @@ function Component$0(props$6) {
     ("<<TODO: handle complex control flow in codegen>>");
   }
 
-  let _$12 = <Component$10 x={x$11}></Component$10>;
+  let _$12 = <Component$0 x={x$11}></Component$0>;
 
-  y$13.push(props$14.p2);
-  return <Component$10 x={x$11} y={y$13}></Component$10>;
+  y$8.push(props$6.p2);
+  return <Component$0 x={x$11} y={y$8}></Component$0>;
 }
 
 ```

compiler/forget/src/__tests__/fixtures/hir/ssa-complex-multiple-if.expect.md

@@ -32,10 +32,9 @@ bb2:
   Goto bb1
 bb1:
   predecessor blocks: bb0 bb2
-  y$13: phi(bb0: y$8, bb2: y$8)
   x$17: phi(bb0: x$7, bb2: x$11)
   Const mutate $12 = 3
-  Const mutate $14 = Binary read y$13 === read $12
+  Const mutate $14 = Binary read y$8 === read $12
   If (read $14) then:bb4 else:bb3
 bb4:
   predecessor blocks: bb1
@@ -59,7 +58,7 @@ function foo$0() {
     ("<<TODO: handle complex control flow in codegen>>");
   }
 
-  if (y$13 === 3) {
+  if (y$8 === 3) {
     x$15 = 5;
     ("<<TODO: handle complex control flow in codegen>>");
   }

compiler/forget/src/__tests__/fixtures/hir/ssa-for-of.expect.md

@@ -0,0 +1,51 @@
+
+## Input
+
+```javascript
+function foo(cond) {
+  let items = [];
+  for (const item of items) {
+    let y = 0;
+    if (cond) {
+      y = 1;
+    }
+  }
+  return items;
+}
+
+```
+
+## HIR
+
+```
+bb0:
+  Let mutate items$5 = Array []
+  Goto bb1
+bb1:
+  predecessor blocks: bb0 bb3 bb5
+  items$6: phi(bb0: items$5, bb3: items$6, bb5: items$6)
+  cond$8: phi(bb0: cond$4, bb3: cond$8, bb5: cond$8)
+  If (read items$6) then:bb3 else:bb2
+bb3:
+  predecessor blocks: bb1
+  Let mutate y$7 = 0
+  If (read cond$8) then:bb5 else:bb1
+bb5:
+  predecessor blocks: bb3
+  Reassign mutate y$9 = 1
+  Goto bb1
+bb2:
+  predecessor blocks: bb1
+  Return freeze items$6
+```
+
+## Code
+
+```javascript
+function foo$0(cond$4) {
+  let items$5 = [];
+  ("<<TODO: handle complex control flow in codegen>>");
+}
+
+```
+      

compiler/forget/src/__tests__/fixtures/hir/ssa-for-of.js

@@ -0,0 +1,10 @@
+function foo(cond) {
+  let items = [];
+  for (const item of items) {
+    let y = 0;
+    if (cond) {
+      y = 1;
+    }
+  }
+  return items;
+}

compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md

@@ -53,13 +53,10 @@ bb2:
   Goto bb1
 bb1:
   predecessor blocks: bb0 bb6 bb2
-  Component$17: phi(bb0: Component$0, bb6: Component$0, bb2: Component$0)
-  x$18: phi(bb0: x$10, bb6: x$10, bb2: x$10)
   y$20: phi(bb0: y$11, bb6: y$15, bb2: y$16)
-  props$21: phi(bb0: props$9, bb6: props$9, bb2: props$9)
-  Const mutate child$19 = JSX <read Component$17 data={freeze x$18} ></read Component$17>
-  Call read y$20.push(read props$21.p4)
-  Const mutate $22 = JSX <read Component$17 data={freeze y$20} >{read child$19}</read Component$17>
+  Const mutate child$19 = JSX <read Component$0 data={freeze x$10} ></read Component$0>
+  Call read y$20.push(read props$9.p4)
+  Const mutate $22 = JSX <read Component$0 data={freeze y$20} >{read child$19}</read Component$0>
   Return read $22
 ```
 
@@ -88,9 +85,9 @@ function Component$0(props$9) {
     }
   }
 
-  const child$19 = <Component$17 data={x$18}></Component$17>;
-  y$20.push(props$21.p4);
-  return <Component$17 data={y$20}>{child$19}</Component$17>;
+  const child$19 = <Component$0 data={x$10}></Component$0>;
+  y$20.push(props$9.p4);
+  return <Component$0 data={y$20}>{child$19}</Component$0>;
 }
 
 ```

compiler/forget/src/__tests__/fixtures/hir/switch.expect.md

@@ -44,20 +44,14 @@ bb4:
   Goto bb2
 bb2:
   predecessor blocks: bb4 bb0
-  x$14: phi(bb4: x$9, bb0: x$9)
-  Component$17: phi(bb4: Component$0, bb0: Component$0)
-  props$22: phi(bb4: props$8, bb0: props$8)
-  Reassign mutate y$15 = read x$14
+  Reassign mutate y$15 = read x$9
   Goto bb1
 bb1:
   predecessor blocks: bb2 bb0
-  Component$16: phi(bb2: Component$17, bb0: Component$0)
-  x$18: phi(bb2: x$14, bb0: x$9)
   y$20: phi(bb2: y$15, bb0: y$10)
-  props$21: phi(bb2: props$22, bb0: props$8)
-  Const mutate child$19 = JSX <read Component$16 data={freeze x$18} ></read Component$16>
-  Call read y$20.push(read props$21.p4)
-  Const mutate $23 = JSX <read Component$16 data={read y$20} >{read child$19}</read Component$16>
+  Const mutate child$19 = JSX <read Component$0 data={freeze x$9} ></read Component$0>
+  Call read y$20.push(read props$8.p4)
+  Const mutate $23 = JSX <read Component$0 data={read y$20} >{read child$19}</read Component$0>
   Return read $23
 ```
 
@@ -76,14 +70,14 @@ function Component$0(props$8) {
     }
 
     case false: {
-      y$15 = x$14;
+      y$15 = x$9;
       ("<<TODO: handle complex control flow in codegen>>");
     }
   }
 
-  const child$19 = <Component$16 data={x$18}></Component$16>;
-  y$20.push(props$21.p4);
-  return <Component$16 data={y$20}>{child$19}</Component$16>;
+  const child$19 = <Component$0 data={x$9}></Component$0>;
+  y$20.push(props$8.p4);
+  return <Component$0 data={y$20}>{child$19}</Component$0>;
 }
 
 ```