[rust] Initial port of EnterSSA

This is a first pass at porting EnterSSA to Rust. First pass in the sense that it's hard to fully test it, and also in the sense that we'll likely figure out even better ways to work w the HIR as we iterate. Oh and i didn't do recusing into function expressions yet, since we can't even represent function expressions yet, and that may require modifying the design a bit (though i have an idea that i think will work, which is for the Builder to have an optional parent. When we encounter a block with no predecessors, we check the parent. I _think_ this will make all the borrowing "just work"). A few notes: Rust's compilation model parallelizes and incrementally computes at the crate granularity, so builds are faster if we split up our code into more but smaller crates. Setting up a clean dependency graph can dramatically improve build performance too. For example, to run the `fixtures` tests we can build `estree` and `hir` in parallel, then once those build _all_ of our other crates can be built in parallel until we get to `fixtures` which depends on the everything else. The various passes don't have any build dependencies on each other so they can build in parallel. Hence the new code for SSA stuff is in a separate `hir-ssa` crate. We should similarly group other passes (approximately one crate per folder in the babel-plugin-react-forget/src/ directory, eg SSA, Inference, Optimization, etc). Second, shared mutable ownership can be modeled in Rust but requires wrappers such as `Rc<RefCell<>>`. It's generally more efficient and more idiomatic to rethink the data model and algorithm. For EnterSSA, the Builder object holds a reference into the HIR that it only ever reads, and the pass (which drives the builder) also holds a reference into the HIR, which it mutates. The previous PR split up Blocks and Instructions, and the value of that is more apparent in `enter_ssa()`. The Rust equivalent of the builder holds a _shared_ (immutable) reference to just the HIR's blocks, while the pass (driving the builder) holds a _unique_ (mutable) reference to just the HIR's instructions. This lets us keep the overall feel of the algorithm while keeping Rust happy. Also note that the other change — to making operands be InstrIx indices into the instructions array — means that the SSA logic is simpler. Most instructions don't have to be visited at all, since they don't deal with loads/stores. Terminals also don't need to be visited, since they reference instructions, not identifiers. The Phi concept seems to just work too. I also updated the printer to print predecessors and phis.
2025-11-01 09:12:30 +00:00 · 2023-07-12 12:38:13 +09:00
parent e9d2822e92
commit c7ccc5dd30
17 changed files with 548 additions and 79 deletions
@@ -6,8 +6,8 @@ use estree::{
 };
 use hir::{
    ArrayElement, BlockKind, BranchTerminal, Environment, ForTerminal, Function, GotoKind,
-    Identifier, IdentifierOperand, InstrIx, InstructionKind, InstructionValue, LValue, LoadGlobal,
-    LoadLocal, Operand, PrimitiveValue, TerminalValue,
+    IdentifierOperand, InstrIx, InstructionKind, InstructionValue, LValue, LoadGlobal, LoadLocal,
+    Operand, PrimitiveValue, TerminalValue,
 };

 use crate::{
@@ -23,17 +23,17 @@ use crate::{
 /// Failures generally include nonsensical input (`delete 1`) or syntax
 /// that is not yet supported.
 pub fn build<'a>(
-    environment: &'a Environment<'a>,
+    env: &'a Environment<'a>,
    fun: FunctionDeclaration,
-) -> Result<Function<'a>, BuildDiagnostic> {
-    let mut builder = Builder::new(environment);
+) -> Result<&'a mut Function<'a>, BuildDiagnostic> {
+    let mut builder = Builder::new(env);

    match fun.function.body {
        Some(estree::FunctionBody::BlockStatement(body)) => {
-            lower_block_statement(environment, &mut builder, *body, None)?
+            lower_block_statement(env, &mut builder, *body, None)?
        }
        Some(estree::FunctionBody::Expression(body)) => {
-            lower_expression(environment, &mut builder, body)?;
+            lower_expression(env, &mut builder, body)?;
        }
        None => {
            return Err(BuildDiagnostic::new(
@@ -44,11 +44,26 @@ pub fn build<'a>(
        }
    }

+    let mut params = Vec::with_capacity_in(fun.function.params.len(), &env.allocator);
+    for param in fun.function.params {
+        match param {
+            Pattern::Identifier(param) => {
+                let identifier = lower_identifier_for_assignment(
+                    env,
+                    &mut builder,
+                    InstructionKind::Let,
+                    *param,
+                )?;
+                params.push(identifier);
+            }
+        }
+    }
+
    // In case the function did not explicitly return, terminate the final
    // block with an explicit `return undefined`. If the function *did* return,
    // this will be unreachable and get pruned later.
    let implicit_return_value = lower_value_to_temporary(
-        environment,
+        env,
        &mut builder,
        InstructionValue::Primitive(hir::Primitive {
            value: PrimitiveValue::Undefined,
@@ -65,11 +80,16 @@ pub fn build<'a>(
    );

    let body = builder.build()?;
-    Ok(Function {
+    Ok(env.alloc(Function {
+        id: fun
+            .function
+            .id
+            .map(|id| String::from_str_in(&id.name, &env.allocator)),
        body,
+        params,
        is_async: fun.function.is_async,
        is_generator: fun.function.is_generator,
-    })
+    }))
 }

 fn lower_block_statement<'a>(
@@ -171,6 +171,7 @@ impl<'a> Builder<'a> {
                        value: terminal,
                    },
                    predecessors: Default::default(),
+                    phis: Vec::new_in(&self.environment.allocator),
                },
                &self.environment.allocator,
            ),
@@ -226,6 +227,7 @@ impl<'a> Builder<'a> {
                        value: terminal,
                    },
                    predecessors: Default::default(),
+                    phis: Vec::new_in(&self.environment.allocator),
                },
                &self.environment.allocator,
            ),
