diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index 8acce876d90..9b713eee06e 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -234,8 +234,17 @@ impl DataFlowGraph { /// 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) -> Option { + 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, + ) -> Option<(FieldElement, Type)> { match self.values[value] { - Value::NumericConstant { constant, .. } => Some(self[constant].value()), + Value::NumericConstant { constant, typ } => Some((self[constant].value(), typ)), _ => None, } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs index 5937b374726..24b30241293 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs @@ -69,9 +69,21 @@ impl std::fmt::Debug for Id { } } -impl std::fmt::Display for Id { +impl std::fmt::Display for Id { 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 { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "v{}", self.index) + } +} + +impl std::fmt::Display for Id { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "f{}", self.index) } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs index a711482e08c..57c573c7bd4 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs +++ b/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, 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,26 +46,36 @@ 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, then_destination, else_destination }) => { writeln!( @@ -65,7 +85,7 @@ pub(crate) fn display_terminator( ) } 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)"), } @@ -81,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)) + } } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs index 30855b8fdc8..48175ebb52b 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs @@ -164,6 +164,11 @@ impl<'a> FunctionContext<'a> { } address } + + pub(super) fn define(&mut self, id: LocalId, value: Values) { + let existing = self.definitions.insert(id, value); + assert!(existing.is_none(), "Variable {id:?} was defined twice in ssa-gen pass"); + } } /// True if the given operator cannot be encoded directly and needs diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs index 04fb88d76d0..f8faf8eeeb4 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs @@ -166,8 +166,34 @@ impl<'a> FunctionContext<'a> { self.builder.insert_cast(lhs, typ).into() } - fn codegen_for(&mut self, _for_expr: &ast::For) -> Values { - todo!() + fn codegen_for(&mut self, for_expr: &ast::For) -> Values { + let loop_entry = self.builder.insert_block(); + let loop_body = self.builder.insert_block(); + let loop_end = self.builder.insert_block(); + + // this is the 'i' in `for i in start .. end { block }` + let loop_index = self.builder.add_block_parameter(loop_entry, Type::field()); + + let start_index = self.codegen_non_tuple_expression(&for_expr.start_range); + let end_index = self.codegen_non_tuple_expression(&for_expr.end_range); + + self.builder.terminate_with_jmp(loop_entry, vec![start_index]); + + // Compile the loop entry block + self.builder.switch_to_block(loop_entry); + let jump_condition = self.builder.insert_binary(loop_index, BinaryOp::Lt, end_index); + self.builder.terminate_with_jmpif(jump_condition, loop_body, loop_end); + + // Compile the loop body + self.builder.switch_to_block(loop_body); + self.define(for_expr.index_variable, loop_index.into()); + self.codegen_expression(&for_expr.block); + let new_loop_index = self.make_offset(loop_index, 1); + self.builder.terminate_with_jmp(loop_entry, vec![new_loop_index]); + + // Finish by switching back to the end of the loop + self.builder.switch_to_block(loop_end); + self.unit_value() } fn codegen_if(&mut self, if_expr: &ast::If) -> Values { diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 15ed0d74222..575a9403ea8 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -1238,46 +1238,46 @@ mod test { ); } - #[test] - fn parse_assert() { - parse_with(assertion(expression()), "assert(x == y)").unwrap(); - - // Currently we disallow constrain statements where the outer infix operator - // produces a value. This would require an implicit `==` which - // may not be intuitive to the user. - // - // If this is deemed useful, one would either apply a transformation - // or interpret it with an `==` in the evaluator - let disallowed_operators = vec![ - BinaryOpKind::And, - BinaryOpKind::Subtract, - BinaryOpKind::Divide, - BinaryOpKind::Multiply, - BinaryOpKind::Or, - ]; - - for operator in disallowed_operators { - let src = format!("assert(x {} y);", operator.as_string()); - parse_with(assertion(expression()), &src).unwrap_err(); - } - - // These are general cases which should always work. - // - // The first case is the most noteworthy. It contains two `==` - // The first (inner) `==` is a predicate which returns 0/1 - // The outer layer is an infix `==` which is - // associated with the Constrain statement - parse_all( - assertion(expression()), - vec![ - "assert(((x + y) == k) + z == y)", - "assert((x + !y) == y)", - "assert((x ^ y) == y)", - "assert((x ^ y) == (y + m))", - "assert(x + x ^ x == y | m)", - ], - ); - } + #[test] + fn parse_assert() { + parse_with(assertion(expression()), "assert(x == y)").unwrap(); + + // Currently we disallow constrain statements where the outer infix operator + // produces a value. This would require an implicit `==` which + // may not be intuitive to the user. + // + // If this is deemed useful, one would either apply a transformation + // or interpret it with an `==` in the evaluator + let disallowed_operators = vec![ + BinaryOpKind::And, + BinaryOpKind::Subtract, + BinaryOpKind::Divide, + BinaryOpKind::Multiply, + BinaryOpKind::Or, + ]; + + for operator in disallowed_operators { + let src = format!("assert(x {} y);", operator.as_string()); + parse_with(assertion(expression()), &src).unwrap_err(); + } + + // These are general cases which should always work. + // + // The first case is the most noteworthy. It contains two `==` + // The first (inner) `==` is a predicate which returns 0/1 + // The outer layer is an infix `==` which is + // associated with the Constrain statement + parse_all( + assertion(expression()), + vec![ + "assert(((x + y) == k) + z == y)", + "assert((x + !y) == y)", + "assert((x ^ y) == y)", + "assert((x ^ y) == (y + m))", + "assert(x + x ^ x == y | m)", + ], + ); + } #[test] fn parse_let() { @@ -1322,7 +1322,7 @@ mod test { "fn f(f: pub Field, y : Field, z : comptime Field) -> u8 { x + a }", "fn func_name(f: Field, y : pub Field, z : pub [u8;5],) {}", "fn func_name(x: [Field], y : [Field;2],y : pub [Field;2], z : pub [u8;5]) {}", - "fn main(x: pub u8, y: pub u8) -> distinct pub [u8; 2] { [x, y] }" + "fn main(x: pub u8, y: pub u8) -> distinct pub [u8; 2] { [x, y] }", ], );