perf(vm): add peephole pass to elide dead Load; Pop pairs ✂️

Runs to a fixed point over the IR, removing consecutive
`Constant|GetGlobal|GetLocal` followed by `Pop` when neither is a jump
target. Skipped on the REPL's resumable path so `halt_ip` stays stable
across resume-from-halt.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Author: timfennis

ndc_vm/src/chunk.rs

@@ -2,7 +2,7 @@ use crate::Value;
 use ndc_core::hash_map::HashMap;
 use ndc_lexer::Span;
 use ndc_parser::CaptureSource;
-use std::rc::Rc;
+use std::{collections::HashSet, rc::Rc};
 
 /// A signed displacement applied to the instruction pointer to perform a jump.
 ///
@@ -267,6 +267,49 @@ impl OptimizerIr {
         }
     }
 
+    pub(crate) fn peephole(&mut self) {
+        let targets: HashSet<LabelId> = self // todo replace with ahash
+            .code
+            .iter()
+            .filter_map(|(_, op, _)| match op {
+                OpCode::Jump(JumpTarget::Label(l))
+                | OpCode::JumpIfFalse(JumpTarget::Label(l))
+                | OpCode::JumpIfTrue(JumpTarget::Label(l))
+                | OpCode::IterNext(JumpTarget::Label(l)) => Some(*l),
+                _ => None,
+            })
+            .collect();
+
+        loop {
+            let mut out = Vec::with_capacity(self.code.len());
+            let mut i = 0;
+            let mut removed = 0;
+            while i < self.code.len() {
+                let elide = i + 1 < self.code.len()
+                    && matches!(
+                        self.code[i].1,
+                        OpCode::Constant(_) | OpCode::GetGlobal(_) | OpCode::GetLocal(_)
+                    )
+                    && matches!(self.code[i + 1].1, OpCode::Pop)
+                    && !targets.contains(&self.code[i].0)
+                    && !targets.contains(&self.code[i + 1].0);
+                if elide {
+                    i += 2;
+                    removed += 2;
+                } else {
+                    out.push(self.code[i].clone());
+                    i += 1;
+                }
+            }
+
+            self.code = out;
+
+            if removed == 0 {
+                break;
+            }
+        }
+    }
+
     pub(crate) fn add_constant(&mut self, value: Value) -> usize {
         self.constants.push(value);
         self.constants.len() - 1

ndc_vm/src/compiler.rs

@@ -9,29 +9,55 @@ use ndc_parser::{
 };
 use std::rc::Rc;
 
-#[derive(Default, Clone)]
+#[derive(Clone)]
 pub struct Compiler {
     ir: OptimizerIr,
     num_locals: usize,
     loop_stack: Vec<LoopContext>,
     allow_return: bool,
+    /// Whether to run the peephole pass on this chunk in `finish()`.
+    /// Disabled for the REPL's resumable compilation: peephole shrinks
+    /// instruction positions, which would invalidate the `halt_ip` captured
+    /// in the checkpoint and break resume-from-halt.
+    optimize: bool,
+}
+
+impl Default for Compiler {
+    fn default() -> Self {
+        Self {
+            ir: OptimizerIr::default(),
+            num_locals: 0,
+            loop_stack: Vec::new(),
+            allow_return: false,
+            optimize: true,
+        }
+    }
 }
 
 impl Compiler {
     pub fn compile(
         expressions: impl Iterator<Item = ExpressionLocation>,
     ) -> Result<CompiledFunction, CompileError> {
-        Ok(Self::compile_resumable(expressions)?.0)
+        let mut compiler = Self::default();
+        for expr_loc in expressions {
+            compiler.compile_expr(expr_loc)?;
+        }
+        Ok(compiler.finish()?.0)
     }
 
     /// Compile expressions and return both the finished function and a
     /// checkpoint that can be passed to `resume` to append more code later.
     /// The checkpoint is the compiler state *before* the `Halt` instruction,
     /// so `resume` can extend the bytecode without re-running old instructions.
+    ///
+    /// Peephole optimization is disabled on this path: the REPL's resume-from-
+    /// halt machinery relies on `halt_ip` matching the position of `Halt` in
+    /// the emitted chunk, and shifting instructions invalidates that.
     pub fn compile_resumable(
         expressions: impl Iterator<Item = ExpressionLocation>,
     ) -> Result<(CompiledFunction, Self), CompileError> {
         let mut compiler = Self::default();
+        compiler.optimize = false;
         for expr_loc in expressions {
             compiler.compile_expr(expr_loc)?;
         }
@@ -71,6 +97,10 @@ impl Compiler {
     fn finish(mut self) -> Result<(CompiledFunction, Self), CompileError> {
         let checkpoint = self.clone();
         self.ir.write(OpCode::Halt, Span::synthetic());
+        if self.optimize {
+            self.ir.peephole();
+        }
+
         let function = CompiledFunction {
             name: None,
             static_type: StaticType::Function {
@@ -723,6 +753,8 @@ impl Compiler {
         fn_compiler.compile_expr(body)?;
         fn_compiler.ir.write(OpCode::Return, Span::synthetic());
 
+        fn_compiler.ir.peephole();
+
         let compiled = CompiledFunction {
             name,
             static_type,

tests/compiler/tests/compiler.rs

@@ -140,12 +140,11 @@ fn test_or() {
 
 // 5;
 //
-// 0: Constant(0)      push `5`
-// 1: Pop              discard value (it's a statement)
-// 2: Halt
+// Pre-peephole: `Constant(0), Pop, Halt`. The peephole pass elides the dead
+// `Constant; Pop` pair, leaving just `Halt`.
 #[test]
 fn test_statement() {
-    assert_eq!(compile("5;"), [Constant(0), Pop, Halt]);
+    assert_eq!(compile("5;"), [Halt]);
 }
 
 // { 5 }
@@ -159,40 +158,35 @@ fn test_block_with_expression() {
 
 // { 5; }
 //
-// 0: Constant(0)      push `5`
-// 1: Pop              discard (trailing semicolon)
-// 2: Constant(1)      push `()` (block result is unit)
-// 3: Halt
+// Pre-peephole: `Constant(0), Pop, Constant(1), Halt`. The peephole pass
+// elides the dead `Constant(0); Pop`, leaving the unit result.
 #[test]
 fn test_block_with_trailing_statement() {
-    assert_eq!(compile("{ 5; }"), [Constant(0), Pop, Constant(1), Halt]);
+    assert_eq!(compile("{ 5; }"), [Constant(1), Halt]);
 }
 
 // { 5; 6 }
 //
-// 0: Constant(0)      push `5`
-// 1: Pop              discard intermediate statement
-// 2: Constant(1)      push `6` (block result)
-// 3: Halt
+// Pre-peephole: `Constant(0), Pop, Constant(1), Halt`. The peephole elides
+// the intermediate `Constant(0); Pop`, leaving just the block result.
 #[test]
 fn test_block_multiple_statements() {
-    assert_eq!(compile("{ 5; 6 }"), [Constant(0), Pop, Constant(1), Halt]);
+    assert_eq!(compile("{ 5; 6 }"), [Constant(1), Halt]);
 }
 
 // if true { 3 } else { 3; }
 //
-// true branch returns 3, false branch returns ()
+// true branch returns 3, false branch returns (). The peephole pass elides
+// the false branch's `Constant; Pop` (pushing `3` only to discard it).
 //
 // 0: Constant(0)      push `true`
 // 1: JumpIfFalse(3)   jump to false branch (index 5)
 // 2: Pop              pop condition (true path)
 // 3: Constant(1)      push `3`
-// 4: Jump(4)          jump to Halt (index 9)
+// 4: Jump(2)          jump to Halt (index 7)
 // 5: Pop              pop condition (false path)
-// 6: Constant(2)      push `3` (inner of `3;`)
-// 7: Pop              discard (trailing semicolon)
-// 8: Constant(3)      push `()` (block result)
-// 9: Halt
+// 6: Constant(3)      push `()` (block result; constant 2 still in table but unreferenced)
+// 7: Halt
 #[test]
 fn test_if_with_statement_else() {
     assert_eq!(
@@ -202,9 +196,7 @@ fn test_if_with_statement_else() {
             JumpIfFalse(JumpTarget::Offset(3)),
             Pop,
             Constant(1),
-            Jump(JumpTarget::Offset(4)),
-            Pop,
-            Constant(2),
+            Jump(JumpTarget::Offset(2)),
             Pop,
             Constant(3),
             Halt
@@ -214,34 +206,27 @@ fn test_if_with_statement_else() {
 
 // if true { 3; } else { 3; }
 //
-// Both branches return () — result is unit regardless of condition
+// Both branches return () — the peephole elides the `Constant; Pop` in each
+// arm (each branch's `3;` becomes nothing).
 //
 // 0: Constant(0)      push `true`
-// 1: JumpIfFalse(5)   jump to false branch (index 7)
+// 1: JumpIfFalse(3)   jump to false branch (index 5)
 // 2: Pop              pop condition (true path)
-// 3: Constant(1)      push `3`
-// 4: Pop              discard
-// 5: Constant(2)      push `()`
-// 6: Jump(4)          jump to Halt (index 11)
-// 7: Pop              pop condition (false path)
-// 8: Constant(3)      push `3`
-// 9: Pop              discard
-// 10: Constant(4)     push `()`
-// 11: Halt
+// 3: Constant(2)      push `()` (true-branch result; constant 1 unreferenced)
+// 4: Jump(2)          jump to Halt (index 7)
+// 5: Pop              pop condition (false path)
+// 6: Constant(4)      push `()` (false-branch result; constant 3 unreferenced)
+// 7: Halt
 #[test]
 fn test_if_with_statement_branches() {
     assert_eq!(
         compile("if true { 3; } else { 3; }"),
         [
             Constant(0),
-            JumpIfFalse(JumpTarget::Offset(5)),
-            Pop,
-            Constant(1),
+            JumpIfFalse(JumpTarget::Offset(3)),
             Pop,
             Constant(2),
-            Jump(JumpTarget::Offset(4)),
-            Pop,
-            Constant(3),
+            Jump(JumpTarget::Offset(2)),
             Pop,
             Constant(4),
             Halt
@@ -251,25 +236,24 @@ fn test_if_with_statement_branches() {
 
 // while true { 1 }
 //
+// The peephole elides the body's `Constant(1); Pop` (the loop body's value
+// is discarded). The backward jump offset shrinks accordingly.
+//
 // 0: Constant(0)      push `true`  ← loop_start
-// 1: JumpIfFalse(4)   if false, jump past body to exit Pop (index 6)
+// 1: JumpIfFalse(2)   if false, jump past body to exit Pop (index 4)
 // 2: Pop              pop condition (true path)
-// 3: Constant(1)      body: push `1`
-// 4: Pop              discard body value (loops produce no value)
-// 5: Jump(-6)         jump back to loop_start (index 0)
-// 6: Pop              pop condition (false path, loop exit)
-// 7: Halt
+// 3: Jump(-4)         jump back to loop_start (index 0)
+// 4: Pop              pop condition (false path, loop exit)
+// 5: Halt
 #[test]
 fn test_while() {
     assert_eq!(
         compile("while true { 1 }"),
         [
             Constant(0),
-            JumpIfFalse(JumpTarget::Offset(4)),
-            Pop,
-            Constant(1),
+            JumpIfFalse(JumpTarget::Offset(2)),
             Pop,
-            Jump(JumpTarget::Offset(-6)),
+            Jump(JumpTarget::Offset(-4)),
             Pop,
             Halt
         ]
@@ -294,30 +278,21 @@ fn test_declaration() {
 // let a = 1;
 // a = 5;
 //
-// Declaration stores 1 into pre-allocated slot 0.
-// Assignment pushes new value, SetLocal overwrites,
-// push unit as the expression result, Pop discards it.
+// Declaration stores 1 into pre-allocated slot 0. Assignment pushes new
+// value, SetLocal overwrites. The peephole elides the trailing
+// `Constant; Pop` (the assignment expression's unit result, discarded as a
+// statement).
 //
 // 0: Constant(0)   push `1`
 // 1: SetLocal(0)   store in slot 0 (declaration)
 // 2: Constant(1)   push `5`
 // 3: SetLocal(0)   overwrite slot 0 (assignment)
-// 4: Constant(2)   push `()` (assignment result)
-// 5: Pop           discard (statement)
-// 6: Halt
+// 4: Halt
 #[test]
 fn test_assignment() {
     assert_eq!(
         compile_with_analysis("let a = 1;\na = 5;"),
-        [
-            Constant(0),
-            SetLocal(0),
-            Constant(1),
-            SetLocal(0),
-            Constant(2),
-            Pop,
-            Halt
-        ]
+        [Constant(0), SetLocal(0), Constant(1), SetLocal(0), Halt]
     );
 }