@@ -11,6 +11,7 @@ insta = "1.30.0"
 estree = { path = "../estree" }
 estree-swc = { path = "../estree-swc" }
 hir = { path = "../hir" }
+hir-ssa = { path = "../hir-ssa" }
 build-hir = { path = "../build-hir" }
 bumpalo = { version = "3.13.0", features = ["collections"] }
 miette = { version = "5.9.0", features = ["backtrace", "fancy"] }
@@ -0,0 +1,16 @@
+function Component(a, b) {
+  let x;
+  let y = 0;
+  let z = 10;
+  if (a) {
+    x = 1;
+    if (b) {
+      z = 20;
+    } else {
+      z = 30;
+    }
+  } else {
+    x = 2;
+  }
+  return x + y + z;
+}
@@ -5,6 +5,7 @@ use bumpalo::Bump;
 use estree::{ModuleItem, Statement};
 use estree_swc::parse;
 use hir::{Environment, Print, Registry};
+use hir_ssa::enter_ssa;
 use insta::{assert_snapshot, glob};
 use miette::{NamedSource, Report};

@@ -31,8 +32,9 @@ fn fixtures() {
                        output.push_str("\n\n");
                    }
                    match build(&environment, *fun) {
-                        Ok(hir) => {
-                            hir.print(&hir.body, &mut output).unwrap();
+                        Ok(mut fun) => {
+                            enter_ssa(&environment, &mut fun).unwrap();
+                            fun.print(&fun.body, &mut output).unwrap();
                        }
                        Err(error) => {
                            write!(&mut output, "{}", error,).unwrap();
@@ -14,33 +14,42 @@ function foo() {


 Output:
+function foo(
+)
 entry bb0
-bb0
+bb0 (block)
  [0] #0 = 0
-  [1] #1 = StoreLocal Let unknown x$0 = unknown #0
+  [1] #1 = StoreLocal Let unknown x$2 = unknown #0
  [2] For init=bb3 test=bb1 update=bb4 body=bb5 fallthrough=bb2
-bb3
+bb3 (loop)
+  predecessors: bb0
  [3] #2 = 0
-  [4] #3 = StoreLocal Let unknown i$1 = unknown #2
+  [4] #3 = StoreLocal Let unknown i$3 = unknown #2
  [5] Goto bb1
-bb1
-  [6] #12 = LoadLocal unknown i$1
+bb1 (loop)
+  predecessors: bb3, bb4
+  i$4: phi(bb3: i$3, bb4: i$7)
+  x$5: phi(bb3: x$2, bb4: x$6)
+  [6] #12 = LoadLocal unknown i$4
  [7] #13 = 10
  [8] #14 = Binary unknown #12 < unknown #13
  [9] Branch unknown #14 consequent=bb5 alternate=bb2
-bb5
-  [10] #8 = LoadLocal unknown x$0
-  [11] #9 = LoadLocal unknown i$1
+bb5 (block)
+  predecessors: bb1
+  [10] #8 = LoadLocal unknown x$5
+  [11] #9 = LoadLocal unknown i$4
  [12] #10 = Binary unknown #8 + unknown #9
-  [13] #11 = StoreLocal Reassign unknown x$0 = unknown #10
+  [13] #11 = StoreLocal Reassign unknown x$6 = unknown #10
  [14] Goto bb4
-bb4
-  [15] #4 = LoadLocal unknown i$1
+bb4 (loop)
+  predecessors: bb5
+  [15] #4 = LoadLocal unknown i$4
  [16] #5 = 1
  [17] #6 = Binary unknown #4 + unknown #5
-  [18] #7 = StoreLocal Reassign unknown i$1 = unknown #6
+  [18] #7 = StoreLocal Reassign unknown i$7 = unknown #6
  [19] Goto bb1
-bb2
-  [20] #15 = LoadLocal unknown x$0
+bb2 (block)
+  predecessors: bb1
+  [20] #15 = LoadLocal unknown x$5
  [21] Return unknown #15

@@ -23,17 +23,20 @@ function id(x) {


 Output:
+function id(
+  unknown x$4,
+)
 entry bb0
-bb0
+bb0 (block)
  [0] #0 = LoadGlobal Math
  [1] #1 = LoadGlobal id
  [2] #2 = true
-  [3] #3 = StoreLocal Let unknown y$1 = unknown #2
+  [3] #3 = StoreLocal Let unknown y$5 = unknown #2
  [4] #4 = false
-  [5] #5 = StoreLocal Reassign unknown y$1 = unknown #4
-  [6] #6 = LoadLocal unknown y$1
-  [7] #7 = DeclareLocal Let unknown z$2
-  [8] #8 = LoadLocal unknown z$2
-  [9] #9 = LoadLocal unknown x$3
+  [5] #5 = StoreLocal Reassign unknown y$6 = unknown #4
+  [6] #6 = LoadLocal unknown y$6
+  [7] #7 = DeclareLocal Let unknown z$7
+  [8] #8 = LoadLocal unknown z$7
+  [9] #9 = LoadLocal unknown x$4
  [10] Return unknown #9

@@ -15,18 +15,27 @@ function foo(a, b, c, d) {


 Output:
+function foo(
+  unknown a$4,
+  unknown b$5,
+  unknown c$6,
+  unknown d$7,
+)
 entry bb0
-bb0
-  [0] #2 = LoadLocal unknown a$2
+bb0 (block)
+  [0] #2 = LoadLocal unknown a$4
  [1] If unknown #2 consequent=bb2 alternate=bb4 fallthrough=bb1
-bb2
-  [2] #0 = LoadLocal unknown b$0
+bb2 (block)
+  predecessors: bb0
+  [2] #0 = LoadLocal unknown b$5
  [3] Return unknown #0
-bb4
-  [4] #1 = LoadLocal unknown c$1
+bb4 (block)
+  predecessors: bb0
+  [4] #1 = LoadLocal unknown c$6
  [5] Goto bb1
-bb1
-  [6] #3 = LoadLocal unknown d$3
+bb1 (block)
+  predecessors: bb4
+  [6] #3 = LoadLocal unknown d$7
  [7] #4 = <undefined>
  [8] Return unknown #4

@@ -11,8 +11,10 @@ function test() {


 Output:
+function test(
+)
 entry bb0
-bb0
+bb0 (block)
  [0] #0 = true
  [1] #1 = false
  [2] #2 = null
@@ -0,0 +1,77 @@
+---
+source: crates/fixtures/tests/fixtures_test.rs
+expression: "format!(\"Input:\\n{input}\\n\\nOutput:\\n{output}\")"
+input_file: crates/fixtures/tests/fixtures/ssa-reassign-if.js
+---
+Input:
+function Component(a, b) {
+  let x;
+  let y = 0;
+  let z = 10;
+  if (a) {
+    x = 1;
+    if (b) {
+      z = 20;
+    } else {
+      z = 30;
+    }
+  } else {
+    x = 2;
+  }
+  return x + y + z;
+}
+
+
+Output:
+function Component(
+  unknown a$5,
+  unknown b$6,
+)
+entry bb0
+bb0 (block)
+  [0] #0 = DeclareLocal Let unknown x$7
+  [1] #1 = 0
+  [2] #2 = StoreLocal Let unknown y$8 = unknown #1
+  [3] #3 = 10
+  [4] #4 = StoreLocal Let unknown z$9 = unknown #3
+  [5] #14 = LoadLocal unknown a$5
+  [6] If unknown #14 consequent=bb2 alternate=bb6 fallthrough=bb1
+bb2 (block)
+  predecessors: bb0
+  [7] #5 = 1
+  [8] #6 = StoreLocal Reassign unknown x$10 = unknown #5
+  [9] #11 = LoadLocal unknown b$6
+  [10] If unknown #11 consequent=bb4 alternate=bb5 fallthrough=bb3
+bb4 (block)
+  predecessors: bb2
+  [11] #7 = 20
+  [12] #8 = StoreLocal Reassign unknown z$11 = unknown #7
+  [13] Goto bb3
+bb5 (block)
+  predecessors: bb2
+  [14] #9 = 30
+  [15] #10 = StoreLocal Reassign unknown z$12 = unknown #9
+  [16] Goto bb3
+bb3 (block)
+  predecessors: bb4, bb5
+  x$15: phi(bb4: x$10, bb5: x$10)
+  y$17: phi(bb4: y$8, bb5: y$8)
+  z$19: phi(bb4: z$11, bb5: z$12)
+  [17] Goto bb1
+bb6 (block)
+  predecessors: bb0
+  [18] #12 = 2
+  [19] #13 = StoreLocal Reassign unknown x$13 = unknown #12
+  [20] Goto bb1
+bb1 (block)
+  predecessors: bb3, bb6
+  x$14: phi(bb3: x$15, bb6: x$13)
+  y$16: phi(bb3: y$17, bb6: y$8)
+  z$18: phi(bb3: z$19, bb6: z$9)
+  [21] #15 = LoadLocal unknown x$14
+  [22] #16 = LoadLocal unknown y$16
+  [23] #17 = Binary unknown #15 + unknown #16
+  [24] #18 = LoadLocal unknown z$18
+  [25] #19 = Binary unknown #17 + unknown #18
+  [26] Return unknown #19
+
@@ -8,5 +8,6 @@ edition = "2021"
 [dependencies]
 hir = { path = "../hir" }
 bumpalo = "3.13.0"
+indexmap = "2.0.0"
 miette = { version = "5.9.0" }
 thiserror = "1.0.41"
@@ -1,38 +1,248 @@
-use std::collections::HashSet;
+use std::{cell::RefCell, rc::Rc};

-use hir::{BlockId, Environment, Function, HIR};
+use bumpalo::collections::{CollectIn, Vec};
+use hir::{
+    BasicBlock, BlockId, Blocks, Environment, Function, Identifier, IdentifierData, IdentifierId,
+    IdentifierOperand, Instruction, LValue, MutableRange, Phi, HIR,
+};
+use indexmap::{IndexMap, IndexSet};
 use thiserror::Error;

 #[derive(Error, Debug)]
 #[error("Error constructing SSA form")]
 pub struct SSAError;

-pub fn enter_ssa<'a>(env: &'a Environment<'a>, hir: &mut Function<'a>) -> Result<(), SSAError> {
-    let mut builder = Builder::new(env);
-    enter_ssa_impl(&mut builder, hir, hir.body.entry)
-}
-
-struct Builder<'a> {
-    env: &'a Environment<'a>,
-}
-
-impl<'a> Builder<'a> {
-    fn new(env: &'a Environment<'a>) -> Self {
-        Self { env }
+pub fn enter_ssa<'a>(env: &Environment<'a>, fun: &mut Function<'a>) -> Result<(), SSAError> {
+    let blocks = &fun.body.blocks;
+    let instructions = &mut fun.body.instructions;
+    let mut builder = Builder::new(env, fun.body.entry, blocks);
+    for param in &mut fun.params {
+        builder.visit_param(param);
    }
-}
+    enter_ssa_impl(&mut builder, instructions)?;

-fn enter_ssa_impl<'a>(
-    builder: &mut Builder<'a>,
-    hir: &mut Function<'a>,
-    root_block_id: BlockId,
-) -> Result<(), SSAError> {
-    let mut visited = HashSet::with_capacity(hir.body.blocks.len());
-    for (block_id, block) in hir.body.blocks.iter_mut() {
-        if !visited.insert(*block_id) {
-            // Visited the same block twice
-            return Err(SSAError);
-        }
+    let mut states = builder.complete();
+
+    for block in fun.body.blocks.values_mut() {
+        let state = states.remove(&block.id).unwrap();
+        block.phis = state.phis;
    }
+
    Ok(())
 }
+
+fn enter_ssa_impl<'a, 'e, 'f>(
+    builder: &mut Builder<'a, 'e, 'f>,
+    instructions: &mut Vec<'a, Instruction<'a>>,
+) -> Result<(), SSAError> {
+    builder.each_block(|block, builder| {
+        for instr_ix in &block.instructions {
+            let instr = &mut instructions[usize::from(*instr_ix)];
+            instr.each_identifier_store(|store| builder.visit_store(store));
+            instr.each_identifier_load(|load| builder.visit_load(load));
+        }
+    });
+    Ok(())
+}
+
+#[derive(Debug)]
+struct Builder<'a, 'e, 'f> {
+    env: &'e Environment<'a>,
+    blocks: &'f Blocks<'a>,
+
+    states: IndexMap<BlockId, BlockState<'a>>,
+    current: BlockId,
+    unsealed_predecessors: IndexMap<BlockId, usize>,
+    unknown: IndexSet<IdentifierId>,
+    context: IndexSet<IdentifierId>,
+}
+
+#[derive(Debug)]
+struct BlockState<'a> {
+    defs: IndexMap<IdentifierId, Identifier<'a>>,
+    incomplete_phis: Vec<'a, IncompletePhi<'a>>,
+    phis: Vec<'a, Phi<'a>>,
+}
+
+impl<'a> BlockState<'a> {
+    fn new(env: &Environment<'a>) -> Self {
+        Self {
+            defs: Default::default(),
+            incomplete_phis: Vec::new_in(&env.allocator),
+            phis: Vec::new_in(&env.allocator),
+        }
+    }
+}
+
+#[derive(Debug)]
+struct IncompletePhi<'a> {
+    old_id: Identifier<'a>,
+    new_id: Identifier<'a>,
+}
+
+impl<'a, 'e, 'f> Builder<'a, 'e, 'f> {
+    fn new(env: &'e Environment<'a>, entry: BlockId, blocks: &'f Blocks<'a>) -> Self {
+        let states = blocks
+            .keys()
+            .map(|block_id| (*block_id, BlockState::new(env)))
+            .collect();
+        Self {
+            env,
+            blocks,
+            states,
+            current: entry,
+            unsealed_predecessors: Default::default(),
+            unknown: Default::default(),
+            context: Default::default(),
+        }
+    }
+
+    fn complete(self) -> IndexMap<BlockId, BlockState<'a>> {
+        self.states
+    }
+
+    fn next_ssa_id(&self) -> IdentifierId {
+        self.env.next_identifier_id()
+    }
+
+    fn visit_store(&mut self, lvalue: &mut LValue<'a>) -> () {
+        let old_identifier = &lvalue.identifier.identifier;
+        // TODO: use Result (?)
+        assert!(!self.unknown.contains(&old_identifier.id), "EnterSSA: Expected identifier to be defined before being used. Identifier {old_identifier:?} is undefined.");
+
+        if self.context.contains(&old_identifier.id) {
+            let new_identifier = self.get_id_at(self.current, old_identifier);
+            lvalue.identifier.identifier = new_identifier;
+            return;
+        }
+
+        let new_identifier = self.make_identifier(old_identifier);
+        let state = self.states.get_mut(&self.current).unwrap();
+        state.defs.insert(old_identifier.id, new_identifier.clone());
+        lvalue.identifier.identifier = new_identifier;
+    }
+
+    fn visit_param(&mut self, param: &mut IdentifierOperand<'a>) -> () {
+        let old_identifier = &param.identifier;
+        let new_identifier = self.make_identifier(old_identifier);
+        let state = self.states.get_mut(&self.current).unwrap();
+        state.defs.insert(old_identifier.id, new_identifier.clone());
+        param.identifier = new_identifier;
+    }
+
+    fn visit_load(&mut self, local: &mut IdentifierOperand<'a>) -> () {
+        let new_identifier = self.get_id_at(self.current, &local.identifier);
+        local.identifier = new_identifier;
+    }
+
+    fn get_id_at(&mut self, block_id: BlockId, old_identifier: &Identifier<'a>) -> Identifier<'a> {
+        // Check if we've already resolved this identifier in this block
+        let state = self.states.get(&block_id).unwrap();
+        if let Some(identifier) = state.defs.get(&old_identifier.id) {
+            return identifier.clone();
+        }
+        // Else we have to look at predecessor blocks: bail if no predecessors
+        let block = self.blocks.get(&block_id).unwrap();
+        if block.predecessors.is_empty() {
+            println!("Unable to find previous id for {old_identifier:?}");
+            self.unknown.insert(old_identifier.id);
+            return old_identifier.clone();
+        }
+        // If we haven't visited all predecessors, synthesize a new identifier
+        // and save it as an incomplete phi
+        if self.unsealed_predecessors.get(&block_id).cloned().unwrap() > 0 {
+            let new_identifier = self.make_identifier(old_identifier);
+            let state = self.states.get_mut(&block_id).unwrap();
+            state.incomplete_phis.push(IncompletePhi {
+                old_id: old_identifier.clone(),
+                new_id: new_identifier.clone(),
+            });
+            state.defs.insert(old_identifier.id, new_identifier.clone());
+            return new_identifier;
+        }
+        // If exactly one predecessor, check to see if we have a definition there
+        if block.predecessors.len() == 1 {
+            let predecessor = block.predecessors.iter().next().unwrap();
+            let new_identifier = self.get_id_at(*predecessor, old_identifier);
+            let state = self.states.get_mut(&block_id).unwrap();
+            state.defs.insert(old_identifier.id, new_identifier.clone());
+            return new_identifier;
+        }
+        // There are multiple predecessors, we may need a phi
+        let new_identifier = self.make_identifier(old_identifier);
+        let state = self.states.get_mut(&block_id).unwrap();
+        state.defs.insert(old_identifier.id, new_identifier.clone());
+        self.add_phi(block_id, old_identifier, new_identifier)
+    }
+
+    fn add_phi(
+        &mut self,
+        block_id: BlockId,
+        old_identifier: &Identifier<'a>,
+        new_identifier: Identifier<'a>,
+    ) -> Identifier<'a> {
+        let mut phi = Phi {
+            identifier: new_identifier.clone(),
+            operands: Default::default(),
+        };
+        let block = self.blocks.get(&block_id).unwrap();
+        let preds = block.predecessors.clone();
+        for pred_block_id in preds {
+            let pred_id = self.get_id_at(pred_block_id, old_identifier);
+            phi.operands.insert(pred_block_id, pred_id);
+        }
+        let state = self.states.get_mut(&block_id).unwrap();
+        state.phis.push(phi);
+        new_identifier
+    }
+
+    fn make_identifier(&self, old_identifier: &Identifier<'a>) -> Identifier<'a> {
+        let old_data = old_identifier.data.borrow();
+        Identifier {
+            id: self.next_ssa_id(),
+            name: old_identifier.name.clone(),
+            data: Rc::new(RefCell::new(IdentifierData {
+                mutable_range: MutableRange::new(),
+                scope: None,
+                type_: old_data.type_.clone(),
+            })),
+        }
+    }
+
+    fn fix_incomplete_phis(&mut self, block_id: BlockId) -> () {
+        let state = self.states.get_mut(&block_id).unwrap();
+        let incomplete_phis =
+            std::mem::replace(&mut state.incomplete_phis, Vec::new_in(&self.env.allocator));
+        for phi in incomplete_phis {
+            self.add_phi(block_id, &phi.old_id, phi.new_id);
+        }
+    }
+
+    fn each_block<F>(&mut self, mut f: F) -> ()
+    where
+        F: FnMut(&BasicBlock<'a>, &mut Self) -> (),
+    {
+        let mut visited = IndexSet::new();
+        let block_ids: Vec<_> = self.blocks.keys().cloned().collect_in(self.env.allocator);
+        for block_id in block_ids {
+            visited.insert(block_id);
+            self.current = block_id;
+            let block = self.blocks.get(&block_id).unwrap();
+            f(block, self);
+            let successors = block.terminal.value.successors();
+            for successor in successors {
+                let block = self.blocks.get(&successor).unwrap();
+                let count = self
+                    .unsealed_predecessors
+                    .get(&successor)
+                    .cloned()
+                    .unwrap_or(block.predecessors.len())
+                    - 1;
+                self.unsealed_predecessors.insert(successor, count);
+                if count == 0 && visited.contains(&successor) {
+                    self.fix_incomplete_phis(successor)
+                }
+            }
+        }
+    }
+}
@@ -1,6 +1,8 @@
-use std::collections::HashSet;
+use std::fmt::Display;

-use crate::{id_types::BlockId, InstrIx, Terminal};
+use indexmap::{IndexMap, IndexSet};
+
+use crate::{id_types::BlockId, Identifier, InstrIx, Terminal};

 /// Represents a sequence of instructions that will always[1] execute
 /// consecutively. Concretely, a block may have zero or more instructions
@@ -26,7 +28,15 @@ pub struct BasicBlock<'a> {
    pub terminal: Terminal<'a>,

    /// The immediate predecessors of this block
-    pub predecessors: HashSet<BlockId>,
+    pub predecessors: IndexSet<BlockId>,
+
+    pub phis: bumpalo::collections::Vec<'a, Phi<'a>>,
+}
+
+#[derive(Debug)]
+pub struct Phi<'a> {
+    pub identifier: Identifier<'a>,
+    pub operands: IndexMap<BlockId, Identifier<'a>>,
 }

 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
@@ -36,3 +46,14 @@ pub enum BlockKind {
    Loop,
    Sequence,
 }
+
+impl Display for BlockKind {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Block => f.write_str("block"),
+            Self::Value => f.write_str("value"),
+            Self::Loop => f.write_str("loop"),
+            Self::Sequence => f.write_str("sequence"),
+        }
+    }
+}
@@ -1,13 +1,19 @@
 use std::cell::Cell;

