From b749264bfbb9721840806e042d409d828e7c68d2 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 5 Jul 2023 13:37:52 +0000 Subject: [PATCH 01/23] generates acir debug information --- crates/noirc_evaluator/src/ssa_refactor.rs | 2 +- .../acir_gen/acir_ir/acir_variable.rs | 5 + .../acir_gen/acir_ir/generated_acir.rs | 13 ++ .../src/ssa_refactor/acir_gen/mod.rs | 2 +- .../src/ssa_refactor/ir/dfg.rs | 18 +++ .../src/ssa_refactor/ir/function_inserter.rs | 21 +++- .../src/ssa_refactor/opt/constant_folding.rs | 17 ++- .../src/ssa_refactor/opt/flatten_cfg.rs | 84 +++++++++---- .../src/ssa_refactor/opt/inlining.rs | 4 +- .../src/ssa_refactor/ssa_builder/mod.rs | 119 ++++++++++++++++-- .../src/ssa_refactor/ssa_gen/context.rs | 61 ++++++--- .../src/ssa_refactor/ssa_gen/mod.rs | 22 ++-- 12 files changed, 291 insertions(+), 77 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor.rs b/crates/noirc_evaluator/src/ssa_refactor.rs index 4f11e147809..869654768c4 100644 --- a/crates/noirc_evaluator/src/ssa_refactor.rs +++ b/crates/noirc_evaluator/src/ssa_refactor.rs @@ -67,7 +67,7 @@ pub fn experimental_create_circuit( show_output: bool, ) -> Result<(Circuit, Abi), RuntimeError> { let func_sig = program.main_function_signature.clone(); - let GeneratedAcir { current_witness_index, opcodes, return_witnesses } = + let GeneratedAcir { current_witness_index, opcodes, return_witnesses, .. } = optimize_into_acir(program, show_output, enable_logging); let abi = gen_abi(func_sig, return_witnesses.clone()); diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs index 6e3a3c0b73d..0303b60b4b6 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs @@ -17,6 +17,7 @@ use acvm::{ FieldElement, }; use iter_extended::vecmap; +use noirc_errors::Location; use std::collections::HashMap; use std::{borrow::Cow, hash::Hash}; @@ -123,6 +124,10 @@ impl AcirContext { self.add_data(var_data) } + pub(crate) fn set_location(&mut self, location: Option) { + self.acir_ir.current_location = location; + } + /// True if the given AcirVar refers to a constant one value pub(crate) fn is_constant_one(&self, var: &AcirVar) -> bool { match self.vars[var] { diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs index 3ce99d5a01c..3146f9f29c0 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs @@ -1,5 +1,7 @@ //! `GeneratedAcir` is constructed as part of the `acir_gen` pass to accumulate all of the ACIR //! program as it is being converted from SSA form. +use std::collections::HashMap; + use crate::brillig::brillig_gen::brillig_directive; use super::errors::AcirGenError; @@ -18,6 +20,7 @@ use acvm::{ FieldElement, }; use iter_extended::{try_vecmap, vecmap}; +use noirc_errors::Location; use num_bigint::BigUint; #[derive(Debug, Default)] @@ -36,6 +39,13 @@ pub(crate) struct GeneratedAcir { /// Note: This may contain repeated indices, which is necessary for later mapping into the /// abi's return type. pub(crate) return_witnesses: Vec, + + /// Correspondance between an opcode index (in opcodes) and the source code location which generated it + locations: HashMap, + + /// Source code location of the current instruction being processed + /// None if we do not know the location + pub(crate) current_location: Option, } impl GeneratedAcir { @@ -47,6 +57,9 @@ impl GeneratedAcir { /// Adds a new opcode into ACIR. fn push_opcode(&mut self, opcode: AcirOpcode) { self.opcodes.push(opcode); + if let Some(location) = self.current_location { + self.locations.insert(self.opcodes.len() - 1, location); + } } /// Updates the witness index counter and returns diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index 307bcce5a35..c252fc16044 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -231,7 +231,7 @@ impl Context { allow_log_ops: bool, ) { let instruction = &dfg[instruction_id]; - + self.acir_context.set_location(dfg.get_location(&instruction_id)); match instruction { Instruction::Binary(binary) => { let result_acir_var = self diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index 393b85fdd2f..32ebd286b04 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -15,6 +15,7 @@ use super::{ use acvm::FieldElement; use iter_extended::vecmap; +use noirc_errors::Location; /// The DataFlowGraph contains most of the actual data in a function including /// its blocks, instructions, and values. This struct is largely responsible for @@ -69,6 +70,8 @@ pub(crate) struct DataFlowGraph { /// for that of another. This information is purely used for printing the SSA, and has no /// material effect on the SSA itself. replaced_value_ids: HashMap, + + locations: HashMap, } impl DataFlowGraph { @@ -137,6 +140,7 @@ impl DataFlowGraph { instruction: Instruction, block: BasicBlockId, ctrl_typevars: Option>, + location: Option, ) -> InsertInstructionResult { use InsertInstructionResult::*; match instruction.simplify(self, block) { @@ -145,6 +149,9 @@ impl DataFlowGraph { SimplifyResult::None => { let id = self.make_instruction(instruction, ctrl_typevars); self.blocks[block].insert_instruction(id); + if let Some(location) = location { + self.locations.insert(id, location); + } InsertInstructionResult::Results(self.instruction_results(id)) } } @@ -362,6 +369,17 @@ impl DataFlowGraph { destination.instructions_mut().append(&mut instructions); destination.set_terminator(terminator); } + + pub(crate) fn get_location(&self, id: &InstructionId) -> Option { + self.locations.get(id).cloned() + } + + pub(crate) fn get_value_location(&self, id: &ValueId) -> Option { + match &self.values[*id] { + Value::Instruction { instruction, .. } => self.get_location(instruction), + _ => None, + } + } } impl std::ops::Index for DataFlowGraph { diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs index d85ad46166f..52b5f1621e0 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use iter_extended::vecmap; +use noirc_errors::Location; use super::{ basic_block::BasicBlockId, @@ -55,13 +56,16 @@ impl<'f> FunctionInserter<'f> { self.values.insert(key, value); } - pub(crate) fn map_instruction(&mut self, id: InstructionId) -> Instruction { - self.function.dfg[id].clone().map_values(|id| self.resolve(id)) + pub(crate) fn map_instruction(&mut self, id: InstructionId) -> (Instruction, Option) { + ( + self.function.dfg[id].clone().map_values(|id| self.resolve(id)), + self.function.dfg.get_location(&id), + ) } pub(crate) fn push_instruction(&mut self, id: InstructionId, block: BasicBlockId) { - let instruction = self.map_instruction(id); - self.push_instruction_value(instruction, id, block); + let (instruction, location) = self.map_instruction(id); + self.push_instruction_value(instruction, id, block, location); } pub(crate) fn push_instruction_value( @@ -69,6 +73,7 @@ impl<'f> FunctionInserter<'f> { instruction: Instruction, id: InstructionId, block: BasicBlockId, + location: Option, ) -> InsertInstructionResult { let results = self.function.dfg.instruction_results(id); let results = vecmap(results, |id| self.function.dfg.resolve(*id)); @@ -77,8 +82,12 @@ impl<'f> FunctionInserter<'f> { .requires_ctrl_typevars() .then(|| vecmap(&results, |result| self.function.dfg.type_of_value(*result))); - let new_results = - self.function.dfg.insert_instruction_and_results(instruction, block, ctrl_typevars); + let new_results = self.function.dfg.insert_instruction_and_results( + instruction, + block, + ctrl_typevars, + location, + ); Self::insert_new_instruction_results(&mut self.values, &results, &new_results); new_results diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs index 958130b73f9..ae1400fbf5e 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs @@ -71,12 +71,17 @@ impl Context { .requires_ctrl_typevars() .then(|| vecmap(&old_results, |result| function.dfg.type_of_value(*result))); - let new_results = - match function.dfg.insert_instruction_and_results(instruction, block, ctrl_typevars) { - InsertInstructionResult::SimplifiedTo(new_result) => vec![new_result], - InsertInstructionResult::Results(new_results) => new_results.to_vec(), - InsertInstructionResult::InstructionRemoved => vec![], - }; + let location = function.dfg.get_location(&id); + let new_results = match function.dfg.insert_instruction_and_results( + instruction, + block, + ctrl_typevars, + location, + ) { + InsertInstructionResult::SimplifiedTo(new_result) => vec![new_result], + InsertInstructionResult::Results(new_results) => new_results.to_vec(), + InsertInstructionResult::InstructionRemoved => vec![], + }; assert_eq!(old_results.len(), new_results.len()); for (old_result, new_result) in old_results.iter().zip(new_results) { function.dfg.set_value_from_id(*old_result, new_result); diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs index 9c8b3c6ee91..7fedbf282ef 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs @@ -138,6 +138,7 @@ use std::{ use acvm::FieldElement; use iter_extended::vecmap; +use noirc_errors::Location; use crate::ssa_refactor::{ ir::{ @@ -263,7 +264,8 @@ impl<'f> Context<'f> { let then_branch = self.inline_branch(block, then_block, old_condition, then_condition, one); - let else_condition = self.insert_instruction(Instruction::Not(then_condition)); + let else_condition = + self.insert_instruction(Instruction::Not(then_condition), None); let zero = FieldElement::zero(); // Make sure the else branch sees the previous values of each store @@ -317,7 +319,7 @@ impl<'f> Context<'f> { if let Some((_, previous_condition)) = self.conditions.last() { let and = Instruction::binary(BinaryOp::And, *previous_condition, condition); - let new_condition = self.insert_instruction(and); + let new_condition = self.insert_instruction(and, None); self.conditions.push((end_block, new_condition)); } else { self.conditions.push((end_block, condition)); @@ -327,9 +329,17 @@ impl<'f> Context<'f> { /// Insert a new instruction into the function's entry block. /// Unlike push_instruction, this function will not map any ValueIds. /// within the given instruction, nor will it modify self.values in any way. - fn insert_instruction(&mut self, instruction: Instruction) -> ValueId { + fn insert_instruction( + &mut self, + instruction: Instruction, + location: Option, + ) -> ValueId { let block = self.inserter.function.entry_block(); - self.inserter.function.dfg.insert_instruction_and_results(instruction, block, None).first() + self.inserter + .function + .dfg + .insert_instruction_and_results(instruction, block, None, location) + .first() } /// Inserts a new instruction into the function's entry block, using the given @@ -342,7 +352,12 @@ impl<'f> Context<'f> { ctrl_typevars: Option>, ) -> InsertInstructionResult { let block = self.inserter.function.entry_block(); - self.inserter.function.dfg.insert_instruction_and_results(instruction, block, ctrl_typevars) + self.inserter.function.dfg.insert_instruction_and_results( + instruction, + block, + ctrl_typevars, + None, + ) } /// Checks the branch condition on the top of the stack and uses it to build and insert an @@ -451,20 +466,38 @@ impl<'f> Context<'f> { "Expected values merged to be of the same type but found {then_type} and {else_type}" ); + let then_location = self.inserter.function.dfg.get_value_location(&then_value); + let else_location = self.inserter.function.dfg.get_value_location(&else_value); + let merge_location = then_location.or(else_location); + // We must cast the bool conditions to the actual numeric type used by each value. - let then_condition = self.insert_instruction(Instruction::Cast(then_condition, then_type)); - let else_condition = self.insert_instruction(Instruction::Cast(else_condition, else_type)); + let then_condition = + self.insert_instruction(Instruction::Cast(then_condition, then_type), then_location); + let else_condition = + self.insert_instruction(Instruction::Cast(else_condition, else_type), else_location); let mul = Instruction::binary(BinaryOp::Mul, then_condition, then_value); - let then_value = - self.inserter.function.dfg.insert_instruction_and_results(mul, block, None).first(); + let then_value = self + .inserter + .function + .dfg + .insert_instruction_and_results(mul, block, None, merge_location) + .first(); let mul = Instruction::binary(BinaryOp::Mul, else_condition, else_value); - let else_value = - self.inserter.function.dfg.insert_instruction_and_results(mul, block, None).first(); + let else_value = self + .inserter + .function + .dfg + .insert_instruction_and_results(mul, block, None, merge_location) + .first(); let add = Instruction::binary(BinaryOp::Add, then_value, else_value); - self.inserter.function.dfg.insert_instruction_and_results(add, block, None).first() + self.inserter + .function + .dfg + .insert_instruction_and_results(add, block, None, merge_location) + .first() } /// Inline one branch of a jmpif instruction. @@ -649,12 +682,12 @@ impl<'f> Context<'f> { /// with a different InstructionId from the original. The results of the given instruction /// will also be mapped to the results of the new instruction. fn push_instruction(&mut self, id: InstructionId) { - let instruction = self.inserter.map_instruction(id); - let instruction = self.handle_instruction_side_effects(instruction); + let (instruction, location) = self.inserter.map_instruction(id); + let instruction = self.handle_instruction_side_effects(instruction, location); let is_allocate = matches!(instruction, Instruction::Allocate); let entry = self.inserter.function.entry_block(); - let results = self.inserter.push_instruction_value(instruction, id, entry); + let results = self.inserter.push_instruction_value(instruction, id, entry, location); // Remember an allocate was created local to this branch so that we do not try to merge store // values across branches for it later. @@ -665,17 +698,22 @@ impl<'f> Context<'f> { /// If we are currently in a branch, we need to modify constrain instructions /// to multiply them by the branch's condition (see optimization #1 in the module comment). - fn handle_instruction_side_effects(&mut self, instruction: Instruction) -> Instruction { + fn handle_instruction_side_effects( + &mut self, + instruction: Instruction, + location: Option, + ) -> Instruction { if let Some((_, condition)) = self.conditions.last().copied() { match instruction { Instruction::Constrain(value) => { - let mul = self.insert_instruction(Instruction::binary( - BinaryOp::Mul, - value, - condition, - )); - let eq = - self.insert_instruction(Instruction::binary(BinaryOp::Eq, mul, condition)); + let mul = self.insert_instruction( + Instruction::binary(BinaryOp::Mul, value, condition), + location, + ); + let eq = self.insert_instruction( + Instruction::binary(BinaryOp::Eq, mul, condition), + location, + ); Instruction::Constrain(eq) } Instruction::Store { address, value } => { diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs index a9077018df5..6de190a56a1 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs @@ -379,6 +379,7 @@ impl<'function> PerFunctionContext<'function> { /// function being inlined into. fn push_instruction(&mut self, id: InstructionId) { let instruction = self.source_function.dfg[id].map_values(|id| self.translate_value(id)); + let location = self.source_function.dfg.get_location(&id); let results = self.source_function.dfg.instruction_results(id); let results = vecmap(results, |id| self.source_function.dfg.resolve(*id)); @@ -386,7 +387,8 @@ impl<'function> PerFunctionContext<'function> { .requires_ctrl_typevars() .then(|| vecmap(&results, |result| self.source_function.dfg.type_of_value(*result))); - let new_results = self.context.builder.insert_instruction(instruction, ctrl_typevars); + let new_results = + self.context.builder.insert_instruction(instruction, ctrl_typevars, location); Self::insert_new_instruction_results(&mut self.values, &results, new_results); } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs index 6609252f042..f209b35d20c 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs @@ -1,6 +1,7 @@ use std::{borrow::Cow, rc::Rc}; use acvm::FieldElement; +use noirc_errors::Location; use crate::ssa_refactor::ir::{ basic_block::BasicBlockId, @@ -143,11 +144,13 @@ impl FunctionBuilder { &mut self, instruction: Instruction, ctrl_typevars: Option>, + location: Option, ) -> InsertInstructionResult { self.current_function.dfg.insert_instruction_and_results( instruction, self.current_block, ctrl_typevars, + location, ) } @@ -167,7 +170,7 @@ impl FunctionBuilder { /// given amount of field elements. Returns the result of the allocate instruction, /// which is always a Reference to the allocated data. pub(crate) fn insert_allocate(&mut self) -> ValueId { - self.insert_instruction(Instruction::Allocate, None).first() + self.insert_instruction(Instruction::Allocate, None, None).first() } /// Insert a Load instruction at the end of the current block, loading from the given offset @@ -178,14 +181,41 @@ impl FunctionBuilder { /// an array will have an offset of 3. /// Returns the element that was loaded. pub(crate) fn insert_load(&mut self, address: ValueId, type_to_load: Type) -> ValueId { - self.insert_instruction(Instruction::Load { address }, Some(vec![type_to_load])).first() + self.insert_instruction(Instruction::Load { address }, Some(vec![type_to_load]), None) + .first() } + // Same as insert_load(), but additionally provides the source location of the load + pub(crate) fn insert_load_with_source_location( + &mut self, + address: ValueId, + type_to_load: Type, + location: Location, + ) -> ValueId { + self.insert_instruction( + Instruction::Load { address }, + Some(vec![type_to_load]), + Some(location), + ) + .first() + } /// Insert a Store instruction at the end of the current block, storing the given element /// at the given address. Expects that the address points somewhere /// within a previous Allocate instruction. pub(crate) fn insert_store(&mut self, address: ValueId, value: ValueId) { - self.insert_instruction(Instruction::Store { address, value }, None); + self.insert_instruction(Instruction::Store { address, value }, None, None); + } + + /// Insert a Store instruction at the end of the current block, storing the given element + /// at the given address. Expects that the address points somewhere + /// within a previous Allocate instruction. + pub(crate) fn insert_store_with_source_location( + &mut self, + address: ValueId, + value: ValueId, + location: Location, + ) { + self.insert_instruction(Instruction::Store { address, value }, None, Some(location)); } /// Insert a binary instruction at the end of the current block. @@ -197,19 +227,32 @@ impl FunctionBuilder { rhs: ValueId, ) -> ValueId { let instruction = Instruction::Binary(Binary { lhs, rhs, operator }); - self.insert_instruction(instruction, None).first() + self.insert_instruction(instruction, None, None).first() + } + + /// Insert a binary instruction at the end of the current block. + /// Returns the result of the binary instruction. + pub(crate) fn insert_binary_with_source_location( + &mut self, + lhs: ValueId, + operator: BinaryOp, + rhs: ValueId, + location: Option, + ) -> ValueId { + let instruction = Instruction::Binary(Binary { lhs, rhs, operator }); + self.insert_instruction(instruction, None, location).first() } /// Insert a not instruction at the end of the current block. /// Returns the result of the instruction. - pub(crate) fn insert_not(&mut self, rhs: ValueId) -> ValueId { - self.insert_instruction(Instruction::Not(rhs), None).first() + pub(crate) fn insert_not(&mut self, rhs: ValueId, location: Option) -> ValueId { + self.insert_instruction(Instruction::Not(rhs), None, location).first() } /// Insert a cast instruction at the end of the current block. /// Returns the result of the cast instruction. pub(crate) fn insert_cast(&mut self, value: ValueId, typ: Type) -> ValueId { - self.insert_instruction(Instruction::Cast(value, typ), None).first() + self.insert_instruction(Instruction::Cast(value, typ), None, None).first() } /// Insert a truncate instruction at the end of the current block. @@ -219,14 +262,28 @@ impl FunctionBuilder { value: ValueId, bit_size: u32, max_bit_size: u32, + location: Location, ) -> ValueId { - self.insert_instruction(Instruction::Truncate { value, bit_size, max_bit_size }, None) - .first() + self.insert_instruction( + Instruction::Truncate { value, bit_size, max_bit_size }, + None, + Some(location), + ) + .first() } /// Insert a constrain instruction at the end of the current block. pub(crate) fn insert_constrain(&mut self, boolean: ValueId) { - self.insert_instruction(Instruction::Constrain(boolean), None); + self.insert_instruction(Instruction::Constrain(boolean), None, None); + } + + /// Insert a constrain instruction at the end of the current block. + pub(crate) fn insert_constrain_with_source_location( + &mut self, + boolean: ValueId, + location: Location, + ) { + self.insert_instruction(Instruction::Constrain(boolean), None, Some(location)); } /// Insert a call instruction at the end of the current block and return @@ -237,7 +294,25 @@ impl FunctionBuilder { arguments: Vec, result_types: Vec, ) -> Cow<[ValueId]> { - self.insert_instruction(Instruction::Call { func, arguments }, Some(result_types)).results() + self.insert_instruction(Instruction::Call { func, arguments }, Some(result_types), None) + .results() + } + + /// Insert a call instruction at the end of the current block and return + /// the results of the call. + pub(crate) fn insert_call_with_source_location( + &mut self, + func: ValueId, + arguments: Vec, + result_types: Vec, + location: Location, + ) -> Cow<[ValueId]> { + self.insert_instruction( + Instruction::Call { func, arguments }, + Some(result_types), + Some(location), + ) + .results() } /// Insert an instruction to extract an element from an array @@ -248,7 +323,25 @@ impl FunctionBuilder { element_type: Type, ) -> ValueId { let element_type = Some(vec![element_type]); - self.insert_instruction(Instruction::ArrayGet { array, index }, element_type).first() + self.insert_instruction(Instruction::ArrayGet { array, index }, element_type, None).first() + } + + /// Insert an instruction to extract an element from an array + /// Provide the source code location + pub(crate) fn insert_array_get_with_source_location( + &mut self, + array: ValueId, + index: ValueId, + element_type: Type, + location: Location, + ) -> ValueId { + let element_type = Some(vec![element_type]); + self.insert_instruction( + Instruction::ArrayGet { array, index }, + element_type, + Some(location), + ) + .first() } /// Insert an instruction to create a new array with the given index replaced with a new value @@ -258,7 +351,7 @@ impl FunctionBuilder { index: ValueId, value: ValueId, ) -> ValueId { - self.insert_instruction(Instruction::ArraySet { array, index, value }, None).first() + self.insert_instruction(Instruction::ArraySet { array, index, value }, None, None).first() } /// Terminates the current block with the given terminator instruction 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 a154af45ecd..026e63cdf3f 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs @@ -4,6 +4,7 @@ use std::sync::{Mutex, RwLock}; use acvm::FieldElement; use iter_extended::vecmap; +use noirc_errors::Location; use noirc_frontend::monomorphization::ast::{self, LocalId, Parameters}; use noirc_frontend::monomorphization::ast::{FuncId, Program}; use noirc_frontend::Signedness; @@ -223,18 +224,20 @@ impl<'a> FunctionContext<'a> { mut lhs: ValueId, operator: noirc_frontend::BinaryOpKind, mut rhs: ValueId, + location: Location, ) -> Values { let op = convert_operator(operator); if op == BinaryOp::Eq && matches!(self.builder.type_of_value(lhs), Type::Array(..)) { - return self.insert_array_equality(lhs, operator, rhs); + return self.insert_array_equality(lhs, operator, rhs, location); } if operator_requires_swapped_operands(operator) { std::mem::swap(&mut lhs, &mut rhs); } - let mut result = self.builder.insert_binary(lhs, op, rhs); + let mut result = + self.builder.insert_binary_with_source_location(lhs, op, rhs, Some(location)); if let Some(max_bit_size) = operator_result_max_bit_size_to_truncate( operator, @@ -250,11 +253,11 @@ impl<'a> FunctionContext<'a> { unreachable!("ICE: Truncation attempted on non-integer"); } }; - result = self.builder.insert_truncate(result, bit_size, max_bit_size); + result = self.builder.insert_truncate(result, bit_size, max_bit_size, location); } if operator_requires_not(operator) { - result = self.builder.insert_not(result); + result = self.builder.insert_not(result, Some(location)); } result.into() } @@ -286,6 +289,7 @@ impl<'a> FunctionContext<'a> { lhs: ValueId, operator: noirc_frontend::BinaryOpKind, rhs: ValueId, + location: Location, ) -> Values { let lhs_type = self.builder.type_of_value(lhs); let rhs_type = self.builder.type_of_value(rhs); @@ -313,7 +317,7 @@ impl<'a> FunctionContext<'a> { // pre-loop let result_alloc = self.builder.insert_allocate(); let true_value = self.builder.numeric_constant(1u128, Type::bool()); - self.builder.insert_store(result_alloc, true_value); + self.builder.insert_store_with_source_location(result_alloc, true_value, location); let zero = self.builder.field_constant(0u128); self.builder.terminate_with_jmp(loop_start, vec![zero]); @@ -321,27 +325,42 @@ impl<'a> FunctionContext<'a> { self.builder.switch_to_block(loop_start); let i = self.builder.add_block_parameter(loop_start, Type::field()); let array_length = self.builder.field_constant(array_length as u128); - let v0 = self.builder.insert_binary(i, BinaryOp::Lt, array_length); + let v0 = self.builder.insert_binary_with_source_location( + i, + BinaryOp::Lt, + array_length, + Some(location), + ); self.builder.terminate_with_jmpif(v0, loop_body, loop_end); // loop body self.builder.switch_to_block(loop_body); - let v1 = self.builder.insert_array_get(lhs, i, element_type.clone()); - let v2 = self.builder.insert_array_get(rhs, i, element_type); - let v3 = self.builder.insert_binary(v1, BinaryOp::Eq, v2); - let v4 = self.builder.insert_load(result_alloc, Type::bool()); - let v5 = self.builder.insert_binary(v4, BinaryOp::And, v3); - self.builder.insert_store(result_alloc, v5); + let v1 = self.builder.insert_array_get_with_source_location( + lhs, + i, + element_type.clone(), + location, + ); + let v2 = self.builder.insert_array_get_with_source_location(rhs, i, element_type, location); + let v3 = + self.builder.insert_binary_with_source_location(v1, BinaryOp::Eq, v2, Some(location)); + let v4 = + self.builder.insert_load_with_source_location(result_alloc, Type::bool(), location); + let v5 = + self.builder.insert_binary_with_source_location(v4, BinaryOp::And, v3, Some(location)); + self.builder.insert_store_with_source_location(result_alloc, v5, location); let one = self.builder.field_constant(1u128); - let v6 = self.builder.insert_binary(i, BinaryOp::Add, one); + let v6 = + self.builder.insert_binary_with_source_location(i, BinaryOp::Add, one, Some(location)); self.builder.terminate_with_jmp(loop_start, vec![v6]); // loop end self.builder.switch_to_block(loop_end); - let mut result = self.builder.insert_load(result_alloc, Type::bool()); + let mut result = + self.builder.insert_load_with_source_location(result_alloc, Type::bool(), location); if operator_requires_not(operator) { - result = self.builder.insert_not(result); + result = self.builder.insert_not(result, Some(location)); } result.into() } @@ -356,9 +375,15 @@ impl<'a> FunctionContext<'a> { function: ValueId, arguments: Vec, result_type: &ast::Type, + location: Location, ) -> Values { let result_types = Self::convert_type(result_type).flatten(); - let results = self.builder.insert_call(function, arguments, result_types); + let results = self.builder.insert_call_with_source_location( + function, + arguments, + result_types, + location, + ); let mut i = 0; let reshaped_return_values = Self::map_type(result_type, |_| { @@ -505,9 +530,9 @@ impl<'a> FunctionContext<'a> { let variable = self.ident_lvalue(ident); (variable.clone(), LValue::Ident(variable)) } - ast::LValue::Index { array, index, element_type, location: _ } => { + ast::LValue::Index { array, index, element_type, location } => { let (old_array, index, index_lvalue) = self.index_lvalue(array, index); - let element = self.codegen_array_index(old_array, index, element_type); + let element = self.codegen_array_index(old_array, index, element_type, *location); (element, index_lvalue) } ast::LValue::MemberAccess { object, field_index: index } => { 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 a7fcf2d664b..1e28db1cf55 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs @@ -167,7 +167,7 @@ impl<'a> FunctionContext<'a> { fn codegen_unary(&mut self, unary: &ast::Unary) -> Values { let rhs = self.codegen_non_tuple_expression(&unary.rhs); match unary.operator { - noirc_frontend::UnaryOp::Not => self.builder.insert_not(rhs).into(), + noirc_frontend::UnaryOp::Not => self.builder.insert_not(rhs, None).into(), noirc_frontend::UnaryOp::Minus => { let typ = self.builder.type_of_value(rhs); let zero = self.builder.numeric_constant(0u128, typ); @@ -179,13 +179,13 @@ impl<'a> FunctionContext<'a> { fn codegen_binary(&mut self, binary: &ast::Binary) -> Values { let lhs = self.codegen_non_tuple_expression(&binary.lhs); let rhs = self.codegen_non_tuple_expression(&binary.rhs); - self.insert_binary(lhs, binary.operator, rhs) + self.insert_binary(lhs, binary.operator, rhs, binary.location) } fn codegen_index(&mut self, index: &ast::Index) -> Values { let array = self.codegen_non_tuple_expression(&index.collection); let index_value = self.codegen_non_tuple_expression(&index.index); - self.codegen_array_index(array, index_value, &index.element_type) + self.codegen_array_index(array, index_value, &index.element_type, index.location) } /// This is broken off from codegen_index so that it can also be @@ -199,17 +199,23 @@ impl<'a> FunctionContext<'a> { array: super::ir::value::ValueId, index: super::ir::value::ValueId, element_type: &ast::Type, + location: Location, ) -> Values { // base_index = index * type_size let type_size = Self::convert_type(element_type).size_of_type(); let type_size = self.builder.field_constant(type_size as u128); - let base_index = self.builder.insert_binary(index, BinaryOp::Mul, type_size); + let base_index = self.builder.insert_binary_with_source_location( + index, + BinaryOp::Mul, + type_size, + Some(location), + ); let mut field_index = 0u128; Self::map_type(element_type, |typ| { let offset = self.make_offset(base_index, field_index); field_index += 1; - self.builder.insert_array_get(array, offset, typ).into() + self.builder.insert_array_get_with_source_location(array, offset, typ, location).into() }) } @@ -350,7 +356,7 @@ impl<'a> FunctionContext<'a> { .collect(); let function = self.codegen_non_tuple_expression(&call.func); - self.insert_call(function, arguments, &call.return_type) + self.insert_call(function, arguments, &call.return_type, call.location) } /// Generate SSA for the given variable. @@ -371,9 +377,9 @@ impl<'a> FunctionContext<'a> { Self::unit_value() } - fn codegen_constrain(&mut self, expr: &Expression, _location: Location) -> Values { + fn codegen_constrain(&mut self, expr: &Expression, location: Location) -> Values { let boolean = self.codegen_non_tuple_expression(expr); - self.builder.insert_constrain(boolean); + self.builder.insert_constrain_with_source_location(boolean, location); Self::unit_value() } From 7745e8c67bf7ab9342e9302635876908069cf239 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 5 Jul 2023 13:41:44 +0000 Subject: [PATCH 02/23] reset location --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index c252fc16044..3dc9c9cc781 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -337,6 +337,7 @@ impl Context { unreachable!("Expected all load instructions to be removed before acir_gen") } } + self.acir_context.set_location(None); } fn gen_brillig_for(&self, func: &Function, brillig: &Brillig) -> Vec { From f2d847c6374b066a75755991b1a18a483f93229f Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 6 Jul 2023 13:39:11 +0000 Subject: [PATCH 03/23] merge from master --- Cargo.lock | 7 + Cargo.toml | 1 + crates/fm/src/file_map.rs | 4 + crates/nargo_cli/Cargo.toml | 1 + .../nargo_cli/src/cli/codegen_verifier_cmd.rs | 2 +- crates/nargo_cli/src/cli/compile_cmd.rs | 6 +- crates/nargo_cli/src/cli/execute_cmd.rs | 70 ++++++++- crates/nargo_cli/src/cli/gates_cmd.rs | 2 +- crates/nargo_cli/src/cli/mod.rs | 4 +- crates/nargo_cli/src/cli/prove_cmd.rs | 2 +- crates/nargo_cli/src/cli/verify_cmd.rs | 2 +- .../test_data_ssa_refactor/2_div/src/main.nr | 2 +- .../arithmetic_binary_operations/src/main.nr | 22 ++- .../array_len/src/main.nr | 33 +++++ .../brillig_calls/src/main.nr | 12 +- .../brillig_conditional/src/main.nr | 12 +- .../brillig_modulo/src/main.nr | 4 +- .../brillig_not/Prover.toml | 2 +- .../brillig_not/src/main.nr | 9 +- .../test_data_ssa_refactor/sha256/Prover.toml | 139 ++++++++++++++---- .../test_data_ssa_refactor/sha256/src/main.nr | 16 +- crates/noirc_driver/src/lib.rs | 25 +++- crates/noirc_driver/src/program.rs | 5 + crates/noirc_errors/src/position.rs | 6 + crates/noirc_evaluator/Cargo.toml | 2 + crates/noirc_evaluator/src/lib.rs | 9 +- crates/noirc_evaluator/src/ssa_refactor.rs | 14 +- .../acir_gen/acir_ir/acir_variable.rs | 11 ++ .../acir_gen/acir_ir/generated_acir.rs | 4 +- noir_stdlib/src/field.nr | 24 +++ 30 files changed, 377 insertions(+), 75 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 03bf41c73dd..9f701208991 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1977,6 +1977,7 @@ dependencies = [ "color-eyre", "const_format", "dirs", + "fm", "hex", "iter-extended", "nargo", @@ -2076,6 +2077,7 @@ version = "0.7.1" dependencies = [ "acvm", "arena", + "fm", "im", "iter-extended", "noirc_abi", @@ -2084,6 +2086,7 @@ dependencies = [ "num-bigint", "num-traits", "rand 0.8.5", + "serde", "thiserror", ] @@ -4218,3 +4221,7 @@ dependencies = [ "quote", "syn 2.0.18", ] + +[[patch.unused]] +name = "acvm" +version = "0.16.0" diff --git a/Cargo.toml b/Cargo.toml index 4f2bc75528e..e23384c48ea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,3 +58,4 @@ wasm-bindgen-test = "0.3.33" [patch.crates-io] async-lsp = { git = "https://github.com/oxalica/async-lsp", rev = "09dbcc11046f7a188a80137f8d36484d86c78c78" } +acvm = { path = "../../noir-4commit/acvm-master/acvm/acvm" } diff --git a/crates/fm/src/file_map.rs b/crates/fm/src/file_map.rs index 4dbfbece0e0..bb891e06d79 100644 --- a/crates/fm/src/file_map.rs +++ b/crates/fm/src/file_map.rs @@ -46,6 +46,10 @@ impl FileId { pub fn dummy() -> FileId { FileId(0) } + + pub fn new(file_id: usize) -> FileId { + FileId(file_id) + } } pub struct File<'input>(&'input SimpleFile); diff --git a/crates/nargo_cli/Cargo.toml b/crates/nargo_cli/Cargo.toml index a6943695846..285fde317c8 100644 --- a/crates/nargo_cli/Cargo.toml +++ b/crates/nargo_cli/Cargo.toml @@ -30,6 +30,7 @@ noirc_frontend.workspace = true noirc_abi.workspace = true noirc_errors.workspace = true acvm.workspace = true +fm.workspace = true toml.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs index b65d64bb917..92af3a99e84 100644 --- a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -51,7 +51,7 @@ pub(crate) fn run( (common_reference_string, program) } None => { - let program = + let (program, driver) = compile_circuit(backend, config.program_dir.as_ref(), &args.compile_options)?; let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 9c732396988..3467202c8af 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -2,6 +2,7 @@ use acvm::Backend; use iter_extended::try_vecmap; use nargo::artifacts::contract::PreprocessedContract; use noirc_driver::{CompileOptions, CompiledProgram, Driver, ErrorsAndWarnings, Warnings}; +use noirc_errors::{FileDiagnostic, CustomDiagnostic}; use noirc_errors::reporter::ReportedErrors; use std::path::Path; @@ -89,7 +90,7 @@ pub(crate) fn run( ); } } else { - let program = compile_circuit(backend, &config.program_dir, &args.compile_options)?; + let (program, driver) = compile_circuit(backend, &config.program_dir, &args.compile_options)?; common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; @@ -115,7 +116,8 @@ pub(crate) fn compile_circuit( &|op| backend.supports_opcode(op), compile_options, ); - report_errors(result, &driver, compile_options.deny_warnings).map_err(Into::into) + let compiled_program = report_errors(result, &driver, compile_options.deny_warnings); + Ok((compiled_program?, driver)) } /// Helper function for reporting any errors in a Result<(T, Warnings), ErrorsAndWarnings> diff --git a/crates/nargo_cli/src/cli/execute_cmd.rs b/crates/nargo_cli/src/cli/execute_cmd.rs index c175c0336f1..f7245a6b7e7 100644 --- a/crates/nargo_cli/src/cli/execute_cmd.rs +++ b/crates/nargo_cli/src/cli/execute_cmd.rs @@ -1,12 +1,17 @@ +use std::io::StdoutLock; use std::path::Path; +use fm::FileId; + use acvm::acir::{circuit::Circuit, native_types::WitnessMap}; use acvm::Backend; use clap::Args; use noirc_abi::input_parser::{Format, InputValue}; use noirc_abi::{Abi, InputMap}; -use noirc_driver::{CompileOptions, CompiledProgram}; +use noirc_driver::{CompileOptions, CompiledProgram, ErrorsAndWarnings}; +use noirc_errors::{Location, Span, FileDiagnostic, CustomDiagnostic}; +use super::compile_cmd::report_errors; use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; use super::NargoConfig; use crate::{ @@ -57,19 +62,70 @@ fn execute_with_path( prover_name: String, compile_options: &CompileOptions, ) -> Result<(Option, WitnessMap), CliError> { - let CompiledProgram { abi, circuit } = compile_circuit(backend, program_dir, compile_options)?; + let (compiled_program, driver) = compile_circuit(backend, program_dir, compile_options)?; + let CompiledProgram { abi, circuit, debug } = compiled_program; + + // Parse the initial witness values from Prover.toml let (inputs_map, _) = read_inputs_from_file(program_dir, prover_name.as_str(), Format::Toml, &abi)?; - let solved_witness = execute_program(backend, circuit, &abi, &inputs_map)?; + let solved_witness_err = execute_program(backend, circuit, &abi, &inputs_map); + + match solved_witness_err { + Ok(solved_witness) => { + let public_abi = abi.public_abi(); + let (_, return_value) = public_abi.decode(&solved_witness)?; + + Ok((return_value, solved_witness)) + }, + Err(err) => { + match extract_unsatisfied_constraint_error(&err) { + Some(opcode_index) => { + dbg!(&opcode_index); + // let loca = Some(Location::new(Span::inclusive(3,4),FileId::new(0))); + if let Some(loc) = debug.opcode_location( + opcode_index + ) { + dbg!(&loc); + let errs_warnings: ErrorsAndWarnings = vec![FileDiagnostic { + file_id: loc.file, + diagnostic: CustomDiagnostic::simple_error("Unsatisfied constraint".to_string(), "happening on this line".to_string(), loc.span) + }]; + noirc_errors::reporter::report_all(driver.file_manager(), &errs_warnings, false); + } + }, + None => (), + }; + Err(err) + }, + } + +} + +fn extract_unsatisfied_constraint_error(err : &CliError) -> Option{ + + let nargo_err = match err { + CliError::NargoError(err) => err, + _=> return None, + }; + + let solving_err= match nargo_err { + nargo::NargoError::SolvingError(err) => err, + _=> return None, + }; + + match solving_err{ + acvm::pwg::OpcodeResolutionError::UnsatisfiedConstrain{opcode_index} => return Some(opcode_index), + _=> return None, + } + + return None + } + - let public_abi = abi.public_abi(); - let (_, return_value) = public_abi.decode(&solved_witness)?; - Ok((return_value, solved_witness)) -} pub(crate) fn execute_program( backend: &B, diff --git a/crates/nargo_cli/src/cli/gates_cmd.rs b/crates/nargo_cli/src/cli/gates_cmd.rs index 88e11c683eb..1cf4a251861 100644 --- a/crates/nargo_cli/src/cli/gates_cmd.rs +++ b/crates/nargo_cli/src/cli/gates_cmd.rs @@ -28,7 +28,7 @@ fn count_gates_with_path>( program_dir: P, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let compiled_program = compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let (compiled_program, driver) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; let num_opcodes = compiled_program.circuit.opcodes.len(); println!( diff --git a/crates/nargo_cli/src/cli/mod.rs b/crates/nargo_cli/src/cli/mod.rs index a8e4fd3e363..f7e909abba8 100644 --- a/crates/nargo_cli/src/cli/mod.rs +++ b/crates/nargo_cli/src/cli/mod.rs @@ -73,7 +73,7 @@ pub fn start_cli() -> eyre::Result<()> { NargoCommand::New(args) => new_cmd::run(&backend, args, config), NargoCommand::Check(args) => check_cmd::run(&backend, args, config), NargoCommand::Compile(args) => compile_cmd::run(&backend, args, config), - NargoCommand::Execute(args) => execute_cmd::run(&backend, args, config), + NargoCommand::Execute(args) =>execute_cmd::run(&backend, args, config), NargoCommand::Prove(args) => prove_cmd::run(&backend, args, config), NargoCommand::Verify(args) => verify_cmd::run(&backend, args, config), NargoCommand::Test(args) => test_cmd::run(&backend, args, config), @@ -99,7 +99,7 @@ pub fn prove_and_verify(program_dir: &Path, experimental_ssa: bool) -> bool { experimental_ssa, }; - let program = + let (program, driver) = compile_circuit(&backend, program_dir, &compile_options).expect("Compile should succeed"); // Parse the initial witness values from Prover.toml diff --git a/crates/nargo_cli/src/cli/prove_cmd.rs b/crates/nargo_cli/src/cli/prove_cmd.rs index 44f3bf62484..6e539b32f71 100644 --- a/crates/nargo_cli/src/cli/prove_cmd.rs +++ b/crates/nargo_cli/src/cli/prove_cmd.rs @@ -103,7 +103,7 @@ pub(crate) fn prove_with_path>( (common_reference_string, program) } None => { - let program = compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let (program, driver) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/nargo_cli/src/cli/verify_cmd.rs b/crates/nargo_cli/src/cli/verify_cmd.rs index e326aafbc52..129debf7b9e 100644 --- a/crates/nargo_cli/src/cli/verify_cmd.rs +++ b/crates/nargo_cli/src/cli/verify_cmd.rs @@ -83,7 +83,7 @@ fn verify_with_path>( (common_reference_string, program) } None => { - let program = compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let (program, driver) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/2_div/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/2_div/src/main.nr index ff0dee755cc..f9756fd1977 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/2_div/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/2_div/src/main.nr @@ -1,7 +1,7 @@ // Testing integer division: 7/3 = 2 fn main(mut x: u32, y: u32, z: u32) { let a = x % y; - assert(x / y == z); + assert(x / y == z+1); assert(a == x - z*y); assert((50 as u64) % (9 as u64) == 5); } diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr index 391aa27049d..357bf268b07 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr @@ -1,12 +1,24 @@ +use dep::std; // Tests a very simple program. // // The features being tested are: // Binary addition, multiplication, division // x = 3, y = 4, z = 5 fn main(x : Field, y : Field, z : Field) -> pub Field { - let a = x + x; // 3 + 3 = 6 - let b = a - y; // 6 - 4 = 2 - let c = b * z; // 2 * 5 = 10 - let d = c / a; // 10 / 6 (This uses field inversion, so we test it by multiplying by `a`) - d * a +// let a = x + x; // 3 + 3 = 6 +// let b = a - y; // 6 - 4 = 2 +// let c = b * z; // 2 * 5 = 10 +// let d = c / a; // 10 / 6 (This uses field inversion, so we test it by multiplying by `a`) +// d * a +let toto = y+z; +assert(toto==toto); + let b = x.to_le_bits_unconstrained(8); + let c = x.to_le_bits(8); + let a:[u1;8] = x.toto(); + assert(a[0]==0); + // assert(b[0]==c[0]); + // assert(b[1]==c[1]); + std::println(b); + std::println(c); + 0 } \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr index 9099cfa2144..53398171cee 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr @@ -12,7 +12,40 @@ fn nested_call(b: [Field]) -> Field { len_plus_1(b) } +fn bar(mut a: [Field]) -> [Field] { + // a.push_back(a[1]); + a +} + +fn foo(mut a: [Field]) -> Field{ + let mut d = -(a[0] as i32); + d = -5; + //let mut b=a; + let mut sum = 0; +let mut b:[Field] = [0,0];// hum.. +let k = b.push_back(3); + if a[0]==0 { + a.push_back(10); + } else { + + } + a = bar(a); + // let c=[0;a.len()]; marche po + for i in 0..a.len()-1 { + // b[i] = a[i]; + b.push_back(a[i]); + } + // for i in b { + // sum = sum+i; + // } + // sum + a[2] +} + fn main(len3: [u8; 3], len4: [Field; 4]) { + + let toto = foo(len4); + std::println(toto); assert(len_plus_1(len3) == 4); assert(len_plus_1(len4) == 5); assert(add_lens(len3, len4) == 7); diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr index 795fc02c35f..c82e61a95cc 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr @@ -1,10 +1,20 @@ // Tests a very simple program. // // The features being tested is brillig calls -fn main(x: u32) { +use dep::std; +fn main(x: u32) -> pub [u32; 2] { + assert(sha(x as u8) != 0); assert(entry_point(x) == 2); swap_entry_point(x, x + 1); assert(deep_entry_point(x) == 4); + [x,x+1] +} + + +unconstrained fn sha(x : u8) -> u8 { + // std::hash::sha256([x])[0] + // std::sha256::digest([x])[0] + 1 } unconstrained fn inner(x : u32) -> u32 { diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr index 4ddd351ad04..088022981ca 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr @@ -2,10 +2,20 @@ // // The features being tested is basic conditonal on brillig fn main(x: Field) { - assert(4 == conditional(x as bool)); + assert(5 == conditional(x as bool)); } unconstrained fn conditional(x : bool) -> Field { + // if x { + // 4 + // }else { + // 5 + // } + let a = toto(x); + a + 1 +} + +unconstrained fn toto(x : bool) -> Field { if x { 4 }else { diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr index 1cab78ecb95..3c63e5949d3 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr @@ -1,7 +1,9 @@ -// Tests a very simple program. + +use dep::std;// Tests a very simple program. // // The features being tested is modulo operations on brillig fn main() { + // std::println(modulo(47, 3)); assert(modulo(47, 3) == 2); assert(modulo(2, 3) == 2); assert(signed_modulo(5, 3) == 2); diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/Prover.toml index a0397e89477..63016ec4ef2 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/Prover.toml +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/Prover.toml @@ -1,2 +1,2 @@ x = "1" -y = "0" +y = "27" diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/src/main.nr index bc94810efb9..0b814c8b14c 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/src/main.nr @@ -2,10 +2,15 @@ // // The features being tested is not instruction on brillig fn main(x: Field, y : Field) { - assert(false == not_operator(x as bool)); - assert(true == not_operator(y as bool)); + assert(!(x as bool) == not_operator(x as bool)); + assert((x as bool)== not_operator(!(x as bool))); + assert(!(y as u8) == not_operator_8bit(y as u8)); } unconstrained fn not_operator(x : bool) -> bool { !x +} + +unconstrained fn not_operator_8bit(x : u8) -> u8 { + !x } \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml index c4df1b749bb..ae1395a9a4b 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml @@ -1,36 +1,111 @@ -x = 0xbd -result = [ +# x = 0xbd +# result = [ +# 0x68, +# 0x32, +# 0x57, +# 0x20, +# 0xaa, +# 0xbd, +# 0x7c, +# 0x82, +# 0xf3, +# 0x0f, +# 0x55, +# 0x4b, +# 0x31, +# 0x3d, +# 0x05, +# 0x70, +# 0xc9, +# 0x5a, +# 0xcc, +# 0xbb, +# 0x7d, +# 0xc4, +# 0xb5, +# 0xaa, +# 0xe1, +# 0x12, +# 0x04, +# 0xc0, +# 0x8f, +# 0xfe, +# 0x73, +# 0x2b, +# ] + +hashed_message = [ + 0x3a, + 0x73, + 0xf4, + 0x12, + 0x3a, + 0x5c, + 0xd2, + 0x12, + 0x1f, + 0x21, + 0xcd, + 0x7e, + 0x8d, + 0x35, + 0x88, + 0x35, + 0x47, + 0x69, + 0x49, + 0xd0, + 0x35, + 0xd9, + 0xc2, + 0xda, 0x68, - 0x32, - 0x57, + 0x06, + 0xb4, + 0x63, + 0x3a, + 0xc8, + 0xc1, + 0xe2, +] +message = [ + 0x49, + 0x6e, + 0x73, + 0x74, + 0x72, + 0x75, + 0x63, + 0x74, + 0x69, + 0x6f, + 0x6e, + 0x73, 0x20, - 0xaa, - 0xbd, - 0x7c, - 0x82, - 0xf3, - 0x0f, - 0x55, - 0x4b, - 0x31, - 0x3d, - 0x05, - 0x70, - 0xc9, - 0x5a, - 0xcc, - 0xbb, - 0x7d, - 0xc4, - 0xb5, - 0xaa, - 0xe1, - 0x12, - 0x04, - 0xc0, - 0x8f, - 0xfe, + 0x75, + 0x6e, + 0x63, + 0x6c, + 0x65, + 0x61, + 0x72, + 0x2c, + 0x20, + 0x61, 0x73, - 0x2b, -] + 0x6b, + 0x20, + 0x61, + 0x67, + 0x61, + 0x69, + 0x6e, + 0x20, + 0x6c, + 0x61, + 0x74, + 0x65, + 0x72, + 0x2e, +] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr index fd5340e2384..87ebbcdb92b 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr @@ -11,9 +11,13 @@ // This can be done in ACIR! use dep::std; -fn main(x: Field, result: [u8; 32]) { - // We use the `as` keyword here to denote the fact that we want to take just the first byte from the x Field - // The padding is taken care of by the program - let digest = std::hash::sha256([x as u8]); - assert(digest == result); -} +// fn main(x: Field, result: [u8; 32]) { +// // We use the `as` keyword here to denote the fact that we want to take just the first byte from the x Field +// // The padding is taken care of by the program +// let digest = std::hash::sha256([x as u8]); +// assert(digest == result); +// } + +fn main(message : [u8;38],hashed_message : [u8;32]) { + assert(hashed_message == std::hash::sha256(message)); +} \ No newline at end of file diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 3049fd4eef9..6e46e40c3a1 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -359,7 +359,7 @@ impl Driver { ) -> Result { let program = monomorphize(main_function, &self.context.def_interner); - let (circuit, abi) = if options.experimental_ssa { + let (circuit, debug, abi) = if options.experimental_ssa { experimental_create_circuit(program, options.show_ssa, options.show_output)? } else { create_circuit(program, options.show_ssa, options.show_output)? @@ -375,7 +375,28 @@ impl Driver { diagnostic: CustomDiagnostic::from_message("produced an acvm compile error"), })?; - Ok(CompiledProgram { circuit: optimized_circuit, abi }) + Ok(CompiledProgram { circuit: optimized_circuit, debug, abi }) + /* match circuit_abi { + Ok((circuit, debug, abi)) => { + //let mut locations = Vec::new(); + // todo si on veut ca (serialisation), on met la methode dans DebugInfo + // for opcode in loc.keys() { + // if let Some(location) = loc.opcode_location(opcode) { + // let file_id = location.file.as_usize(); + // let start = location.span.start(); + // let end = location.span.end(); + // locations.push((opcode, file_id,start,end)); + // } + // } + //locations.serialize(serializer) + Ok(CompiledProgram { circuit, abi, debug }) + }, + Err(err) => { + // The FileId here will be the file id of the file with the main file + // Errors will be shown at the call site without a stacktrace + Err(err.into()) + } + }*/ } /// Returns a list of all functions in the current crate marked with #[test] diff --git a/crates/noirc_driver/src/program.rs b/crates/noirc_driver/src/program.rs index 95405f36a5b..5f983b8911d 100644 --- a/crates/noirc_driver/src/program.rs +++ b/crates/noirc_driver/src/program.rs @@ -1,5 +1,8 @@ +use std::collections::{HashMap, BTreeMap}; + use acvm::acir::circuit::Circuit; +use noirc_evaluator::debug_info::DebugInfo; use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[derive(Debug, Serialize, Deserialize, Clone)] @@ -7,6 +10,7 @@ pub struct CompiledProgram { #[serde(serialize_with = "serialize_circuit", deserialize_with = "deserialize_circuit")] pub circuit: Circuit, pub abi: noirc_abi::Abi, + pub debug: DebugInfo, } pub(crate) fn serialize_circuit(circuit: &Circuit, s: S) -> Result @@ -27,3 +31,4 @@ where let circuit = Circuit::read(&*circuit_bytes).unwrap(); Ok(circuit) } + diff --git a/crates/noirc_errors/src/position.rs b/crates/noirc_errors/src/position.rs index 97f510d91bf..e16ab669712 100644 --- a/crates/noirc_errors/src/position.rs +++ b/crates/noirc_errors/src/position.rs @@ -1,3 +1,4 @@ + use codespan::Span as ByteSpan; use fm::FileId; use std::{ @@ -51,6 +52,7 @@ impl std::borrow::Borrow for Spanned { } #[derive(PartialEq, PartialOrd, Eq, Ord, Hash, Debug, Copy, Clone, Default)] +#[cfg_attr(feature = "serialization", derive(Deserialize, Serialize))] pub struct Span(ByteSpan); impl Span { @@ -115,6 +117,7 @@ impl chumsky::Span for Span { } #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "serialization", derive(Deserialize, Serialize))] pub struct Location { pub span: Span, pub file: FileId, @@ -124,4 +127,7 @@ impl Location { pub fn new(span: Span, file: FileId) -> Self { Self { span, file } } + pub fn flatten(&self) -> (u32, u32, usize) { + (self.span.start(), self.span.end(), self.file.as_usize()) + } } diff --git a/crates/noirc_evaluator/Cargo.toml b/crates/noirc_evaluator/Cargo.toml index b2a01b142d0..e03f3c6de55 100644 --- a/crates/noirc_evaluator/Cargo.toml +++ b/crates/noirc_evaluator/Cargo.toml @@ -11,7 +11,9 @@ noirc_frontend.workspace = true noirc_errors.workspace = true noirc_abi.workspace = true acvm.workspace = true +fm.workspace = true arena.workspace = true +serde.workspace = true iter-extended.workspace = true thiserror.workspace = true num-bigint = "0.4" diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index 05204943c49..fde73161350 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -11,17 +11,20 @@ mod ssa; pub mod ssa_refactor; pub mod brillig; +pub mod debug_info; use acvm::{ acir::circuit::{opcodes::Opcode as AcirOpcode, Circuit, PublicInputs}, acir::native_types::{Expression, Witness}, }; +use debug_info::DebugInfo; use errors::{RuntimeError, RuntimeErrorKind}; use iter_extended::vecmap; use noirc_abi::{Abi, AbiType, AbiVisibility}; +use noirc_errors::{Location}; use noirc_frontend::monomorphization::ast::*; use ssa::{node::ObjectType, ssa_gen::IrGenerator}; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; #[derive(Default)] pub struct Evaluator { @@ -67,7 +70,7 @@ pub fn create_circuit( program: Program, enable_logging: bool, show_output: bool, -) -> Result<(Circuit, Abi), RuntimeError> { +) -> Result<(Circuit, DebugInfo, Abi), RuntimeError> { let mut evaluator = Evaluator::default(); // First evaluate the main function @@ -91,7 +94,7 @@ pub fn create_circuit( let (parameters, return_type) = program.main_function_signature; let abi = Abi { parameters, param_witnesses, return_type, return_witnesses: return_values }; - Ok((circuit, abi)) + Ok((circuit, DebugInfo::default(), abi)) } impl Evaluator { diff --git a/crates/noirc_evaluator/src/ssa_refactor.rs b/crates/noirc_evaluator/src/ssa_refactor.rs index ed9e9b56a78..007f338d44b 100644 --- a/crates/noirc_evaluator/src/ssa_refactor.rs +++ b/crates/noirc_evaluator/src/ssa_refactor.rs @@ -9,6 +9,9 @@ use crate::errors::RuntimeError; use acvm::acir::circuit::{Circuit, PublicInputs}; + +use crate::{debug_info::DebugInfo}; + use noirc_abi::Abi; use noirc_frontend::monomorphization::ast::Program; @@ -61,9 +64,9 @@ pub fn experimental_create_circuit( program: Program, enable_logging: bool, show_output: bool, -) -> Result<(Circuit, Abi), RuntimeError> { +) -> Result<(Circuit, DebugInfo, Abi), RuntimeError> { let func_sig = program.main_function_signature.clone(); - let GeneratedAcir { current_witness_index, opcodes, return_witnesses, .. } = + let GeneratedAcir { current_witness_index, opcodes, return_witnesses, locations, .. } = optimize_into_acir(program, show_output, enable_logging); let abi = gen_abi(func_sig, return_witnesses.clone()); @@ -73,9 +76,14 @@ pub fn experimental_create_circuit( PublicInputs(public_abi.param_witnesses.values().flatten().copied().collect()); let return_values = PublicInputs(return_witnesses.into_iter().collect()); + let circuit = Circuit { current_witness_index, opcodes, public_parameters, return_values }; - Ok((circuit, abi)) + //dbg!(&locations); + let mut debug_info = DebugInfo::new(locations); + //let opcode_ids = (0..optimized_circuit.opcodes.len()).collect(); + //debug_info.update_acir(opcode_ids); + Ok((circuit, debug_info, abi)) } impl Ssa { diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs index d5b63dfce0c..1d1c90ee5d4 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs @@ -766,6 +766,17 @@ impl AcirContext { /// Terminates the context and takes the resulting `GeneratedAcir` pub(crate) fn finish(self) -> GeneratedAcir { + // let mut diag = Vec::new(); + // for op in self.acir_ir.locations.keys() { + // diag.push( + // Diagnostic::simple_error( + // format!("opcode:{}", op), + // self.acir_ir.opcodes[*op].to_string(), + // self.acir_ir.locations[op], + // )); + // } + + self.acir_ir } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs index cdf07ae06db..2fc38d18d0f 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs @@ -41,7 +41,7 @@ pub(crate) struct GeneratedAcir { pub(crate) return_witnesses: Vec, /// Correspondance between an opcode index (in opcodes) and the source code location which generated it - locations: HashMap, + pub(crate) locations: HashMap, /// Source code location of the current instruction being processed /// None if we do not know the location @@ -58,7 +58,7 @@ impl GeneratedAcir { fn push_opcode(&mut self, opcode: AcirOpcode) { self.opcodes.push(opcode); if let Some(location) = self.current_location { - self.locations.insert(self.opcodes.len() - 1, location); + self.locations.insert(self.opcodes.len() - 1, location.flatten()); } } diff --git a/noir_stdlib/src/field.nr b/noir_stdlib/src/field.nr index b7773182d66..e26e9ea6a3d 100644 --- a/noir_stdlib/src/field.nr +++ b/noir_stdlib/src/field.nr @@ -33,6 +33,30 @@ impl Field { r } + // fn to_le_bits(x : Field) -> [u1; N] { + // let bits = x.to_le_bits_unconstrained(N); + // let result = [0;N]; + // for i in 0..N { + // result[i] = bits[i]; + // } + // result + // } + fn toto(_x : Field) -> [u1; N] { + [0;N] + } + + unconstrained fn to_le_bits_unconstrained(self, bit_size: u32) -> [u1; 254] { + let mut bits = [0; 254]; + let mut a = self; + for i in 0..bit_size { + let b = (a as u120) % 2; ///temp + bits[i as Field] = b as u1; + a = (a - b as Field) /2; + } + bits + } + + // Parity of (prime) Field element, i.e. sgn0(x mod p) = 0 if x ∈ {0, ..., p-1} is even, otherwise sgn0(x mod p) = 1. fn sgn0(self) -> u1 { self as u1 From 9e65375ba999509155545d25c6c96c8730c1b904 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 6 Jul 2023 15:22:08 +0000 Subject: [PATCH 04/23] Display the location in the error and use index from acvm --- crates/fm/Cargo.toml | 1 + crates/fm/src/file_map.rs | 10 +- .../nargo_cli/src/cli/codegen_verifier_cmd.rs | 2 +- crates/nargo_cli/src/cli/compile_cmd.rs | 5 +- crates/nargo_cli/src/cli/execute_cmd.rs | 105 ++++++++++-------- crates/nargo_cli/src/cli/gates_cmd.rs | 2 +- crates/nargo_cli/src/cli/mod.rs | 12 +- crates/nargo_cli/src/cli/prove_cmd.rs | 17 ++- crates/nargo_cli/src/cli/verify_cmd.rs | 2 +- crates/noirc_driver/src/lib.rs | 27 +---- crates/noirc_driver/src/program.rs | 5 +- crates/noirc_errors/Cargo.toml | 1 + crates/noirc_errors/src/debug_info.rs | 39 +++++++ crates/noirc_errors/src/lib.rs | 1 + crates/noirc_errors/src/position.rs | 13 +-- crates/noirc_evaluator/Cargo.toml | 2 - crates/noirc_evaluator/src/lib.rs | 7 +- crates/noirc_evaluator/src/ssa_refactor.rs | 8 +- .../acir_gen/acir_ir/acir_variable.rs | 11 -- .../acir_gen/acir_ir/generated_acir.rs | 4 +- 20 files changed, 149 insertions(+), 125 deletions(-) create mode 100644 crates/noirc_errors/src/debug_info.rs diff --git a/crates/fm/Cargo.toml b/crates/fm/Cargo.toml index 8614671a861..48f1932f9d6 100644 --- a/crates/fm/Cargo.toml +++ b/crates/fm/Cargo.toml @@ -10,6 +10,7 @@ edition.workspace = true codespan-reporting.workspace = true cfg-if.workspace = true rust-embed = "6.6.0" +serde.workspace = true [target.'cfg(target_arch = "wasm32")'.dependencies] wasm-bindgen.workspace = true diff --git a/crates/fm/src/file_map.rs b/crates/fm/src/file_map.rs index bb891e06d79..8bbfcf99dd7 100644 --- a/crates/fm/src/file_map.rs +++ b/crates/fm/src/file_map.rs @@ -1,8 +1,8 @@ +use crate::FileManager; use codespan_reporting::files::{SimpleFile, SimpleFiles}; +use serde::{Deserialize, Serialize}; use std::path::PathBuf; -use crate::FileManager; - // XXX: File and FileMap serve as opaque types, so that the rest of the library does not need to import the dependency // or worry about when we change the dep @@ -34,7 +34,7 @@ impl From<&PathBuf> for PathString { pub struct FileMap(SimpleFiles); // XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId -#[derive(Default, Debug, Clone, PartialEq, Eq, Copy, Hash)] +#[derive(Default, Debug, Clone, PartialEq, Eq, Copy, Hash, Serialize, Deserialize)] pub struct FileId(usize); impl FileId { @@ -46,10 +46,6 @@ impl FileId { pub fn dummy() -> FileId { FileId(0) } - - pub fn new(file_id: usize) -> FileId { - FileId(file_id) - } } pub struct File<'input>(&'input SimpleFile); diff --git a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs index 92af3a99e84..2e0c330366a 100644 --- a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -51,7 +51,7 @@ pub(crate) fn run( (common_reference_string, program) } None => { - let (program, driver) = + let (program, _) = compile_circuit(backend, config.program_dir.as_ref(), &args.compile_options)?; let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 3467202c8af..a228f07cad7 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -2,7 +2,6 @@ use acvm::Backend; use iter_extended::try_vecmap; use nargo::artifacts::contract::PreprocessedContract; use noirc_driver::{CompileOptions, CompiledProgram, Driver, ErrorsAndWarnings, Warnings}; -use noirc_errors::{FileDiagnostic, CustomDiagnostic}; use noirc_errors::reporter::ReportedErrors; use std::path::Path; @@ -90,7 +89,7 @@ pub(crate) fn run( ); } } else { - let (program, driver) = compile_circuit(backend, &config.program_dir, &args.compile_options)?; + let (program, _) = compile_circuit(backend, &config.program_dir, &args.compile_options)?; common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; @@ -109,7 +108,7 @@ pub(crate) fn compile_circuit( backend: &B, program_dir: &Path, compile_options: &CompileOptions, -) -> Result> { +) -> Result<(CompiledProgram, Driver), CliError> { let mut driver = Resolver::resolve_root_manifest(program_dir)?; let result = driver.compile_main( backend.np_language(), diff --git a/crates/nargo_cli/src/cli/execute_cmd.rs b/crates/nargo_cli/src/cli/execute_cmd.rs index f7245a6b7e7..a2b777cc8fb 100644 --- a/crates/nargo_cli/src/cli/execute_cmd.rs +++ b/crates/nargo_cli/src/cli/execute_cmd.rs @@ -1,17 +1,14 @@ -use std::io::StdoutLock; use std::path::Path; -use fm::FileId; - use acvm::acir::{circuit::Circuit, native_types::WitnessMap}; use acvm::Backend; use clap::Args; +use nargo::NargoError; use noirc_abi::input_parser::{Format, InputValue}; use noirc_abi::{Abi, InputMap}; -use noirc_driver::{CompileOptions, CompiledProgram, ErrorsAndWarnings}; -use noirc_errors::{Location, Span, FileDiagnostic, CustomDiagnostic}; +use noirc_driver::{CompileOptions, CompiledProgram, Driver, ErrorsAndWarnings}; +use noirc_errors::{debug_info::DebugInfo, CustomDiagnostic, FileDiagnostic}; -use super::compile_cmd::report_errors; use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; use super::NargoConfig; use crate::{ @@ -65,69 +62,90 @@ fn execute_with_path( let (compiled_program, driver) = compile_circuit(backend, program_dir, compile_options)?; let CompiledProgram { abi, circuit, debug } = compiled_program; - - // Parse the initial witness values from Prover.toml let (inputs_map, _) = read_inputs_from_file(program_dir, prover_name.as_str(), Format::Toml, &abi)?; - let solved_witness_err = execute_program(backend, circuit, &abi, &inputs_map); + let solved_witness_err = execute_program(backend, circuit, &abi, &inputs_map, &debug, &driver); match solved_witness_err { Ok(solved_witness) => { let public_abi = abi.public_abi(); let (_, return_value) = public_abi.decode(&solved_witness)?; - + Ok((return_value, solved_witness)) - }, + } Err(err) => { - match extract_unsatisfied_constraint_error(&err) { - Some(opcode_index) => { - dbg!(&opcode_index); - // let loca = Some(Location::new(Span::inclusive(3,4),FileId::new(0))); - if let Some(loc) = debug.opcode_location( - opcode_index - ) { - dbg!(&loc); - let errs_warnings: ErrorsAndWarnings = vec![FileDiagnostic { - file_id: loc.file, - diagnostic: CustomDiagnostic::simple_error("Unsatisfied constraint".to_string(), "happening on this line".to_string(), loc.span) - }]; - noirc_errors::reporter::report_all(driver.file_manager(), &errs_warnings, false); - } - }, - None => (), - }; + let opcode_idx = extract_unsatisfied_constraint_error(&err); + report_unsatisfied_constraint_error(opcode_idx, &debug, &driver); Err(err) - }, + } } - } -fn extract_unsatisfied_constraint_error(err : &CliError) -> Option{ - +fn extract_unsatisfied_constraint_error(err: &CliError) -> Option { let nargo_err = match err { CliError::NargoError(err) => err, - _=> return None, + _ => return None, }; + extract_unsatisfied_constraint_from_nargo_error(nargo_err) +} - let solving_err= match nargo_err { +fn extract_unsatisfied_constraint_from_nargo_error(nargo_err: &NargoError) -> Option { + let solving_err = match nargo_err { nargo::NargoError::SolvingError(err) => err, - _=> return None, + _ => return None, }; - match solving_err{ - acvm::pwg::OpcodeResolutionError::UnsatisfiedConstrain{opcode_index} => return Some(opcode_index), - _=> return None, + match solving_err { + acvm::pwg::OpcodeResolutionError::UnsatisfiedConstrain { opcode_index } => { + Some(*opcode_index) + } + _ => None, } +} +fn report_unsatisfied_constraint_error( + opcode_idx: Option, + debug: &DebugInfo, + driver: &Driver, +) { + if let Some(opcode_index) = opcode_idx { + if let Some(loc) = debug.opcode_location(opcode_index) { + let errs_warnings: ErrorsAndWarnings = vec![FileDiagnostic { + file_id: loc.file, + diagnostic: CustomDiagnostic::simple_error( + "Unsatisfied constraint".to_string(), + "happening on this line".to_string(), + loc.span, + ), + }]; + noirc_errors::reporter::report_all(driver.file_manager(), &errs_warnings, false); + } + } +} - return None - } - - +pub(crate) fn execute_program( + backend: &B, + circuit: Circuit, + abi: &Abi, + inputs_map: &InputMap, + debug: &DebugInfo, + driver: &Driver, +) -> Result> { + let initial_witness = abi.encode(inputs_map, None)?; + let solved_witness_err = nargo::ops::execute_circuit(backend, circuit, initial_witness); + match solved_witness_err { + Ok(solved_witness) => Ok(solved_witness), + Err(err) => { + let opcode_idx = extract_unsatisfied_constraint_from_nargo_error(&err); + report_unsatisfied_constraint_error(opcode_idx, debug, driver); + Err(crate::errors::CliError::NargoError(err)) + } + } +} -pub(crate) fn execute_program( +pub(crate) fn execute_program_without_debug( backend: &B, circuit: Circuit, abi: &Abi, @@ -136,6 +154,5 @@ pub(crate) fn execute_program( let initial_witness = abi.encode(inputs_map, None)?; let solved_witness = nargo::ops::execute_circuit(backend, circuit, initial_witness)?; - Ok(solved_witness) } diff --git a/crates/nargo_cli/src/cli/gates_cmd.rs b/crates/nargo_cli/src/cli/gates_cmd.rs index 1cf4a251861..030daef9719 100644 --- a/crates/nargo_cli/src/cli/gates_cmd.rs +++ b/crates/nargo_cli/src/cli/gates_cmd.rs @@ -28,7 +28,7 @@ fn count_gates_with_path>( program_dir: P, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let (compiled_program, driver) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let (compiled_program, _) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; let num_opcodes = compiled_program.circuit.opcodes.len(); println!( diff --git a/crates/nargo_cli/src/cli/mod.rs b/crates/nargo_cli/src/cli/mod.rs index f7e909abba8..9fa51e5f3c5 100644 --- a/crates/nargo_cli/src/cli/mod.rs +++ b/crates/nargo_cli/src/cli/mod.rs @@ -73,7 +73,7 @@ pub fn start_cli() -> eyre::Result<()> { NargoCommand::New(args) => new_cmd::run(&backend, args, config), NargoCommand::Check(args) => check_cmd::run(&backend, args, config), NargoCommand::Compile(args) => compile_cmd::run(&backend, args, config), - NargoCommand::Execute(args) =>execute_cmd::run(&backend, args, config), + NargoCommand::Execute(args) => execute_cmd::run(&backend, args, config), NargoCommand::Prove(args) => prove_cmd::run(&backend, args, config), NargoCommand::Verify(args) => verify_cmd::run(&backend, args, config), NargoCommand::Test(args) => test_cmd::run(&backend, args, config), @@ -111,7 +111,15 @@ pub fn prove_and_verify(program_dir: &Path, experimental_ssa: bool) -> bool { ) .expect("Should read inputs"); - execute_cmd::execute_program(&backend, program.circuit, &program.abi, &inputs_map).is_ok() + execute_cmd::execute_program( + &backend, + program.circuit, + &program.abi, + &inputs_map, + &program.debug, + &driver, + ) + .is_ok() } // FIXME: I not sure that this is the right place for this tests. diff --git a/crates/nargo_cli/src/cli/prove_cmd.rs b/crates/nargo_cli/src/cli/prove_cmd.rs index 6e539b32f71..74154d1517b 100644 --- a/crates/nargo_cli/src/cli/prove_cmd.rs +++ b/crates/nargo_cli/src/cli/prove_cmd.rs @@ -22,6 +22,7 @@ use super::{ }; use crate::{ cli::execute_cmd::execute_program, + cli::execute_cmd::execute_program_without_debug, constants::{PROOFS_DIR, PROVER_INPUT_FILE, TARGET_DIR, VERIFIER_INPUT_FILE}, errors::CliError, }; @@ -91,7 +92,7 @@ pub(crate) fn prove_with_path>( ) -> Result, CliError> { let common_reference_string = read_cached_common_reference_string(); - let (common_reference_string, preprocessed_program) = match circuit_build_path { + let (common_reference_string, preprocessed_program, debug, driver) = match circuit_build_path { Some(circuit_build_path) => { let program = read_program_from_file(circuit_build_path)?; let common_reference_string = update_common_reference_string( @@ -100,16 +101,18 @@ pub(crate) fn prove_with_path>( &program.bytecode, ) .map_err(CliError::CommonReferenceStringError)?; - (common_reference_string, program) + (common_reference_string, program, None, None) } None => { - let (program, driver) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let (program, driver) = + compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let debug = program.debug.clone(); let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; let program = preprocess_program(backend, &common_reference_string, program) .map_err(CliError::ProofSystemCompilerError)?; - (common_reference_string, program) + (common_reference_string, program, Some(debug), Some(driver)) } }; @@ -122,7 +125,11 @@ pub(crate) fn prove_with_path>( let (inputs_map, _) = read_inputs_from_file(&program_dir, prover_name.as_str(), Format::Toml, &abi)?; - let solved_witness = execute_program(backend, bytecode.clone(), &abi, &inputs_map)?; + let solved_witness = if let Some(driver) = driver { + execute_program(backend, bytecode.clone(), &abi, &inputs_map, &debug.unwrap(), &driver)? + } else { + execute_program_without_debug(backend, bytecode.clone(), &abi, &inputs_map)? + }; // Write public inputs into Verifier.toml let public_abi = abi.public_abi(); diff --git a/crates/nargo_cli/src/cli/verify_cmd.rs b/crates/nargo_cli/src/cli/verify_cmd.rs index 129debf7b9e..b69ee7545eb 100644 --- a/crates/nargo_cli/src/cli/verify_cmd.rs +++ b/crates/nargo_cli/src/cli/verify_cmd.rs @@ -83,7 +83,7 @@ fn verify_with_path>( (common_reference_string, program) } None => { - let (program, driver) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let (program, _) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 6e46e40c3a1..80b26f70e6b 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -359,7 +359,7 @@ impl Driver { ) -> Result { let program = monomorphize(main_function, &self.context.def_interner); - let (circuit, debug, abi) = if options.experimental_ssa { + let (circuit, mut debug, abi) = if options.experimental_ssa { experimental_create_circuit(program, options.show_ssa, options.show_output)? } else { create_circuit(program, options.show_ssa, options.show_output)? @@ -368,35 +368,14 @@ impl Driver { let abi_len = abi.field_count(); let simplifier = CircuitSimplifier::new(abi_len); - let optimized_circuit = + let (optimized_circuit, opcode_ids) = acvm::compiler::compile(circuit, np_language, is_opcode_supported, &simplifier) .map_err(|_| FileDiagnostic { file_id: FileId::dummy(), diagnostic: CustomDiagnostic::from_message("produced an acvm compile error"), })?; - + debug.update_acir(opcode_ids); Ok(CompiledProgram { circuit: optimized_circuit, debug, abi }) - /* match circuit_abi { - Ok((circuit, debug, abi)) => { - //let mut locations = Vec::new(); - // todo si on veut ca (serialisation), on met la methode dans DebugInfo - // for opcode in loc.keys() { - // if let Some(location) = loc.opcode_location(opcode) { - // let file_id = location.file.as_usize(); - // let start = location.span.start(); - // let end = location.span.end(); - // locations.push((opcode, file_id,start,end)); - // } - // } - //locations.serialize(serializer) - Ok(CompiledProgram { circuit, abi, debug }) - }, - Err(err) => { - // The FileId here will be the file id of the file with the main file - // Errors will be shown at the call site without a stacktrace - Err(err.into()) - } - }*/ } /// Returns a list of all functions in the current crate marked with #[test] diff --git a/crates/noirc_driver/src/program.rs b/crates/noirc_driver/src/program.rs index 5f983b8911d..0e5adf3b39c 100644 --- a/crates/noirc_driver/src/program.rs +++ b/crates/noirc_driver/src/program.rs @@ -1,8 +1,6 @@ -use std::collections::{HashMap, BTreeMap}; - use acvm::acir::circuit::Circuit; -use noirc_evaluator::debug_info::DebugInfo; +use noirc_errors::debug_info::DebugInfo; use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[derive(Debug, Serialize, Deserialize, Clone)] @@ -31,4 +29,3 @@ where let circuit = Circuit::read(&*circuit_bytes).unwrap(); Ok(circuit) } - diff --git a/crates/noirc_errors/Cargo.toml b/crates/noirc_errors/Cargo.toml index ed4c18fd0a2..8ab8420a166 100644 --- a/crates/noirc_errors/Cargo.toml +++ b/crates/noirc_errors/Cargo.toml @@ -11,3 +11,4 @@ codespan-reporting.workspace = true codespan.workspace = true fm.workspace = true chumsky.workspace = true +serde.workspace = true \ No newline at end of file diff --git a/crates/noirc_errors/src/debug_info.rs b/crates/noirc_errors/src/debug_info.rs new file mode 100644 index 00000000000..48e047abb3c --- /dev/null +++ b/crates/noirc_errors/src/debug_info.rs @@ -0,0 +1,39 @@ +use std::collections::HashMap; + +//use noirc_errors::Span; +use serde::{Deserialize, Serialize}; +//use fm::FileId; +use crate::Location; + +#[derive(Default, Debug, Clone, Deserialize, Serialize)] +pub struct DebugInfo { + /// Map opcode index of an ACIR circuit into the source code location + pub locations: HashMap, +} + +impl DebugInfo { + pub fn new(locations: HashMap) -> Self { + DebugInfo { locations } + } + + pub fn update_acir(&mut self, opcode_idx: Vec) { + let mut new_locations = HashMap::new(); + for (i, idx) in opcode_idx.iter().enumerate() { + if self.locations.contains_key(idx) { + new_locations.insert(i, self.locations[idx]); + } + } + self.locations = new_locations; + } + + pub fn opcode_location(&self, idx: usize) -> Option<&Location> { + self.locations.get(&idx) + // if let Some((start, end, file_id)) = self.locations.get(&idx) { + + // let span = Span::exclusive(*start,*end); + // let f = FileId::new(*file_id); + // return Some(Location::new(span, f)); + // } + // None + } +} diff --git a/crates/noirc_errors/src/lib.rs b/crates/noirc_errors/src/lib.rs index 9112f56aece..43d5715fc54 100644 --- a/crates/noirc_errors/src/lib.rs +++ b/crates/noirc_errors/src/lib.rs @@ -3,6 +3,7 @@ #![warn(unreachable_pub)] #![warn(clippy::semicolon_if_nothing_returned)] +pub mod debug_info; mod position; pub mod reporter; pub use position::{Location, Position, Span, Spanned}; diff --git a/crates/noirc_errors/src/position.rs b/crates/noirc_errors/src/position.rs index e16ab669712..09c59da1980 100644 --- a/crates/noirc_errors/src/position.rs +++ b/crates/noirc_errors/src/position.rs @@ -1,6 +1,6 @@ - use codespan::Span as ByteSpan; use fm::FileId; +use serde::{Deserialize, Serialize}; use std::{ hash::{Hash, Hasher}, ops::Range, @@ -51,8 +51,9 @@ impl std::borrow::Borrow for Spanned { } } -#[derive(PartialEq, PartialOrd, Eq, Ord, Hash, Debug, Copy, Clone, Default)] -#[cfg_attr(feature = "serialization", derive(Deserialize, Serialize))] +#[derive( + PartialEq, PartialOrd, Eq, Ord, Hash, Debug, Copy, Clone, Default, Deserialize, Serialize, +)] pub struct Span(ByteSpan); impl Span { @@ -116,8 +117,7 @@ impl chumsky::Span for Span { } } -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] -#[cfg_attr(feature = "serialization", derive(Deserialize, Serialize))] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Deserialize, Serialize)] pub struct Location { pub span: Span, pub file: FileId, @@ -127,7 +127,4 @@ impl Location { pub fn new(span: Span, file: FileId) -> Self { Self { span, file } } - pub fn flatten(&self) -> (u32, u32, usize) { - (self.span.start(), self.span.end(), self.file.as_usize()) - } } diff --git a/crates/noirc_evaluator/Cargo.toml b/crates/noirc_evaluator/Cargo.toml index e03f3c6de55..b2a01b142d0 100644 --- a/crates/noirc_evaluator/Cargo.toml +++ b/crates/noirc_evaluator/Cargo.toml @@ -11,9 +11,7 @@ noirc_frontend.workspace = true noirc_errors.workspace = true noirc_abi.workspace = true acvm.workspace = true -fm.workspace = true arena.workspace = true -serde.workspace = true iter-extended.workspace = true thiserror.workspace = true num-bigint = "0.4" diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index fde73161350..79849525747 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -11,20 +11,19 @@ mod ssa; pub mod ssa_refactor; pub mod brillig; -pub mod debug_info; use acvm::{ acir::circuit::{opcodes::Opcode as AcirOpcode, Circuit, PublicInputs}, acir::native_types::{Expression, Witness}, }; -use debug_info::DebugInfo; + use errors::{RuntimeError, RuntimeErrorKind}; use iter_extended::vecmap; use noirc_abi::{Abi, AbiType, AbiVisibility}; -use noirc_errors::{Location}; +use noirc_errors::debug_info::DebugInfo; use noirc_frontend::monomorphization::ast::*; use ssa::{node::ObjectType, ssa_gen::IrGenerator}; -use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::collections::{BTreeMap, BTreeSet}; #[derive(Default)] pub struct Evaluator { diff --git a/crates/noirc_evaluator/src/ssa_refactor.rs b/crates/noirc_evaluator/src/ssa_refactor.rs index 007f338d44b..23713b9c10a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor.rs +++ b/crates/noirc_evaluator/src/ssa_refactor.rs @@ -10,7 +10,7 @@ use crate::errors::RuntimeError; use acvm::acir::circuit::{Circuit, PublicInputs}; -use crate::{debug_info::DebugInfo}; +use noirc_errors::debug_info::DebugInfo; use noirc_abi::Abi; @@ -76,13 +76,9 @@ pub fn experimental_create_circuit( PublicInputs(public_abi.param_witnesses.values().flatten().copied().collect()); let return_values = PublicInputs(return_witnesses.into_iter().collect()); - let circuit = Circuit { current_witness_index, opcodes, public_parameters, return_values }; + let debug_info = DebugInfo::new(locations); - //dbg!(&locations); - let mut debug_info = DebugInfo::new(locations); - //let opcode_ids = (0..optimized_circuit.opcodes.len()).collect(); - //debug_info.update_acir(opcode_ids); Ok((circuit, debug_info, abi)) } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs index 1d1c90ee5d4..d5b63dfce0c 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs @@ -766,17 +766,6 @@ impl AcirContext { /// Terminates the context and takes the resulting `GeneratedAcir` pub(crate) fn finish(self) -> GeneratedAcir { - // let mut diag = Vec::new(); - // for op in self.acir_ir.locations.keys() { - // diag.push( - // Diagnostic::simple_error( - // format!("opcode:{}", op), - // self.acir_ir.opcodes[*op].to_string(), - // self.acir_ir.locations[op], - // )); - // } - - self.acir_ir } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs index 2fc38d18d0f..9930b8e266d 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs @@ -41,7 +41,7 @@ pub(crate) struct GeneratedAcir { pub(crate) return_witnesses: Vec, /// Correspondance between an opcode index (in opcodes) and the source code location which generated it - pub(crate) locations: HashMap, + pub(crate) locations: HashMap, /// Source code location of the current instruction being processed /// None if we do not know the location @@ -58,7 +58,7 @@ impl GeneratedAcir { fn push_opcode(&mut self, opcode: AcirOpcode) { self.opcodes.push(opcode); if let Some(location) = self.current_location { - self.locations.insert(self.opcodes.len() - 1, location.flatten()); + self.locations.insert(self.opcodes.len() - 1, location); } } From f338a72633c1602e57cdea8093bb82787d95c4f5 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 6 Jul 2023 15:31:23 +0000 Subject: [PATCH 05/23] update cargo.toml --- Cargo.lock | 10 ++++------ Cargo.toml | 3 +-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9f701208991..0da0b294913 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -699,6 +699,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3362992a0d9f1dd7c3d0e89e0ab2bb540b7a95fea8cd798090e758fda2899b5e" dependencies = [ "codespan-reporting", + "serde", ] [[package]] @@ -718,6 +719,7 @@ version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3538270d33cc669650c4b093848450d380def10c331d38c768e34cac80576e6e" dependencies = [ + "serde", "termcolor", "unicode-width", ] @@ -1220,6 +1222,7 @@ dependencies = [ "cfg-if", "codespan-reporting", "rust-embed", + "serde", "tempfile", "wasm-bindgen", ] @@ -2069,6 +2072,7 @@ dependencies = [ "codespan", "codespan-reporting", "fm", + "serde", ] [[package]] @@ -2077,7 +2081,6 @@ version = "0.7.1" dependencies = [ "acvm", "arena", - "fm", "im", "iter-extended", "noirc_abi", @@ -2086,7 +2089,6 @@ dependencies = [ "num-bigint", "num-traits", "rand 0.8.5", - "serde", "thiserror", ] @@ -4221,7 +4223,3 @@ dependencies = [ "quote", "syn 2.0.18", ] - -[[patch.unused]] -name = "acvm" -version = "0.16.0" diff --git a/Cargo.toml b/Cargo.toml index e23384c48ea..0987c3d3e90 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,7 +40,7 @@ noir_wasm = { path = "crates/wasm" } cfg-if = "1.0.0" clap = { version = "4.1.4", features = ["derive"] } -codespan = "0.11.1" +codespan = {version = "0.11.1", features = ["serialization"]} codespan-lsp = "0.11.1" codespan-reporting = "0.11.1" chumsky = { git = "https://github.com/jfecher/chumsky", rev = "ad9d312" } @@ -58,4 +58,3 @@ wasm-bindgen-test = "0.3.33" [patch.crates-io] async-lsp = { git = "https://github.com/oxalica/async-lsp", rev = "09dbcc11046f7a188a80137f8d36484d86c78c78" } -acvm = { path = "../../noir-4commit/acvm-master/acvm/acvm" } From 3bf33a54fd9644c677e221494d45d4d2cb3ee0f9 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 6 Jul 2023 15:51:32 +0000 Subject: [PATCH 06/23] clean-up: restore unit tests --- .../test_data_ssa_refactor/2_div/src/main.nr | 2 +- .../arithmetic_binary_operations/src/main.nr | 21 +++------ .../array_len/src/main.nr | 32 ------------- .../brillig_conditional/src/main.nr | 45 ++++++++++++------- .../brillig_modulo/src/main.nr | 3 +- .../brillig_not/Prover.toml | 2 +- .../brillig_not/src/main.nr | 9 +--- .../test_data_ssa_refactor/sha256/src/main.nr | 14 +++--- 8 files changed, 43 insertions(+), 85 deletions(-) diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/2_div/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/2_div/src/main.nr index f9756fd1977..ff0dee755cc 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/2_div/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/2_div/src/main.nr @@ -1,7 +1,7 @@ // Testing integer division: 7/3 = 2 fn main(mut x: u32, y: u32, z: u32) { let a = x % y; - assert(x / y == z+1); + assert(x / y == z); assert(a == x - z*y); assert((50 as u64) % (9 as u64) == 5); } diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr index 357bf268b07..4327685a27c 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr @@ -5,20 +5,9 @@ use dep::std; // Binary addition, multiplication, division // x = 3, y = 4, z = 5 fn main(x : Field, y : Field, z : Field) -> pub Field { -// let a = x + x; // 3 + 3 = 6 -// let b = a - y; // 6 - 4 = 2 -// let c = b * z; // 2 * 5 = 10 -// let d = c / a; // 10 / 6 (This uses field inversion, so we test it by multiplying by `a`) -// d * a -let toto = y+z; -assert(toto==toto); - let b = x.to_le_bits_unconstrained(8); - let c = x.to_le_bits(8); - let a:[u1;8] = x.toto(); - assert(a[0]==0); - // assert(b[0]==c[0]); - // assert(b[1]==c[1]); - std::println(b); - std::println(c); - 0 + let a = x + x; // 3 + 3 = 6 + let b = a - y; // 6 - 4 = 2 + let c = b * z; // 2 * 5 = 10 + let d = c / a; // 10 / 6 (This uses field inversion, so we test it by multiplying by `a`) + d * a } \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr index 53398171cee..101957b03b7 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr @@ -12,40 +12,8 @@ fn nested_call(b: [Field]) -> Field { len_plus_1(b) } -fn bar(mut a: [Field]) -> [Field] { - // a.push_back(a[1]); - a -} - -fn foo(mut a: [Field]) -> Field{ - let mut d = -(a[0] as i32); - d = -5; - //let mut b=a; - let mut sum = 0; -let mut b:[Field] = [0,0];// hum.. -let k = b.push_back(3); - if a[0]==0 { - a.push_back(10); - } else { - - } - a = bar(a); - // let c=[0;a.len()]; marche po - for i in 0..a.len()-1 { - // b[i] = a[i]; - b.push_back(a[i]); - } - // for i in b { - // sum = sum+i; - // } - // sum - a[2] -} fn main(len3: [u8; 3], len4: [Field; 4]) { - - let toto = foo(len4); - std::println(toto); assert(len_plus_1(len3) == 4); assert(len_plus_1(len4) == 5); assert(add_lens(len3, len4) == 7); diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr index 088022981ca..8de3ba57935 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr @@ -1,24 +1,35 @@ // Tests a very simple program. // -// The features being tested is basic conditonal on brillig -fn main(x: Field) { - assert(5 == conditional(x as bool)); +// The features being tested is brillig calls with conditionals +fn main(x: [u32; 3]) { + assert(entry_point(x[0]) == 7); + assert(entry_point(x[1]) == 8); + assert(entry_point(x[2]) == 9); + assert(entry_point(42) == 0); } -unconstrained fn conditional(x : bool) -> Field { - // if x { - // 4 - // }else { - // 5 - // } - let a = toto(x); - a + 1 +unconstrained fn inner_1() -> u32 { + 7 } -unconstrained fn toto(x : bool) -> Field { - if x { - 4 - }else { - 5 - } +unconstrained fn inner_2() -> u32 { + 8 +} + +unconstrained fn inner_3() -> u32 { + 9 +} + +unconstrained fn entry_point(x: u32) -> u32 { + let mut result: u32 = 0; + + if x == 1 { + result = inner_1(); + } else if x == 2 { + result = inner_2(); + } else if x == 3 { + result = inner_3(); + } + + result } \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr index 3c63e5949d3..5104916a3ea 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr @@ -1,9 +1,8 @@ -use dep::std;// Tests a very simple program. +// Tests a very simple program. // // The features being tested is modulo operations on brillig fn main() { - // std::println(modulo(47, 3)); assert(modulo(47, 3) == 2); assert(modulo(2, 3) == 2); assert(signed_modulo(5, 3) == 2); diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/Prover.toml index 63016ec4ef2..a0397e89477 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/Prover.toml +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/Prover.toml @@ -1,2 +1,2 @@ x = "1" -y = "27" +y = "0" diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/src/main.nr index 0b814c8b14c..bc94810efb9 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_not/src/main.nr @@ -2,15 +2,10 @@ // // The features being tested is not instruction on brillig fn main(x: Field, y : Field) { - assert(!(x as bool) == not_operator(x as bool)); - assert((x as bool)== not_operator(!(x as bool))); - assert(!(y as u8) == not_operator_8bit(y as u8)); + assert(false == not_operator(x as bool)); + assert(true == not_operator(y as bool)); } unconstrained fn not_operator(x : bool) -> bool { !x -} - -unconstrained fn not_operator_8bit(x : u8) -> u8 { - !x } \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr index 87ebbcdb92b..f758b8e7e28 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr @@ -11,13 +11,9 @@ // This can be done in ACIR! use dep::std; -// fn main(x: Field, result: [u8; 32]) { -// // We use the `as` keyword here to denote the fact that we want to take just the first byte from the x Field -// // The padding is taken care of by the program -// let digest = std::hash::sha256([x as u8]); -// assert(digest == result); -// } - -fn main(message : [u8;38],hashed_message : [u8;32]) { - assert(hashed_message == std::hash::sha256(message)); +fn main(x: Field, result: [u8; 32]) { + // We use the `as` keyword here to denote the fact that we want to take just the first byte from the x Field + // The padding is taken care of by the program + let digest = std::hash::sha256([x as u8]); + assert(digest == result); } \ No newline at end of file From f52d26e1f0243e16985a4c000466539d31630210 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 6 Jul 2023 15:52:54 +0000 Subject: [PATCH 07/23] restore the prover.toml --- .../test_data_ssa_refactor/sha256/Prover.toml | 137 ++++-------------- 1 file changed, 31 insertions(+), 106 deletions(-) diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml index ae1395a9a4b..6b17aaf482c 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml @@ -1,111 +1,36 @@ -# x = 0xbd -# result = [ -# 0x68, -# 0x32, -# 0x57, -# 0x20, -# 0xaa, -# 0xbd, -# 0x7c, -# 0x82, -# 0xf3, -# 0x0f, -# 0x55, -# 0x4b, -# 0x31, -# 0x3d, -# 0x05, -# 0x70, -# 0xc9, -# 0x5a, -# 0xcc, -# 0xbb, -# 0x7d, -# 0xc4, -# 0xb5, -# 0xaa, -# 0xe1, -# 0x12, -# 0x04, -# 0xc0, -# 0x8f, -# 0xfe, -# 0x73, -# 0x2b, -# ] - -hashed_message = [ - 0x3a, - 0x73, - 0xf4, - 0x12, - 0x3a, - 0x5c, - 0xd2, - 0x12, - 0x1f, - 0x21, - 0xcd, - 0x7e, - 0x8d, - 0x35, - 0x88, - 0x35, - 0x47, - 0x69, - 0x49, - 0xd0, - 0x35, - 0xd9, - 0xc2, - 0xda, +x = 0xbd +result = [ 0x68, - 0x06, - 0xb4, - 0x63, - 0x3a, - 0xc8, - 0xc1, - 0xe2, -] -message = [ - 0x49, - 0x6e, - 0x73, - 0x74, - 0x72, - 0x75, - 0x63, - 0x74, - 0x69, - 0x6f, - 0x6e, - 0x73, - 0x20, - 0x75, - 0x6e, - 0x63, - 0x6c, - 0x65, - 0x61, - 0x72, - 0x2c, + 0x32, + 0x57, 0x20, - 0x61, + 0xaa, + 0xbd, + 0x7c, + 0x82, + 0xf3, + 0x0f, + 0x55, + 0x4b, + 0x31, + 0x3d, + 0x05, + 0x70, + 0xc9, + 0x5a, + 0xcc, + 0xbb, + 0x7d, + 0xc4, + 0xb5, + 0xaa, + 0xe1, + 0x12, + 0x04, + 0xc0, + 0x8f, + 0xfe, 0x73, - 0x6b, - 0x20, - 0x61, - 0x67, - 0x61, - 0x69, - 0x6e, - 0x20, - 0x6c, - 0x61, - 0x74, - 0x65, - 0x72, - 0x2e, + 0x2b, ] \ No newline at end of file From 20a34bf7abe8616a42d0151f34410e80f23ecd4a Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 7 Jul 2023 10:26:25 +0000 Subject: [PATCH 08/23] notify unsatisfied constraint only once --- crates/nargo_cli/src/cli/compile_cmd.rs | 9 ++++-- crates/nargo_cli/src/cli/execute_cmd.rs | 39 +++++++------------------ crates/nargo_cli/src/cli/prove_cmd.rs | 8 ++--- crates/noirc_driver/src/lib.rs | 2 +- 4 files changed, 23 insertions(+), 35 deletions(-) diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 0cd1e420113..c7f76f0d8ad 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -112,7 +112,7 @@ pub(crate) fn compile_circuit( backend: &B, program_dir: &Path, compile_options: &CompileOptions, -) -> Result> { +) -> Result<(CompiledProgram, Context), CliError> { let mut context = resolve_root_manifest(program_dir)?; let result = compile_main( &mut context, @@ -120,7 +120,12 @@ pub(crate) fn compile_circuit( &|op| backend.supports_opcode(op), compile_options, ); - report_errors(result, &context, compile_options.deny_warnings).map_err(Into::into) + let result: Result> = + report_errors(result, &context, compile_options.deny_warnings).map_err(Into::into); + match result { + Ok(program) => Ok((program, context)), + Err(err) => Err(err), + } } /// Helper function for reporting any errors in a Result<(T, Warnings), ErrorsAndWarnings> diff --git a/crates/nargo_cli/src/cli/execute_cmd.rs b/crates/nargo_cli/src/cli/execute_cmd.rs index a2b777cc8fb..4ffa6a20728 100644 --- a/crates/nargo_cli/src/cli/execute_cmd.rs +++ b/crates/nargo_cli/src/cli/execute_cmd.rs @@ -6,8 +6,9 @@ use clap::Args; use nargo::NargoError; use noirc_abi::input_parser::{Format, InputValue}; use noirc_abi::{Abi, InputMap}; -use noirc_driver::{CompileOptions, CompiledProgram, Driver, ErrorsAndWarnings}; +use noirc_driver::{CompileOptions, CompiledProgram, ErrorsAndWarnings}; use noirc_errors::{debug_info::DebugInfo, CustomDiagnostic, FileDiagnostic}; +use noirc_frontend::hir::Context; use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; use super::NargoConfig; @@ -59,36 +60,18 @@ fn execute_with_path( prover_name: String, compile_options: &CompileOptions, ) -> Result<(Option, WitnessMap), CliError> { - let (compiled_program, driver) = compile_circuit(backend, program_dir, compile_options)?; + let (compiled_program, context) = compile_circuit(backend, program_dir, compile_options)?; let CompiledProgram { abi, circuit, debug } = compiled_program; // Parse the initial witness values from Prover.toml let (inputs_map, _) = read_inputs_from_file(program_dir, prover_name.as_str(), Format::Toml, &abi)?; - let solved_witness_err = execute_program(backend, circuit, &abi, &inputs_map, &debug, &driver); + let solved_witness = execute_program(backend, circuit, &abi, &inputs_map, &debug, &context)?; + let public_abi = abi.public_abi(); + let (_, return_value) = public_abi.decode(&solved_witness)?; - match solved_witness_err { - Ok(solved_witness) => { - let public_abi = abi.public_abi(); - let (_, return_value) = public_abi.decode(&solved_witness)?; - - Ok((return_value, solved_witness)) - } - Err(err) => { - let opcode_idx = extract_unsatisfied_constraint_error(&err); - report_unsatisfied_constraint_error(opcode_idx, &debug, &driver); - Err(err) - } - } -} - -fn extract_unsatisfied_constraint_error(err: &CliError) -> Option { - let nargo_err = match err { - CliError::NargoError(err) => err, - _ => return None, - }; - extract_unsatisfied_constraint_from_nargo_error(nargo_err) + Ok((return_value, solved_witness)) } fn extract_unsatisfied_constraint_from_nargo_error(nargo_err: &NargoError) -> Option { @@ -107,7 +90,7 @@ fn extract_unsatisfied_constraint_from_nargo_error(nargo_err: &NargoError) -> Op fn report_unsatisfied_constraint_error( opcode_idx: Option, debug: &DebugInfo, - driver: &Driver, + context: &Context, ) { if let Some(opcode_index) = opcode_idx { if let Some(loc) = debug.opcode_location(opcode_index) { @@ -119,7 +102,7 @@ fn report_unsatisfied_constraint_error( loc.span, ), }]; - noirc_errors::reporter::report_all(driver.file_manager(), &errs_warnings, false); + noirc_errors::reporter::report_all(&context.file_manager, &errs_warnings, false); } } } @@ -130,7 +113,7 @@ pub(crate) fn execute_program( abi: &Abi, inputs_map: &InputMap, debug: &DebugInfo, - driver: &Driver, + context: &Context, ) -> Result> { let initial_witness = abi.encode(inputs_map, None)?; @@ -139,7 +122,7 @@ pub(crate) fn execute_program( Ok(solved_witness) => Ok(solved_witness), Err(err) => { let opcode_idx = extract_unsatisfied_constraint_from_nargo_error(&err); - report_unsatisfied_constraint_error(opcode_idx, debug, driver); + report_unsatisfied_constraint_error(opcode_idx, debug, context); Err(crate::errors::CliError::NargoError(err)) } } diff --git a/crates/nargo_cli/src/cli/prove_cmd.rs b/crates/nargo_cli/src/cli/prove_cmd.rs index 74154d1517b..07b4ea9967e 100644 --- a/crates/nargo_cli/src/cli/prove_cmd.rs +++ b/crates/nargo_cli/src/cli/prove_cmd.rs @@ -92,7 +92,7 @@ pub(crate) fn prove_with_path>( ) -> Result, CliError> { let common_reference_string = read_cached_common_reference_string(); - let (common_reference_string, preprocessed_program, debug, driver) = match circuit_build_path { + let (common_reference_string, preprocessed_program, debug, context) = match circuit_build_path { Some(circuit_build_path) => { let program = read_program_from_file(circuit_build_path)?; let common_reference_string = update_common_reference_string( @@ -104,7 +104,7 @@ pub(crate) fn prove_with_path>( (common_reference_string, program, None, None) } None => { - let (program, driver) = + let (program, context) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; let debug = program.debug.clone(); let common_reference_string = @@ -112,7 +112,7 @@ pub(crate) fn prove_with_path>( .map_err(CliError::CommonReferenceStringError)?; let program = preprocess_program(backend, &common_reference_string, program) .map_err(CliError::ProofSystemCompilerError)?; - (common_reference_string, program, Some(debug), Some(driver)) + (common_reference_string, program, Some(debug), Some(context)) } }; @@ -125,7 +125,7 @@ pub(crate) fn prove_with_path>( let (inputs_map, _) = read_inputs_from_file(&program_dir, prover_name.as_str(), Format::Toml, &abi)?; - let solved_witness = if let Some(driver) = driver { + let solved_witness = if let Some(driver) = context { execute_program(backend, bytecode.clone(), &abi, &inputs_map, &debug.unwrap(), &driver)? } else { execute_program_without_debug(backend, bytecode.clone(), &abi, &inputs_map)? diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index de0df6aa2a1..25947a52fb3 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -328,7 +328,7 @@ pub fn compile_no_check( ) -> Result { let program = monomorphize(main_function, &context.def_interner); - let (circuit, debug, abi) = if options.experimental_ssa { + let (circuit, mut debug, abi) = if options.experimental_ssa { experimental_create_circuit(program, options.show_ssa, options.show_output)? } else { create_circuit(program, options.show_ssa, options.show_output)? From b86380692888ba5508181c1973aa7ba8b3f05d47 Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 7 Jul 2023 12:34:42 +0000 Subject: [PATCH 09/23] set the location and use it when inserting an instruction --- .../src/ssa_refactor/opt/inlining.rs | 6 +- .../src/ssa_refactor/ssa_builder/mod.rs | 131 ++++-------------- .../src/ssa_refactor/ssa_gen/context.rs | 53 +++---- .../src/ssa_refactor/ssa_gen/mod.rs | 14 +- 4 files changed, 52 insertions(+), 152 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs index 6de190a56a1..230517ce7f2 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs @@ -387,8 +387,10 @@ impl<'function> PerFunctionContext<'function> { .requires_ctrl_typevars() .then(|| vecmap(&results, |result| self.source_function.dfg.type_of_value(*result))); - let new_results = - self.context.builder.insert_instruction(instruction, ctrl_typevars, location); + if let Some(location) = location { + self.context.builder.set_location(location); + } + let new_results = self.context.builder.insert_instruction(instruction, ctrl_typevars); Self::insert_new_instruction_results(&mut self.values, &results, new_results); } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs index f209b35d20c..deb0053ad59 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs @@ -33,6 +33,7 @@ pub(crate) struct FunctionBuilder { pub(super) current_function: Function, current_block: BasicBlockId, finished_functions: Vec, + current_location: Option, } impl FunctionBuilder { @@ -49,7 +50,12 @@ impl FunctionBuilder { new_function.set_runtime(runtime); let current_block = new_function.entry_block(); - Self { current_function: new_function, current_block, finished_functions: Vec::new() } + Self { + current_function: new_function, + current_block, + finished_functions: Vec::new(), + current_location: None, + } } /// Finish the current function and create a new function. @@ -144,13 +150,12 @@ impl FunctionBuilder { &mut self, instruction: Instruction, ctrl_typevars: Option>, - location: Option, ) -> InsertInstructionResult { self.current_function.dfg.insert_instruction_and_results( instruction, self.current_block, ctrl_typevars, - location, + self.current_location, ) } @@ -170,7 +175,12 @@ impl FunctionBuilder { /// given amount of field elements. Returns the result of the allocate instruction, /// which is always a Reference to the allocated data. pub(crate) fn insert_allocate(&mut self) -> ValueId { - self.insert_instruction(Instruction::Allocate, None, None).first() + self.insert_instruction(Instruction::Allocate, None).first() + } + + pub(crate) fn set_location(&mut self, location: Location) -> &mut FunctionBuilder { + self.current_location = Some(location); + self } /// Insert a Load instruction at the end of the current block, loading from the given offset @@ -181,41 +191,14 @@ impl FunctionBuilder { /// an array will have an offset of 3. /// Returns the element that was loaded. pub(crate) fn insert_load(&mut self, address: ValueId, type_to_load: Type) -> ValueId { - self.insert_instruction(Instruction::Load { address }, Some(vec![type_to_load]), None) - .first() + self.insert_instruction(Instruction::Load { address }, Some(vec![type_to_load])).first() } - // Same as insert_load(), but additionally provides the source location of the load - pub(crate) fn insert_load_with_source_location( - &mut self, - address: ValueId, - type_to_load: Type, - location: Location, - ) -> ValueId { - self.insert_instruction( - Instruction::Load { address }, - Some(vec![type_to_load]), - Some(location), - ) - .first() - } /// Insert a Store instruction at the end of the current block, storing the given element /// at the given address. Expects that the address points somewhere /// within a previous Allocate instruction. pub(crate) fn insert_store(&mut self, address: ValueId, value: ValueId) { - self.insert_instruction(Instruction::Store { address, value }, None, None); - } - - /// Insert a Store instruction at the end of the current block, storing the given element - /// at the given address. Expects that the address points somewhere - /// within a previous Allocate instruction. - pub(crate) fn insert_store_with_source_location( - &mut self, - address: ValueId, - value: ValueId, - location: Location, - ) { - self.insert_instruction(Instruction::Store { address, value }, None, Some(location)); + self.insert_instruction(Instruction::Store { address, value }, None); } /// Insert a binary instruction at the end of the current block. @@ -227,32 +210,20 @@ impl FunctionBuilder { rhs: ValueId, ) -> ValueId { let instruction = Instruction::Binary(Binary { lhs, rhs, operator }); - self.insert_instruction(instruction, None, None).first() + self.insert_instruction(instruction, None).first() } - /// Insert a binary instruction at the end of the current block. - /// Returns the result of the binary instruction. - pub(crate) fn insert_binary_with_source_location( - &mut self, - lhs: ValueId, - operator: BinaryOp, - rhs: ValueId, - location: Option, - ) -> ValueId { - let instruction = Instruction::Binary(Binary { lhs, rhs, operator }); - self.insert_instruction(instruction, None, location).first() - } /// Insert a not instruction at the end of the current block. /// Returns the result of the instruction. - pub(crate) fn insert_not(&mut self, rhs: ValueId, location: Option) -> ValueId { - self.insert_instruction(Instruction::Not(rhs), None, location).first() + pub(crate) fn insert_not(&mut self, rhs: ValueId) -> ValueId { + self.insert_instruction(Instruction::Not(rhs), None).first() } /// Insert a cast instruction at the end of the current block. /// Returns the result of the cast instruction. pub(crate) fn insert_cast(&mut self, value: ValueId, typ: Type) -> ValueId { - self.insert_instruction(Instruction::Cast(value, typ), None, None).first() + self.insert_instruction(Instruction::Cast(value, typ), None).first() } /// Insert a truncate instruction at the end of the current block. @@ -262,28 +233,14 @@ impl FunctionBuilder { value: ValueId, bit_size: u32, max_bit_size: u32, - location: Location, ) -> ValueId { - self.insert_instruction( - Instruction::Truncate { value, bit_size, max_bit_size }, - None, - Some(location), - ) - .first() + self.insert_instruction(Instruction::Truncate { value, bit_size, max_bit_size }, None) + .first() } /// Insert a constrain instruction at the end of the current block. pub(crate) fn insert_constrain(&mut self, boolean: ValueId) { - self.insert_instruction(Instruction::Constrain(boolean), None, None); - } - - /// Insert a constrain instruction at the end of the current block. - pub(crate) fn insert_constrain_with_source_location( - &mut self, - boolean: ValueId, - location: Location, - ) { - self.insert_instruction(Instruction::Constrain(boolean), None, Some(location)); + self.insert_instruction(Instruction::Constrain(boolean), None); } /// Insert a call instruction at the end of the current block and return @@ -294,25 +251,7 @@ impl FunctionBuilder { arguments: Vec, result_types: Vec, ) -> Cow<[ValueId]> { - self.insert_instruction(Instruction::Call { func, arguments }, Some(result_types), None) - .results() - } - - /// Insert a call instruction at the end of the current block and return - /// the results of the call. - pub(crate) fn insert_call_with_source_location( - &mut self, - func: ValueId, - arguments: Vec, - result_types: Vec, - location: Location, - ) -> Cow<[ValueId]> { - self.insert_instruction( - Instruction::Call { func, arguments }, - Some(result_types), - Some(location), - ) - .results() + self.insert_instruction(Instruction::Call { func, arguments }, Some(result_types)).results() } /// Insert an instruction to extract an element from an array @@ -323,25 +262,7 @@ impl FunctionBuilder { element_type: Type, ) -> ValueId { let element_type = Some(vec![element_type]); - self.insert_instruction(Instruction::ArrayGet { array, index }, element_type, None).first() - } - - /// Insert an instruction to extract an element from an array - /// Provide the source code location - pub(crate) fn insert_array_get_with_source_location( - &mut self, - array: ValueId, - index: ValueId, - element_type: Type, - location: Location, - ) -> ValueId { - let element_type = Some(vec![element_type]); - self.insert_instruction( - Instruction::ArrayGet { array, index }, - element_type, - Some(location), - ) - .first() + self.insert_instruction(Instruction::ArrayGet { array, index }, element_type).first() } /// Insert an instruction to create a new array with the given index replaced with a new value @@ -351,7 +272,7 @@ impl FunctionBuilder { index: ValueId, value: ValueId, ) -> ValueId { - self.insert_instruction(Instruction::ArraySet { array, index, value }, None, None).first() + self.insert_instruction(Instruction::ArraySet { array, index, value }, None).first() } /// Terminates the current block with the given terminator instruction 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 e15a6a5e79f..ef755d5e7d3 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs @@ -246,8 +246,7 @@ impl<'a> FunctionContext<'a> { std::mem::swap(&mut lhs, &mut rhs); } - let mut result = - self.builder.insert_binary_with_source_location(lhs, op, rhs, Some(location)); + let mut result = self.builder.set_location(location).insert_binary(lhs, op, rhs); if let Some(max_bit_size) = operator_result_max_bit_size_to_truncate( operator, @@ -263,11 +262,11 @@ impl<'a> FunctionContext<'a> { unreachable!("ICE: Truncation attempted on non-integer"); } }; - result = self.builder.insert_truncate(result, bit_size, max_bit_size, location); + result = self.builder.insert_truncate(result, bit_size, max_bit_size); } if operator_requires_not(operator) { - result = self.builder.insert_not(result, Some(location)); + result = self.builder.insert_not(result); } result.into() } @@ -327,7 +326,7 @@ impl<'a> FunctionContext<'a> { // pre-loop let result_alloc = self.builder.insert_allocate(); let true_value = self.builder.numeric_constant(1u128, Type::bool()); - self.builder.insert_store_with_source_location(result_alloc, true_value, location); + self.builder.set_location(location).insert_store(result_alloc, true_value); let zero = self.builder.field_constant(0u128); self.builder.terminate_with_jmp(loop_start, vec![zero]); @@ -335,42 +334,28 @@ impl<'a> FunctionContext<'a> { self.builder.switch_to_block(loop_start); let i = self.builder.add_block_parameter(loop_start, Type::field()); let array_length = self.builder.field_constant(array_length as u128); - let v0 = self.builder.insert_binary_with_source_location( - i, - BinaryOp::Lt, - array_length, - Some(location), - ); + let v0 = self.builder.insert_binary(i, BinaryOp::Lt, array_length); self.builder.terminate_with_jmpif(v0, loop_body, loop_end); // loop body self.builder.switch_to_block(loop_body); - let v1 = self.builder.insert_array_get_with_source_location( - lhs, - i, - element_type.clone(), - location, - ); - let v2 = self.builder.insert_array_get_with_source_location(rhs, i, element_type, location); - let v3 = - self.builder.insert_binary_with_source_location(v1, BinaryOp::Eq, v2, Some(location)); - let v4 = - self.builder.insert_load_with_source_location(result_alloc, Type::bool(), location); - let v5 = - self.builder.insert_binary_with_source_location(v4, BinaryOp::And, v3, Some(location)); - self.builder.insert_store_with_source_location(result_alloc, v5, location); + let v1 = self.builder.insert_array_get(lhs, i, element_type.clone()); + let v2 = self.builder.insert_array_get(rhs, i, element_type); + let v3 = self.builder.insert_binary(v1, BinaryOp::Eq, v2); + let v4 = self.builder.insert_load(result_alloc, Type::bool()); + let v5 = self.builder.insert_binary(v4, BinaryOp::And, v3); + self.builder.insert_store(result_alloc, v5); let one = self.builder.field_constant(1u128); - let v6 = - self.builder.insert_binary_with_source_location(i, BinaryOp::Add, one, Some(location)); + let v6 = self.builder.insert_binary(i, BinaryOp::Add, one); + //insert_binary_with_source_location(i, BinaryOp::Add, one, Some(location)); self.builder.terminate_with_jmp(loop_start, vec![v6]); // loop end self.builder.switch_to_block(loop_end); - let mut result = - self.builder.insert_load_with_source_location(result_alloc, Type::bool(), location); + let mut result = self.builder.insert_load(result_alloc, Type::bool()); if operator_requires_not(operator) { - result = self.builder.insert_not(result, Some(location)); + result = self.builder.insert_not(result); } result.into() } @@ -388,12 +373,8 @@ impl<'a> FunctionContext<'a> { location: Location, ) -> Values { let result_types = Self::convert_type(result_type).flatten(); - let results = self.builder.insert_call_with_source_location( - function, - arguments, - result_types, - location, - ); + let results = + self.builder.set_location(location).insert_call(function, arguments, result_types); let mut i = 0; let reshaped_return_values = Self::map_type(result_type, |_| { 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 0bbda3c3417..13e67f26cc5 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs @@ -169,7 +169,7 @@ impl<'a> FunctionContext<'a> { match unary.operator { noirc_frontend::UnaryOp::Not => { let rhs = rhs.into_leaf().eval(self); - self.builder.insert_not(rhs, None).into() + self.builder.insert_not(rhs).into() } noirc_frontend::UnaryOp::Minus => { let rhs = rhs.into_leaf().eval(self); @@ -223,18 +223,14 @@ impl<'a> FunctionContext<'a> { // base_index = index * type_size let type_size = Self::convert_type(element_type).size_of_type(); let type_size = self.builder.field_constant(type_size as u128); - let base_index = self.builder.insert_binary_with_source_location( - index, - BinaryOp::Mul, - type_size, - Some(location), - ); + let base_index = + self.builder.set_location(location).insert_binary(index, BinaryOp::Mul, type_size); let mut field_index = 0u128; Self::map_type(element_type, |typ| { let offset = self.make_offset(base_index, field_index); field_index += 1; - self.builder.insert_array_get_with_source_location(array, offset, typ, location).into() + self.builder.insert_array_get(array, offset, typ).into() }) } @@ -398,7 +394,7 @@ impl<'a> FunctionContext<'a> { fn codegen_constrain(&mut self, expr: &Expression, location: Location) -> Values { let boolean = self.codegen_non_tuple_expression(expr); - self.builder.insert_constrain_with_source_location(boolean, location); + self.builder.set_location(location).insert_constrain(boolean); Self::unit_value() } From 0ee7a8f796671c20c722de81a0641dab4b4aa9b5 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 11 Jul 2023 12:47:08 +0000 Subject: [PATCH 10/23] update to the corresponding acvm PR --- crates/nargo_cli/src/cli/execute_cmd.rs | 10 ++++++++-- crates/noirc_driver/Cargo.toml | 1 + crates/noirc_driver/src/lib.rs | 9 ++++++++- .../src/ssa_refactor/ssa_builder/mod.rs | 1 - 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/nargo_cli/src/cli/execute_cmd.rs b/crates/nargo_cli/src/cli/execute_cmd.rs index 4ffa6a20728..da7fcacd430 100644 --- a/crates/nargo_cli/src/cli/execute_cmd.rs +++ b/crates/nargo_cli/src/cli/execute_cmd.rs @@ -1,5 +1,6 @@ use std::path::Path; +use acvm::acir::circuit::OpcodeLabel; use acvm::acir::{circuit::Circuit, native_types::WitnessMap}; use acvm::Backend; use clap::Args; @@ -81,8 +82,13 @@ fn extract_unsatisfied_constraint_from_nargo_error(nargo_err: &NargoError) -> Op }; match solving_err { - acvm::pwg::OpcodeResolutionError::UnsatisfiedConstrain { opcode_index } => { - Some(*opcode_index) + acvm::pwg::OpcodeResolutionError::UnsatisfiedConstrain { opcode_label } => { + match opcode_label { + OpcodeLabel::Unresolved => { + unreachable!("Cannot resolve index for unsatisifed constraint") + } + OpcodeLabel::Resolved(opcode_index) => Some(*opcode_index as usize), + } } _ => None, } diff --git a/crates/noirc_driver/Cargo.toml b/crates/noirc_driver/Cargo.toml index 3f75dc7da8e..e9d95a06419 100644 --- a/crates/noirc_driver/Cargo.toml +++ b/crates/noirc_driver/Cargo.toml @@ -15,3 +15,4 @@ noirc_abi.workspace = true acvm.workspace = true fm.workspace = true serde.workspace = true +iter-extended.workspace = true diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 701bbfd9bb8..81a2e48d353 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -3,7 +3,7 @@ #![warn(unreachable_pub)] #![warn(clippy::semicolon_if_nothing_returned)] -use acvm::acir::circuit::Opcode; +use acvm::acir::circuit::{Opcode, OpcodeLabel}; use acvm::compiler::CircuitSimplifier; use acvm::Language; use clap::Args; @@ -23,6 +23,7 @@ mod contract; mod program; pub use contract::{CompiledContract, ContractFunction, ContractFunctionType}; +use iter_extended::vecmap; pub use program::CompiledProgram; #[derive(Args, Clone, Debug, Serialize, Deserialize)] @@ -348,6 +349,12 @@ pub fn compile_no_check( diagnostic: CustomDiagnostic::from_message("produced an acvm compile error"), }, )?; + let opcode_ids = vecmap(opcode_ids, |id| match id { + OpcodeLabel::Unresolved => { + unreachable!("Compiled circuit opcodes must resolve to some index") + } + OpcodeLabel::Resolved(index) => index as usize, + }); debug.update_acir(opcode_ids); Ok(CompiledProgram { circuit: optimized_circuit, debug, abi }) } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs index deb0053ad59..d3d9e56b3af 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs @@ -213,7 +213,6 @@ impl FunctionBuilder { self.insert_instruction(instruction, None).first() } - /// Insert a not instruction at the end of the current block. /// Returns the result of the instruction. pub(crate) fn insert_not(&mut self, rhs: ValueId) -> ValueId { From 1ffb258b399caad95d8e0a638b081134bd59cd6a Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 11 Jul 2023 14:44:09 +0000 Subject: [PATCH 11/23] update cargo toml --- crates/noirc_driver/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/noirc_driver/Cargo.toml b/crates/noirc_driver/Cargo.toml index e9d95a06419..3f75dc7da8e 100644 --- a/crates/noirc_driver/Cargo.toml +++ b/crates/noirc_driver/Cargo.toml @@ -15,4 +15,3 @@ noirc_abi.workspace = true acvm.workspace = true fm.workspace = true serde.workspace = true -iter-extended.workspace = true From 90afcae254eb9b60e5f7157e0a764b62dd9842a7 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 12 Jul 2023 08:06:55 +0000 Subject: [PATCH 12/23] code review: remove temp code --- noir_stdlib/src/field.nr | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/noir_stdlib/src/field.nr b/noir_stdlib/src/field.nr index e26e9ea6a3d..b7773182d66 100644 --- a/noir_stdlib/src/field.nr +++ b/noir_stdlib/src/field.nr @@ -33,30 +33,6 @@ impl Field { r } - // fn to_le_bits(x : Field) -> [u1; N] { - // let bits = x.to_le_bits_unconstrained(N); - // let result = [0;N]; - // for i in 0..N { - // result[i] = bits[i]; - // } - // result - // } - fn toto(_x : Field) -> [u1; N] { - [0;N] - } - - unconstrained fn to_le_bits_unconstrained(self, bit_size: u32) -> [u1; 254] { - let mut bits = [0; 254]; - let mut a = self; - for i in 0..bit_size { - let b = (a as u120) % 2; ///temp - bits[i as Field] = b as u1; - a = (a - b as Field) /2; - } - bits - } - - // Parity of (prime) Field element, i.e. sgn0(x mod p) = 0 if x ∈ {0, ..., p-1} is even, otherwise sgn0(x mod p) = 1. fn sgn0(self) -> u1 { self as u1 From 7e48183136d007bb6fdf934969b388e309850f25 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 12 Jul 2023 08:23:31 +0000 Subject: [PATCH 13/23] Code review --- .../array_len/src/main.nr | 3 +-- .../brillig_calls/src/main.nr | 14 ++----------- crates/noirc_errors/src/debug_info.rs | 20 ++++++++++--------- .../src/ssa_refactor/ir/dfg.rs | 4 ++++ .../src/ssa_refactor/ssa_gen/context.rs | 5 ++--- 5 files changed, 20 insertions(+), 26 deletions(-) diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr index 101957b03b7..8d2d4853db0 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr @@ -12,7 +12,6 @@ fn nested_call(b: [Field]) -> Field { len_plus_1(b) } - fn main(len3: [u8; 3], len4: [Field; 4]) { assert(len_plus_1(len3) == 4); assert(len_plus_1(len4) == 5); @@ -21,4 +20,4 @@ fn main(len3: [u8; 3], len4: [Field; 4]) { // std::array::len returns a comptime value assert(len4[len3.len()] == 4); -} +} \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr index c82e61a95cc..4cb7fc19ac7 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr @@ -1,20 +1,10 @@ // Tests a very simple program. // // The features being tested is brillig calls -use dep::std; -fn main(x: u32) -> pub [u32; 2] { - assert(sha(x as u8) != 0); +fn main(x: u32) { assert(entry_point(x) == 2); swap_entry_point(x, x + 1); assert(deep_entry_point(x) == 4); - [x,x+1] -} - - -unconstrained fn sha(x : u8) -> u8 { - // std::hash::sha256([x])[0] - // std::sha256::digest([x])[0] - 1 } unconstrained fn inner(x : u32) -> u32 { @@ -52,4 +42,4 @@ unconstrained fn level_1(x : u32) -> u32 { unconstrained fn deep_entry_point(x : u32) -> u32 { level_1(x + 1) -} +} \ No newline at end of file diff --git a/crates/noirc_errors/src/debug_info.rs b/crates/noirc_errors/src/debug_info.rs index 48e047abb3c..d18ee7703df 100644 --- a/crates/noirc_errors/src/debug_info.rs +++ b/crates/noirc_errors/src/debug_info.rs @@ -16,9 +16,18 @@ impl DebugInfo { DebugInfo { locations } } - pub fn update_acir(&mut self, opcode_idx: Vec) { + /// Updates the locations map when the circuit is modified + /// + /// When the circuit is generated, the indices are 0,1,..,n + /// When the circuit is modified, the opcodes are eventually + /// mixed, removed, or with new ones. For instance 5,2,6,n+1,0,12,.. + /// Since new opcodes (n+1 in the ex) don't have a location + /// we use the indice of the old opcode that they replace. + /// This is the case during fallback or width 'optimization' + /// opcode_indices is this list of mixed indices + pub fn update_acir(&mut self, opcode_indices: Vec) { let mut new_locations = HashMap::new(); - for (i, idx) in opcode_idx.iter().enumerate() { + for (i, idx) in opcode_indices.iter().enumerate() { if self.locations.contains_key(idx) { new_locations.insert(i, self.locations[idx]); } @@ -28,12 +37,5 @@ impl DebugInfo { pub fn opcode_location(&self, idx: usize) -> Option<&Location> { self.locations.get(&idx) - // if let Some((start, end, file_id)) = self.locations.get(&idx) { - - // let span = Span::exclusive(*start,*end); - // let f = FileId::new(*file_id); - // return Some(Location::new(span, f)); - // } - // None } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index 5a5fb343fa1..5c9fde280a8 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -71,6 +71,10 @@ pub(crate) struct DataFlowGraph { /// material effect on the SSA itself. replaced_value_ids: HashMap, + /// Source location of each instruction for debugging and issuing errors. + /// + /// Instructions inserted by internal SSA passes that don't correspond to user code + /// may not have a corresponding location. locations: HashMap, } 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 fd199f87359..07ee32998d2 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs @@ -323,9 +323,9 @@ impl<'a> FunctionContext<'a> { let loop_end = self.builder.insert_block(); // pre-loop - let result_alloc = self.builder.insert_allocate(); + let result_alloc = self.builder.set_location(location).insert_allocate(); let true_value = self.builder.numeric_constant(1u128, Type::bool()); - self.builder.set_location(location).insert_store(result_alloc, true_value); + self.builder.insert_store(result_alloc, true_value); let zero = self.builder.field_constant(0u128); self.builder.terminate_with_jmp(loop_start, vec![zero]); @@ -346,7 +346,6 @@ impl<'a> FunctionContext<'a> { self.builder.insert_store(result_alloc, v5); let one = self.builder.field_constant(1u128); let v6 = self.builder.insert_binary(i, BinaryOp::Add, one); - //insert_binary_with_source_location(i, BinaryOp::Add, one, Some(location)); self.builder.terminate_with_jmp(loop_start, vec![v6]); // loop end From 8bd17d993bbb35f66c9dce0fc02eba912d560a05 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 12 Jul 2023 09:09:04 +0000 Subject: [PATCH 14/23] Code review --- crates/nargo_cli/src/cli/compile_cmd.rs | 14 +++--- crates/nargo_cli/src/cli/execute_cmd.rs | 43 ++++++++----------- crates/nargo_cli/src/cli/prove_cmd.rs | 13 ++---- crates/nargo_cli/src/cli/test_cmd.rs | 2 +- .../brillig_modulo/src/main.nr | 1 - crates/noirc_errors/src/debug_info.rs | 4 +- 6 files changed, 31 insertions(+), 46 deletions(-) diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 2da439fc9ea..63812d6d23a 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -1,11 +1,11 @@ use acvm::{ acir::circuit::{Circuit, OpcodeLabel}, - compiler::{CircuitSimplifier, CompileError}, + compiler::CircuitSimplifier, Backend, }; use iter_extended::try_vecmap; use iter_extended::vecmap; -use nargo::artifacts::contract::PreprocessedContract; +use nargo::{artifacts::contract::PreprocessedContract, NargoError}; use noirc_driver::{ compile_contracts, compile_main, CompileOptions, CompiledProgram, ErrorsAndWarnings, Warnings, }; @@ -73,7 +73,7 @@ pub(crate) fn run( try_vecmap(contracts, |contract| { let preprocessed_contract_functions = try_vecmap(contract.functions, |mut func| { - func.bytecode = optimize_circuit(backend, func.bytecode).unwrap().0; + func.bytecode = optimize_circuit(backend, func.bytecode)?.0; common_reference_string = update_common_reference_string( backend, &common_reference_string, @@ -148,17 +148,19 @@ pub(crate) fn compile_circuit( pub(super) fn optimize_circuit( backend: &B, circuit: Circuit, - //context: &Context, -) -> Result<(Circuit, Vec), CompileError> { +) -> Result<(Circuit, Vec), CliError> { // Note that this makes the `CircuitSimplifier` a noop. // The `CircuitSimplifier` should be reworked to not rely on values being inserted during ACIR gen. let simplifier = CircuitSimplifier::new(0); - acvm::compiler::compile( + let result = acvm::compiler::compile( circuit, backend.np_language(), |opcode| backend.supports_opcode(opcode), &simplifier, ) + .map_err(|_| NargoError::CompilationError)?; + + Ok(result) } /// Helper function for reporting any errors in a Result<(T, Warnings), ErrorsAndWarnings> diff --git a/crates/nargo_cli/src/cli/execute_cmd.rs b/crates/nargo_cli/src/cli/execute_cmd.rs index da7fcacd430..f94f8b630e9 100644 --- a/crates/nargo_cli/src/cli/execute_cmd.rs +++ b/crates/nargo_cli/src/cli/execute_cmd.rs @@ -7,8 +7,8 @@ use clap::Args; use nargo::NargoError; use noirc_abi::input_parser::{Format, InputValue}; use noirc_abi::{Abi, InputMap}; -use noirc_driver::{CompileOptions, CompiledProgram, ErrorsAndWarnings}; -use noirc_errors::{debug_info::DebugInfo, CustomDiagnostic, FileDiagnostic}; +use noirc_driver::{CompileOptions, CompiledProgram}; +use noirc_errors::{debug_info::DebugInfo, CustomDiagnostic}; use noirc_frontend::hir::Context; use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; @@ -68,7 +68,8 @@ fn execute_with_path( let (inputs_map, _) = read_inputs_from_file(program_dir, prover_name.as_str(), Format::Toml, &abi)?; - let solved_witness = execute_program(backend, circuit, &abi, &inputs_map, &debug, &context)?; + let solved_witness = + execute_program(backend, circuit, &abi, &inputs_map, Some((debug, context)))?; let public_abi = abi.public_abi(); let (_, return_value) = public_abi.decode(&solved_witness)?; @@ -100,15 +101,16 @@ fn report_unsatisfied_constraint_error( ) { if let Some(opcode_index) = opcode_idx { if let Some(loc) = debug.opcode_location(opcode_index) { - let errs_warnings: ErrorsAndWarnings = vec![FileDiagnostic { - file_id: loc.file, - diagnostic: CustomDiagnostic::simple_error( + noirc_errors::reporter::report( + &context.file_manager, + &CustomDiagnostic::simple_error( "Unsatisfied constraint".to_string(), - "happening on this line".to_string(), + "Constraint failed".to_string(), loc.span, ), - }]; - noirc_errors::reporter::report_all(&context.file_manager, &errs_warnings, false); + Some(loc.file), + false, + ); } } } @@ -118,30 +120,19 @@ pub(crate) fn execute_program( circuit: Circuit, abi: &Abi, inputs_map: &InputMap, - debug: &DebugInfo, - context: &Context, + debug_data: Option<(DebugInfo, Context)>, ) -> Result> { let initial_witness = abi.encode(inputs_map, None)?; - let solved_witness_err = nargo::ops::execute_circuit(backend, circuit, initial_witness); match solved_witness_err { Ok(solved_witness) => Ok(solved_witness), Err(err) => { - let opcode_idx = extract_unsatisfied_constraint_from_nargo_error(&err); - report_unsatisfied_constraint_error(opcode_idx, debug, context); + if let Some((debug, context)) = debug_data { + let opcode_idx = extract_unsatisfied_constraint_from_nargo_error(&err); + report_unsatisfied_constraint_error(opcode_idx, &debug, &context); + } + Err(crate::errors::CliError::NargoError(err)) } } } - -pub(crate) fn execute_program_without_debug( - backend: &B, - circuit: Circuit, - abi: &Abi, - inputs_map: &InputMap, -) -> Result> { - let initial_witness = abi.encode(inputs_map, None)?; - - let solved_witness = nargo::ops::execute_circuit(backend, circuit, initial_witness)?; - Ok(solved_witness) -} diff --git a/crates/nargo_cli/src/cli/prove_cmd.rs b/crates/nargo_cli/src/cli/prove_cmd.rs index 1bb290502a1..d55aaa7f8b4 100644 --- a/crates/nargo_cli/src/cli/prove_cmd.rs +++ b/crates/nargo_cli/src/cli/prove_cmd.rs @@ -22,7 +22,6 @@ use super::{ }; use crate::{ cli::execute_cmd::execute_program, - cli::execute_cmd::execute_program_without_debug, constants::{PROOFS_DIR, PROVER_INPUT_FILE, TARGET_DIR, VERIFIER_INPUT_FILE}, errors::CliError, }; @@ -92,7 +91,7 @@ pub(crate) fn prove_with_path>( ) -> Result, CliError> { let common_reference_string = read_cached_common_reference_string(); - let (common_reference_string, preprocessed_program, debug, context) = match circuit_build_path { + let (common_reference_string, preprocessed_program, debug_data) = match circuit_build_path { Some(circuit_build_path) => { let program = read_program_from_file(circuit_build_path)?; let common_reference_string = update_common_reference_string( @@ -101,7 +100,7 @@ pub(crate) fn prove_with_path>( &program.bytecode, ) .map_err(CliError::CommonReferenceStringError)?; - (common_reference_string, program, None, None) + (common_reference_string, program, None) } None => { let (program, context) = @@ -112,7 +111,7 @@ pub(crate) fn prove_with_path>( .map_err(CliError::CommonReferenceStringError)?; let program = preprocess_program(backend, true, &common_reference_string, program) .map_err(CliError::ProofSystemCompilerError)?; - (common_reference_string, program, Some(debug), Some(context)) + (common_reference_string, program, Some((debug, context))) } }; @@ -125,11 +124,7 @@ pub(crate) fn prove_with_path>( let (inputs_map, _) = read_inputs_from_file(&program_dir, prover_name.as_str(), Format::Toml, &abi)?; - let solved_witness = if let Some(driver) = context { - execute_program(backend, bytecode.clone(), &abi, &inputs_map, &debug.unwrap(), &driver)? - } else { - execute_program_without_debug(backend, bytecode.clone(), &abi, &inputs_map)? - }; + let solved_witness = execute_program(backend, bytecode.clone(), &abi, &inputs_map, debug_data)?; // Write public inputs into Verifier.toml let public_abi = abi.public_abi(); diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index e129b38cac9..896ee6c4445 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -92,7 +92,7 @@ fn run_test( let mut program = compile_no_check(context, config, main) .map_err(|_| CliError::Generic(format!("Test '{test_name}' failed to compile")))?; // Note: We could perform this test using the unoptimized ACIR as generated by `compile_no_check`. - program.circuit = optimize_circuit(backend, program.circuit).unwrap(); + program.circuit = optimize_circuit(backend, program.circuit).unwrap().0; // Run the backend to ensure the PWG evaluates functions like std::hash::pedersen, // otherwise constraints involving these expressions will not error. diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr index 5104916a3ea..1cab78ecb95 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr @@ -1,4 +1,3 @@ - // Tests a very simple program. // // The features being tested is modulo operations on brillig diff --git a/crates/noirc_errors/src/debug_info.rs b/crates/noirc_errors/src/debug_info.rs index d18ee7703df..488363db9bf 100644 --- a/crates/noirc_errors/src/debug_info.rs +++ b/crates/noirc_errors/src/debug_info.rs @@ -1,9 +1,7 @@ use std::collections::HashMap; -//use noirc_errors::Span; -use serde::{Deserialize, Serialize}; -//use fm::FileId; use crate::Location; +use serde::{Deserialize, Serialize}; #[derive(Default, Debug, Clone, Deserialize, Serialize)] pub struct DebugInfo { From e066dea7ea6839d3b5156a1a9455880daf5d304c Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 12 Jul 2023 20:45:16 +0100 Subject: [PATCH 15/23] Update crates/noirc_errors/src/debug_info.rs Co-authored-by: jfecher --- crates/noirc_errors/src/debug_info.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_errors/src/debug_info.rs b/crates/noirc_errors/src/debug_info.rs index 488363db9bf..3217dba7a38 100644 --- a/crates/noirc_errors/src/debug_info.rs +++ b/crates/noirc_errors/src/debug_info.rs @@ -20,7 +20,7 @@ impl DebugInfo { /// When the circuit is modified, the opcodes are eventually /// mixed, removed, or with new ones. For instance 5,2,6,n+1,0,12,.. /// Since new opcodes (n+1 in the ex) don't have a location - /// we use the indice of the old opcode that they replace. + /// we use the index of the old opcode that they replace. /// This is the case during fallback or width 'optimization' /// opcode_indices is this list of mixed indices pub fn update_acir(&mut self, opcode_indices: Vec) { From d4400221dbc5174c05d5846c02502259a8226018 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 12 Jul 2023 19:48:58 +0000 Subject: [PATCH 16/23] fix typo --- crates/nargo_cli/src/cli/execute_cmd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nargo_cli/src/cli/execute_cmd.rs b/crates/nargo_cli/src/cli/execute_cmd.rs index f94f8b630e9..d8b764a6282 100644 --- a/crates/nargo_cli/src/cli/execute_cmd.rs +++ b/crates/nargo_cli/src/cli/execute_cmd.rs @@ -86,7 +86,7 @@ fn extract_unsatisfied_constraint_from_nargo_error(nargo_err: &NargoError) -> Op acvm::pwg::OpcodeResolutionError::UnsatisfiedConstrain { opcode_label } => { match opcode_label { OpcodeLabel::Unresolved => { - unreachable!("Cannot resolve index for unsatisifed constraint") + unreachable!("Cannot resolve index for unsatisfied constraint") } OpcodeLabel::Resolved(opcode_index) => Some(*opcode_index as usize), } From 2ce27f7e713f88084adbe3ce7e5de2827c5b6ce1 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 12 Jul 2023 20:04:14 +0000 Subject: [PATCH 17/23] fix merge --- crates/nargo_cli/src/cli/compile_cmd.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 1e7e5c26127..443b3d56090 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -1,3 +1,4 @@ +use acvm::acir::circuit::OpcodeLabel; use acvm::{acir::circuit::Circuit, Backend}; use iter_extended::try_vecmap; use iter_extended::vecmap; @@ -144,12 +145,11 @@ pub(crate) fn compile_circuit( pub(super) fn optimize_circuit( backend: &B, circuit: Circuit, -) -> Result> { - let (optimized_circuit, _): (Circuit, Vec) = - acvm::compiler::compile(circuit, backend.np_language(), |opcode| { - backend.supports_opcode(opcode) - }) - .map_err(|_| NargoError::CompilationError)?; +) -> Result<(Circuit, Vec), CliError> { + let result = acvm::compiler::compile(circuit, backend.np_language(), |opcode| { + backend.supports_opcode(opcode) + }) + .map_err(|_| NargoError::CompilationError)?; Ok(result) } From 97b3f8d1a4f282256b28e39c425cd5a882a88d82 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 12 Jul 2023 20:10:39 +0000 Subject: [PATCH 18/23] minor nit --- crates/nargo_cli/src/cli/compile_cmd.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 443b3d56090..c12c90be16b 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -127,10 +127,9 @@ pub(crate) fn compile_circuit( let mut program = report_errors(result, &context, compile_options.deny_warnings)?; // Apply backend specific optimizations. - let optimised = optimize_circuit(backend, program.circuit).unwrap(); + let (optimized_circuit, opcode_labels) = optimize_circuit(backend, program.circuit).unwrap(); - program.circuit = optimised.0; - let opcode_labels = optimised.1; + program.circuit = optimized_circuit; let opcode_ids = vecmap(opcode_labels, |label| match label { OpcodeLabel::Unresolved => { unreachable!("Compiled circuit opcodes must resolve to some index") From f1587b84fbe0136715196a9c0fd3165b774d5d1c Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 12 Jul 2023 21:01:03 +0000 Subject: [PATCH 19/23] revert all test changes --- .../arithmetic_binary_operations/src/main.nr | 11 +++--- .../array_len/src/main.nr | 2 +- .../brillig_calls/src/main.nr | 2 +- .../brillig_conditional/src/main.nr | 39 +++++-------------- .../test_data_ssa_refactor/sha256/Prover.toml | 2 +- .../test_data_ssa_refactor/sha256/src/main.nr | 2 +- 6 files changed, 18 insertions(+), 40 deletions(-) diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr index 4327685a27c..391aa27049d 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/arithmetic_binary_operations/src/main.nr @@ -1,13 +1,12 @@ -use dep::std; // Tests a very simple program. // // The features being tested are: // Binary addition, multiplication, division // x = 3, y = 4, z = 5 fn main(x : Field, y : Field, z : Field) -> pub Field { - let a = x + x; // 3 + 3 = 6 - let b = a - y; // 6 - 4 = 2 - let c = b * z; // 2 * 5 = 10 - let d = c / a; // 10 / 6 (This uses field inversion, so we test it by multiplying by `a`) - d * a + let a = x + x; // 3 + 3 = 6 + let b = a - y; // 6 - 4 = 2 + let c = b * z; // 2 * 5 = 10 + let d = c / a; // 10 / 6 (This uses field inversion, so we test it by multiplying by `a`) + d * a } \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr index 8d2d4853db0..9099cfa2144 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/array_len/src/main.nr @@ -20,4 +20,4 @@ fn main(len3: [u8; 3], len4: [Field; 4]) { // std::array::len returns a comptime value assert(len4[len3.len()] == 4); -} \ No newline at end of file +} diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr index 4cb7fc19ac7..795fc02c35f 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_calls/src/main.nr @@ -42,4 +42,4 @@ unconstrained fn level_1(x : u32) -> u32 { unconstrained fn deep_entry_point(x : u32) -> u32 { level_1(x + 1) -} \ No newline at end of file +} diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr index 8de3ba57935..4ddd351ad04 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_conditional/src/main.nr @@ -1,35 +1,14 @@ // Tests a very simple program. // -// The features being tested is brillig calls with conditionals -fn main(x: [u32; 3]) { - assert(entry_point(x[0]) == 7); - assert(entry_point(x[1]) == 8); - assert(entry_point(x[2]) == 9); - assert(entry_point(42) == 0); +// The features being tested is basic conditonal on brillig +fn main(x: Field) { + assert(4 == conditional(x as bool)); } -unconstrained fn inner_1() -> u32 { - 7 -} - -unconstrained fn inner_2() -> u32 { - 8 -} - -unconstrained fn inner_3() -> u32 { - 9 -} - -unconstrained fn entry_point(x: u32) -> u32 { - let mut result: u32 = 0; - - if x == 1 { - result = inner_1(); - } else if x == 2 { - result = inner_2(); - } else if x == 3 { - result = inner_3(); - } - - result +unconstrained fn conditional(x : bool) -> Field { + if x { + 4 + }else { + 5 + } } \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml index 6b17aaf482c..c4df1b749bb 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/Prover.toml @@ -33,4 +33,4 @@ result = [ 0xfe, 0x73, 0x2b, -] \ No newline at end of file +] diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr index f758b8e7e28..fd5340e2384 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/sha256/src/main.nr @@ -16,4 +16,4 @@ fn main(x: Field, result: [u8; 32]) { // The padding is taken care of by the program let digest = std::hash::sha256([x as u8]); assert(digest == result); -} \ No newline at end of file +} From 566648dedba5709b5ff1de67859514d166a2f53c Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 12 Jul 2023 21:17:30 +0000 Subject: [PATCH 20/23] add noirc_errors as a dependency to nargo --- Cargo.lock | 1 + crates/nargo/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 704204fc9d7..6b7bf85bfa8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1997,6 +1997,7 @@ dependencies = [ "iter-extended", "noirc_abi", "noirc_driver", + "noirc_errors", "rustc_version", "serde", "thiserror", diff --git a/crates/nargo/Cargo.toml b/crates/nargo/Cargo.toml index 8d3c9fbd3cd..f85c18ec001 100644 --- a/crates/nargo/Cargo.toml +++ b/crates/nargo/Cargo.toml @@ -18,3 +18,4 @@ iter-extended.workspace = true toml.workspace = true serde.workspace = true thiserror.workspace = true +noirc_errors.workspace = true From 82241b6d665e2f43939319165cf105f376a06f94 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 12 Jul 2023 21:17:50 +0000 Subject: [PATCH 21/23] preprocess_program returns DebugInfo --- crates/nargo/src/ops/preprocess.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/crates/nargo/src/ops/preprocess.rs b/crates/nargo/src/ops/preprocess.rs index b1613ea3195..d07da256ede 100644 --- a/crates/nargo/src/ops/preprocess.rs +++ b/crates/nargo/src/ops/preprocess.rs @@ -1,5 +1,6 @@ use acvm::ProofSystemCompiler; use noirc_driver::{CompiledProgram, ContractFunction}; +use noirc_errors::debug_info::DebugInfo; use crate::artifacts::{contract::PreprocessedContractFunction, program::PreprocessedProgram}; @@ -11,7 +12,7 @@ pub fn preprocess_program( include_keys: bool, common_reference_string: &[u8], compiled_program: CompiledProgram, -) -> Result { +) -> Result<(PreprocessedProgram, DebugInfo), B::Error> { // TODO: currently `compiled_program`'s bytecode is already optimized for the backend. // In future we'll need to apply those optimizations here. let optimized_bytecode = compiled_program.circuit; @@ -24,13 +25,16 @@ pub fn preprocess_program( (None, None) }; - Ok(PreprocessedProgram { - backend: String::from(BACKEND_IDENTIFIER), - abi: compiled_program.abi, - bytecode: optimized_bytecode, - proving_key, - verification_key, - }) + Ok(( + PreprocessedProgram { + backend: String::from(BACKEND_IDENTIFIER), + abi: compiled_program.abi, + bytecode: optimized_bytecode, + proving_key, + verification_key, + }, + compiled_program.debug, + )) } pub fn preprocess_contract_function( From aa7a8eae1db5e8781fa9b77c8a8653f44a4a6513 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 12 Jul 2023 21:18:04 +0000 Subject: [PATCH 22/23] modify all methods which call preprocess_program --- crates/nargo_cli/src/cli/codegen_verifier_cmd.rs | 2 +- crates/nargo_cli/src/cli/compile_cmd.rs | 2 +- crates/nargo_cli/src/cli/prove_cmd.rs | 6 +++--- crates/nargo_cli/src/cli/verify_cmd.rs | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs index 59e7e5ca009..c19ed5df3a6 100644 --- a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -56,7 +56,7 @@ pub(crate) fn run( let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; - let program = preprocess_program(backend, true, &common_reference_string, program) + let (program, _) = preprocess_program(backend, true, &common_reference_string, program) .map_err(CliError::ProofSystemCompilerError)?; (common_reference_string, program) } diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index c12c90be16b..cb7b8272657 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -106,7 +106,7 @@ pub(crate) fn run( update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; - let preprocessed_program = + let (preprocessed_program, _) = preprocess_program(backend, args.include_keys, &common_reference_string, program) .map_err(CliError::ProofSystemCompilerError)?; save_program_to_file(&preprocessed_program, &args.circuit_name, circuit_dir); diff --git a/crates/nargo_cli/src/cli/prove_cmd.rs b/crates/nargo_cli/src/cli/prove_cmd.rs index d55aaa7f8b4..346638835e3 100644 --- a/crates/nargo_cli/src/cli/prove_cmd.rs +++ b/crates/nargo_cli/src/cli/prove_cmd.rs @@ -105,12 +105,12 @@ pub(crate) fn prove_with_path>( None => { let (program, context) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; - let debug = program.debug.clone(); let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; - let program = preprocess_program(backend, true, &common_reference_string, program) - .map_err(CliError::ProofSystemCompilerError)?; + let (program, debug) = + preprocess_program(backend, true, &common_reference_string, program) + .map_err(CliError::ProofSystemCompilerError)?; (common_reference_string, program, Some((debug, context))) } }; diff --git a/crates/nargo_cli/src/cli/verify_cmd.rs b/crates/nargo_cli/src/cli/verify_cmd.rs index 2662bbb3c04..cc85159e488 100644 --- a/crates/nargo_cli/src/cli/verify_cmd.rs +++ b/crates/nargo_cli/src/cli/verify_cmd.rs @@ -87,7 +87,7 @@ fn verify_with_path>( let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; - let program = preprocess_program(backend, true, &common_reference_string, program) + let (program, _) = preprocess_program(backend, true, &common_reference_string, program) .map_err(CliError::ProofSystemCompilerError)?; (common_reference_string, program) } From b0c34b613c663c8ff6e05a286173dc656d355ccf Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 12 Jul 2023 21:21:09 +0000 Subject: [PATCH 23/23] change unwrap to expect --- crates/nargo_cli/src/cli/compile_cmd.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index cb7b8272657..da945e6e15e 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -127,7 +127,8 @@ pub(crate) fn compile_circuit( let mut program = report_errors(result, &context, compile_options.deny_warnings)?; // Apply backend specific optimizations. - let (optimized_circuit, opcode_labels) = optimize_circuit(backend, program.circuit).unwrap(); + let (optimized_circuit, opcode_labels) = optimize_circuit(backend, program.circuit) + .expect("Backend does not support an opcode that is in the IR"); program.circuit = optimized_circuit; let opcode_ids = vecmap(opcode_labels, |label| match label {