chore(ssa refactor): Fix recursive printing of blocks

crates/noirc_evaluator/src/ssa_refactor/ir/cfg.rs

@@ -133,29 +133,27 @@ mod tests {
         // Build function of form
         // fn func {
         // block0(cond: u1):
-        //     jmpif cond(), then: block2, else: block1
+        //     jmpif cond, then: block2, else: block1
         // block1():
-        //     jmpif cond(), then: block1, else: block2
+        //     jmpif cond, then: block1, else: block2
         // block2():
-        //     return
+        //     return ()
         // }
         let mut func = Function::new("func".into());
         let block0_id = func.entry_block();
         let cond = func.dfg.add_block_parameter(block0_id, Type::unsigned(1));
-        let block1_id = func.dfg.new_block();
-        let block2_id = func.dfg.new_block();
+        let block1_id = func.dfg.make_block();
+        let block2_id = func.dfg.make_block();
 
         func.dfg[block0_id].set_terminator(TerminatorInstruction::JmpIf {
             condition: cond,
             then_destination: block2_id,
             else_destination: block1_id,
-            arguments: vec![],
         });
         func.dfg[block1_id].set_terminator(TerminatorInstruction::JmpIf {
             condition: cond,
             then_destination: block1_id,
             else_destination: block2_id,
-            arguments: vec![],
         });
         func.dfg[block2_id].set_terminator(TerminatorInstruction::Return { return_values: vec![] });
 
@@ -192,15 +190,15 @@ mod tests {
         // Modify function to form:
         // fn func {
         // block0(cond: u1):
-        //     jmpif cond(), then: block1, else: ret_block
+        //     jmpif cond, then: block1, else: ret_block
         // block1():
-        //     jmpif cond(), then: block1, else: block2
+        //     jmpif cond, then: block1, else: block2
         // block2():
-        //     jmp ret_block
+        //     jmp ret_block()
         // ret_block():
-        //     return
+        //     return ()
         // }
-        let ret_block_id = func.dfg.new_block();
+        let ret_block_id = func.dfg.make_block();
         func.dfg[ret_block_id]
             .set_terminator(TerminatorInstruction::Return { return_values: vec![] });
         func.dfg[block2_id].set_terminator(TerminatorInstruction::Jmp {
@@ -211,7 +209,6 @@ mod tests {
             condition: cond,
             then_destination: block1_id,
             else_destination: ret_block_id,
-            arguments: vec![],
         });
 
         // Recompute new and changed blocks

crates/noirc_evaluator/src/ssa_refactor/ir/constant.rs

@@ -16,8 +16,8 @@ impl NumericConstant {
         Self(value)
     }
 
-    pub(crate) fn value(&self) -> &FieldElement {
-        &self.0
+    pub(crate) fn value(&self) -> FieldElement {
+        self.0
     }
 }
 

crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs

@@ -2,7 +2,7 @@ use super::{
     basic_block::{BasicBlock, BasicBlockId},
     constant::{NumericConstant, NumericConstantId},
     function::Signature,
-    instruction::{Instruction, InstructionId, InstructionResultType},
+    instruction::{Instruction, InstructionId, InstructionResultType, TerminatorInstruction},
     map::{DenseMap, Id, SecondaryMap, TwoWayMap},
     types::Type,
     value::{Value, ValueId},
@@ -75,14 +75,14 @@ impl DataFlowGraph {
     /// Creates a new basic block with no parameters.
     /// After being created, the block is unreachable in the current function
     /// until another block is made to jump to it.
-    pub(crate) fn new_block(&mut self) -> BasicBlockId {
+    pub(crate) fn make_block(&mut self) -> BasicBlockId {
         self.blocks.insert(BasicBlock::new(Vec::new()))
     }
 
     /// Creates a new basic block with the given parameters.
     /// After being created, the block is unreachable in the current function
     /// until another block is made to jump to it.
-    pub(crate) fn new_block_with_parameters(
+    pub(crate) fn make_block_with_parameters(
         &mut self,
         parameter_types: impl Iterator<Item = Type>,
     ) -> BasicBlockId {
@@ -230,6 +230,33 @@ impl DataFlowGraph {
     ) {
         self.blocks[block].insert_instruction(instruction);
     }
+
+    /// Returns the field element represented by this value if it is a numeric constant.
+    /// Returns None if the given value is not a numeric constant.
+    pub(crate) fn get_numeric_constant(&self, value: Id<Value>) -> Option<FieldElement> {
+        self.get_numeric_constant_with_type(value).map(|(value, _typ)| value)
+    }
+
+    /// Returns the field element and type represented by this value if it is a numeric constant.
+    /// Returns None if the given value is not a numeric constant.
+    pub(crate) fn get_numeric_constant_with_type(
+        &self,
+        value: Id<Value>,
+    ) -> Option<(FieldElement, Type)> {
+        match self.values[value] {
+            Value::NumericConstant { constant, typ } => Some((self[constant].value(), typ)),
+            _ => None,
+        }
+    }
+
+    /// Sets the terminator instruction for the given basic block
+    pub(crate) fn set_block_terminator(
+        &mut self,
+        block: BasicBlockId,
+        terminator: TerminatorInstruction,
+    ) {
+        self.blocks[block].set_terminator(terminator);
+    }
 }
 
 impl std::ops::Index<InstructionId> for DataFlowGraph {

crates/noirc_evaluator/src/ssa_refactor/ir/function.rs

@@ -32,7 +32,7 @@ impl Function {
     /// Note that any parameters to the function must be manually added later.
     pub(crate) fn new(name: String) -> Self {
         let mut dfg = DataFlowGraph::default();
-        let entry_block = dfg.new_block();
+        let entry_block = dfg.make_block();
         Self { name, source_locations: SecondaryMap::new(), entry_block, dfg }
     }
 

crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs

@@ -150,14 +150,9 @@ pub(crate) enum TerminatorInstruction {
     ///
     /// Jump If
     ///
-    /// If the condition is true: jump to the specified `then_destination` with `arguments`.
-    /// Otherwise, jump to the specified `else_destination` with `arguments`.
-    JmpIf {
-        condition: ValueId,
-        then_destination: BasicBlockId,
-        else_destination: BasicBlockId,
-        arguments: Vec<ValueId>,
-    },
+    /// If the condition is true: jump to the specified `then_destination`.
+    /// Otherwise, jump to the specified `else_destination`.
+    JmpIf { condition: ValueId, then_destination: BasicBlockId, else_destination: BasicBlockId },
 
     /// Unconditional Jump
     ///

crates/noirc_evaluator/src/ssa_refactor/ir/map.rs

@@ -69,9 +69,21 @@ impl<T> std::fmt::Debug for Id<T> {
     }
 }
 
-impl<T> std::fmt::Display for Id<T> {
+impl std::fmt::Display for Id<super::basic_block::BasicBlock> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "${}", self.index)
+        write!(f, "b{}", self.index)
+    }
+}
+
+impl std::fmt::Display for Id<super::value::Value> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "v{}", self.index)
+    }
+}
+
+impl std::fmt::Display for Id<super::function::Function> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "f{}", self.index)
     }
 }
 

crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs

@@ -1,5 +1,8 @@
 //! This file is for pretty-printing the SSA IR in a human-readable form for debugging.
-use std::fmt::{Formatter, Result};
+use std::{
+    collections::HashSet,
+    fmt::{Formatter, Result},
+};
 
 use iter_extended::vecmap;
 
@@ -12,19 +15,26 @@ use super::{
 
 pub(crate) fn display_function(function: &Function, f: &mut Formatter) -> Result {
     writeln!(f, "fn {} {{", function.name)?;
-    display_block_with_successors(function, function.entry_block, f)?;
+    display_block_with_successors(function, function.entry_block, &mut HashSet::new(), f)?;
     write!(f, "}}")
 }
 
+/// Displays a block followed by all of its successors recursively.
+/// This uses a HashSet to keep track of the visited blocks. Otherwise,
+/// there would be infinite recursion for any loops in the IR.
 pub(crate) fn display_block_with_successors(
     function: &Function,
     block_id: BasicBlockId,
+    visited: &mut HashSet<BasicBlockId>,
     f: &mut Formatter,
 ) -> Result {
     display_block(function, block_id, f)?;
+    visited.insert(block_id);
 
     for successor in function.dfg[block_id].successors() {
-        display_block(function, successor, f)?;
+        if !visited.contains(&successor) {
+            display_block_with_successors(function, successor, visited, f)?;
+        }
     }
     Ok(())
 }
@@ -36,42 +46,46 @@ pub(crate) fn display_block(
 ) -> Result {
     let block = &function.dfg[block_id];
 
-    writeln!(f, "{}({}):", block_id, value_list(block.parameters()))?;
+    writeln!(f, "  {}({}):", block_id, value_list(function, block.parameters()))?;
 
     for instruction in block.instructions() {
         display_instruction(function, *instruction, f)?;
     }
 
-    display_terminator(block.terminator(), f)
+    display_terminator(function, block.terminator(), f)
+}
+
+/// Specialize displaying value ids so that if they refer to constants we
+/// print the constant directly
+fn value(function: &Function, id: ValueId) -> String {
+    match function.dfg.get_numeric_constant_with_type(id) {
+        Some((value, typ)) => format!("{} {}", value, typ),
+        None => id.to_string(),
+    }
 }
 
-fn value_list(values: &[ValueId]) -> String {
-    vecmap(values, ToString::to_string).join(", ")
+fn value_list(function: &Function, values: &[ValueId]) -> String {
+    vecmap(values, |id| value(function, *id)).join(", ")
 }
 
 pub(crate) fn display_terminator(
+    function: &Function,
     terminator: Option<&TerminatorInstruction>,
     f: &mut Formatter,
 ) -> Result {
     match terminator {
         Some(TerminatorInstruction::Jmp { destination, arguments }) => {
-            writeln!(f, "    jmp {}({})", destination, value_list(arguments))
+            writeln!(f, "    jmp {}({})", destination, value_list(function, arguments))
         }
-        Some(TerminatorInstruction::JmpIf {
-            condition,
-            arguments,
-            then_destination,
-            else_destination,
-        }) => {
-            let args = value_list(arguments);
+        Some(TerminatorInstruction::JmpIf { condition, then_destination, else_destination }) => {
             writeln!(
                 f,
-                "    jmpif {}({}) then: {}, else: {}",
-                condition, args, then_destination, else_destination
+                "    jmpif {} then: {}, else: {}",
+                condition, then_destination, else_destination
             )
         }
         Some(TerminatorInstruction::Return { return_values }) => {
-            writeln!(f, "    return {}", value_list(return_values))
+            writeln!(f, "    return {}", value_list(function, return_values))
         }
         None => writeln!(f, "    (no terminator instruction)"),
     }
@@ -87,29 +101,34 @@ pub(crate) fn display_instruction(
 
     let results = function.dfg.instruction_results(instruction);
     if !results.is_empty() {
-        write!(f, "{} = ", value_list(results))?;
+        write!(f, "{} = ", value_list(function, results))?;
     }
 
+    let show = |id| value(function, id);
+
     match &function.dfg[instruction] {
         Instruction::Binary(binary) => {
-            writeln!(f, "{} {}, {}", binary.operator, binary.lhs, binary.rhs)
+            writeln!(f, "{} {}, {}", binary.operator, show(binary.lhs), show(binary.rhs))
         }
-        Instruction::Cast(value, typ) => writeln!(f, "cast {value} as {typ}"),
-        Instruction::Not(value) => writeln!(f, "not {value}"),
+        Instruction::Cast(lhs, typ) => writeln!(f, "cast {} as {typ}", show(*lhs)),
+        Instruction::Not(rhs) => writeln!(f, "not {}", show(*rhs)),
         Instruction::Truncate { value, bit_size, max_bit_size } => {
-            writeln!(f, "truncate {value} to {bit_size} bits, max_bit_size: {max_bit_size}")
+            let value = show(*value);
+            writeln!(f, "truncate {value} to {bit_size} bits, max_bit_size: {max_bit_size}",)
         }
         Instruction::Constrain(value) => {
-            writeln!(f, "constrain {value}")
+            writeln!(f, "constrain {}", show(*value))
         }
         Instruction::Call { func, arguments } => {
-            writeln!(f, "call {func}({})", value_list(arguments))
+            writeln!(f, "call {func}({})", value_list(function, arguments))
         }
         Instruction::Intrinsic { func, arguments } => {
-            writeln!(f, "intrinsic {func}({})", value_list(arguments))
+            writeln!(f, "intrinsic {func}({})", value_list(function, arguments))
         }
         Instruction::Allocate { size } => writeln!(f, "alloc {size} fields"),
-        Instruction::Load { address } => writeln!(f, "load {address}"),
-        Instruction::Store { address, value } => writeln!(f, "store {value} at {address}"),
+        Instruction::Load { address } => writeln!(f, "load {}", show(*address)),
+        Instruction::Store { address, value } => {
+            writeln!(f, "store {} at {}", show(*address), show(*value))
+        }
     }
 }