-use bumpalo::{boxed::Box, collections::Vec};
+use bumpalo::{
+    boxed::Box,
+    collections::{String, Vec},
+};
 use indexmap::IndexMap;

-use crate::{BasicBlock, BlockId, Instruction};
+use crate::{BasicBlock, BlockId, IdentifierOperand, Instruction};

 /// Represents either a React function or a function expression
+#[derive(Debug)]
 pub struct Function<'a> {
+    pub id: Option<String<'a>>,
    pub body: HIR<'a>,
+    pub params: Vec<'a, IdentifierOperand<'a>>,
    pub is_async: bool,
    pub is_generator: bool,
 }
@@ -16,18 +22,21 @@ pub struct Function<'a> {
 /// Blocks are stored in reverse postorder (predecessors before successors)
 /// so that compiler passes can complete forward data flow analysis in a
 /// single pass over the CFG in the case where there are no loops.
+#[derive(Debug)]
 pub struct HIR<'a> {
    /// The id of the first block
    pub entry: BlockId,

    /// Blocks are stored in a map for easy retrieval by their id,
    /// but the blocks are in reverse postorder
-    pub blocks: IndexMap<BlockId, Box<'a, BasicBlock<'a>>>,
+    pub blocks: Blocks<'a>,

    /// All instructions for the block. This may contain unused items,
    pub instructions: Vec<'a, Instruction<'a>>,
 }

+pub type Blocks<'a> = IndexMap<BlockId, Box<'a, BasicBlock<'a>>>;
+
 impl<'a> HIR<'a> {
    pub fn block(&self, id: BlockId) -> &BasicBlock<'a> {
        self.blocks.get(&id).unwrap()
@@ -11,6 +11,48 @@ pub struct Instruction<'a> {
    pub value: InstructionValue<'a>,
 }

+impl<'a> Instruction<'a> {
+    pub fn each_identifier_store<F>(&mut self, mut f: F) -> ()
+    where
+        F: FnMut(&mut LValue<'a>) -> (),
+    {
+        match &mut self.value {
+            InstructionValue::Array(_) => {}
+            InstructionValue::Binary(_) => {}
+            InstructionValue::DeclareContext(instr) => {
+                f(&mut instr.lvalue);
+            }
+            InstructionValue::DeclareLocal(instr) => {
+                f(&mut instr.lvalue);
+            }
+            InstructionValue::LoadContext(_) => {}
+            InstructionValue::LoadGlobal(_) => {}
+            InstructionValue::LoadLocal(_) => {}
+            InstructionValue::Primitive(_) => {}
+            InstructionValue::StoreLocal(instr) => {
+                f(&mut instr.lvalue);
+            }
+        }
+    }
+
+    pub fn each_identifier_load<F>(&mut self, mut f: F) -> ()
+    where
+        F: FnMut(&mut IdentifierOperand<'a>) -> (),
+    {
+        match &mut self.value {
+            InstructionValue::Array(_) => {}
+            InstructionValue::Binary(_) => {}
+            InstructionValue::DeclareContext(_) => {}
+            InstructionValue::DeclareLocal(_) => {}
+            InstructionValue::LoadContext(_) => {}
+            InstructionValue::LoadGlobal(_) => {}
+            InstructionValue::LoadLocal(instr) => f(&mut instr.place),
+            InstructionValue::Primitive(_) => {}
+            InstructionValue::StoreLocal(_) => {}
+        }
+    }
+}
+
 #[derive(Debug)]
 pub enum InstructionValue<'a> {
    Array(Array<'a>),
@@ -2,7 +2,7 @@ use std::fmt::{Result, Write};

 use crate::{
    ArrayElement, BasicBlock, Function, Identifier, IdentifierOperand, Instruction,
-    InstructionValue, LValue, Operand, PrimitiveValue, Terminal, TerminalValue, HIR,
+    InstructionValue, LValue, Operand, Phi, PrimitiveValue, Terminal, TerminalValue, HIR,
 };

 /// Trait for HIR types to describe how they print themselves.
@@ -16,6 +16,20 @@ pub trait Print<'a> {

 impl<'a> Print<'a> for Function<'a> {
    fn print(&self, hir: &HIR<'a>, out: &mut impl Write) -> Result {
+        writeln!(
+            out,
+            "function {}(",
+            match &self.id {
+                Some(id) => id,
+                None => "<anonymous>",
+            }
+        )?;
+        for param in &self.params {
+            write!(out, "  ")?;
+            param.print(hir, out)?;
+            writeln!(out, ",")?;
+        }
+        writeln!(out, ")")?;
        writeln!(out, "entry {}", self.body.entry)?;
        for (_, block) in self.body.blocks.iter() {
            block.print(hir, out)?;
@@ -26,7 +40,21 @@ impl<'a> Print<'a> for Function<'a> {

 impl<'a> Print<'a> for BasicBlock<'a> {
    fn print(&self, hir: &HIR<'a>, out: &mut impl Write) -> Result {
-        writeln!(out, "{}", self.id)?;
+        writeln!(out, "{} ({})", self.id, self.kind)?;
+        if !self.predecessors.is_empty() {
+            write!(out, "  predecessors: ")?;
+            for (ix, pred) in self.predecessors.iter().enumerate() {
+                if ix != 0 {
+                    write!(out, ", ")?;
+                }
+                write!(out, "{}", *pred)?;
+            }
+            writeln!(out)?;
+        }
+        for phi in self.phis.iter() {
+            phi.print(hir, out)?;
+            writeln!(out)?;
+        }
        for ix in &self.instructions {
            let instr = &hir.instructions[usize::from(*ix)];
            write!(out, "  {} {} = ", instr.id, ix)?;
@@ -38,6 +66,23 @@ impl<'a> Print<'a> for BasicBlock<'a> {
    }
 }

+impl<'a> Print<'a> for Phi<'a> {
+    fn print(&self, hir: &HIR<'a>, out: &mut impl Write) -> Result {
+        write!(out, "  ")?;
+        self.identifier.print(hir, out)?;
+        write!(out, ": phi(")?;
+        for (ix, (pred_id, id)) in self.operands.iter().enumerate() {
+            if ix != 0 {
+                write!(out, ", ")?;
+            }
+            write!(out, "{}: ", pred_id)?;
+            id.print(hir, out)?;
+        }
+        write!(out, ")")?;
+        Ok(())
+    }
+}
+
 impl<'a> Print<'a> for Instruction<'a> {
    fn print(&self, hir: &HIR<'a>, out: &mut impl Write) -> Result {
        write!(out, "  {} ", self.id)?;
@@ -1,6 +1,6 @@
 use crate::{FunctionId, ObjectId, TypeVarId};

-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub enum Type {
    Builtin(BuiltinType),
    // Phi(Box<PhiType>),
@@ -9,7 +9,7 @@ pub enum Type {
    // Prop(Box<PropType>),
 }

-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub enum BuiltinType {
    Primitive,
    Function(Option<FunctionId>),