From 541edff7a970ec2c48a67822d6c14383f53504bc Mon Sep 17 00:00:00 2001 From: Blaine Bublitz Date: Mon, 13 Feb 2023 16:17:32 -0700 Subject: [PATCH 1/4] chore(ci): Add release token to enable runs against release PR (#840) --- .github/workflows/release.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 68d72bca72e..26b826ed4c8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,6 +17,7 @@ jobs: id: release uses: google-github-actions/release-please-action@v3 with: + token: ${{ secrets.NOIR_RELEASES_TOKEN }} release-type: simple package-name: noir bump-minor-pre-major: true From 9af18d790e06832fb8e656d12772aae06acd4a25 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 14 Feb 2023 11:54:50 +0000 Subject: [PATCH 2/4] chore: remove unwanted print of Ok() in nargo compile (#843) --- crates/nargo/src/cli/compile_cmd.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/nargo/src/cli/compile_cmd.rs b/crates/nargo/src/cli/compile_cmd.rs index 2049f2468b9..ff0f922e70a 100644 --- a/crates/nargo/src/cli/compile_cmd.rs +++ b/crates/nargo/src/cli/compile_cmd.rs @@ -52,7 +52,6 @@ pub fn generate_circuit_and_witness_to_disk>( circuit_path.set_extension(ACIR_EXT); let path = write_to_file(serialized.as_slice(), &circuit_path); println!("Generated ACIR code into {path}"); - println!("{:?}", std::fs::canonicalize(&circuit_path)); if generate_witness { let (_, solved_witness) = From 06f058b5738cbf8c2fcc331b231222e59c8ee7bb Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Tue, 14 Feb 2023 12:58:21 +0100 Subject: [PATCH 3/4] chore(ssa): per array memory map (#832) * assign a VM per array * cargo format * Code review * code review --- crates/noirc_evaluator/src/ssa/acir_gen.rs | 18 ++++----- .../acir_gen/{memory_map.rs => acir_mem.rs} | 37 ++++++++++++------- .../src/ssa/acir_gen/operations/binary.rs | 4 +- .../src/ssa/acir_gen/operations/cmp.rs | 10 ++--- .../src/ssa/acir_gen/operations/intrinsics.rs | 17 ++++----- .../src/ssa/acir_gen/operations/load.rs | 4 +- .../src/ssa/acir_gen/operations/return.rs | 4 +- .../src/ssa/acir_gen/operations/store.rs | 7 ++-- 8 files changed, 54 insertions(+), 47 deletions(-) rename crates/noirc_evaluator/src/ssa/acir_gen/{memory_map.rs => acir_mem.rs} (74%) diff --git a/crates/noirc_evaluator/src/ssa/acir_gen.rs b/crates/noirc_evaluator/src/ssa/acir_gen.rs index 52fe4de857f..6ac54ef7afa 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen.rs @@ -18,12 +18,12 @@ use internal_var_cache::InternalVarCache; // Expose this to the crate as we need to apply range constraints when // converting the ABI(main parameters) to Noir types pub(crate) use constraints::range_constraint; -mod memory_map; -use memory_map::MemoryMap; +mod acir_mem; +use acir_mem::AcirMem; #[derive(Default)] pub struct Acir { - memory_map: MemoryMap, + memory: AcirMem, var_cache: InternalVarCache, } @@ -39,12 +39,12 @@ impl Acir { binary, condition, constrain, intrinsics, load, not, r#return, store, truncate, }; - let memory_map = &mut self.memory_map; + let acir_mem = &mut self.memory; let var_cache = &mut self.var_cache; let output = match &ins.operation { Operation::Binary(binary) => { - binary::evaluate(binary, ins.res_type, var_cache, memory_map, evaluator, ctx) + binary::evaluate(binary, ins.res_type, var_cache, acir_mem, evaluator, ctx) } Operation::Constrain(value, ..) => { constrain::evaluate(value, var_cache, evaluator, ctx) @@ -57,19 +57,19 @@ impl Acir { truncate::evaluate(value, *bit_size, *max_bit_size, var_cache, evaluator, ctx) } Operation::Intrinsic(opcode, args) => { - intrinsics::evaluate(args, ins, *opcode, var_cache, memory_map, ctx, evaluator) + intrinsics::evaluate(args, ins, *opcode, var_cache, acir_mem, ctx, evaluator) } Operation::Return(node_ids) => { - r#return::evaluate(node_ids, memory_map, var_cache, evaluator, ctx)? + r#return::evaluate(node_ids, acir_mem, var_cache, evaluator, ctx)? } Operation::Cond { condition, val_true: lhs, val_false: rhs } => { condition::evaluate(*condition, *lhs, *rhs, var_cache, evaluator, ctx) } Operation::Load { array_id, index } => { - load::evaluate(*array_id, *index, memory_map, var_cache, evaluator, ctx) + load::evaluate(*array_id, *index, acir_mem, var_cache, evaluator, ctx) } Operation::Store { array_id, index, value } => { - store::evaluate(*array_id, *index, *value, memory_map, var_cache, evaluator, ctx) + store::evaluate(*array_id, *index, *value, acir_mem, var_cache, evaluator, ctx) } Operation::Nop => None, i @ Operation::Jne(..) diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/memory_map.rs b/crates/noirc_evaluator/src/ssa/acir_gen/acir_mem.rs similarity index 74% rename from crates/noirc_evaluator/src/ssa/acir_gen/memory_map.rs rename to crates/noirc_evaluator/src/ssa/acir_gen/acir_mem.rs index 5e91ad52aa3..29aa254f52a 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/memory_map.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/acir_mem.rs @@ -5,26 +5,41 @@ use crate::ssa::{ }; use acvm::acir::native_types::Witness; use iter_extended::vecmap; -use std::collections::HashMap; +use std::collections::BTreeMap; -// maps memory address to expression #[derive(Default)] -pub struct MemoryMap { - inner: HashMap, +pub struct ArrayHeap { + // maps memory address to InternalVar + memory_map: BTreeMap, } -impl MemoryMap { +/// Handle virtual memory access +#[derive(Default)] +pub struct AcirMem { + virtual_memory: BTreeMap, +} + +impl AcirMem { + // Returns the memory_map for the array + fn array_map_mut(&mut self, array_id: ArrayId) -> &mut BTreeMap { + &mut self.virtual_memory.entry(array_id).or_default().memory_map + } + + // Write the value to the array's VM at the specified index + pub fn insert(&mut self, array_id: ArrayId, index: u32, value: InternalVar) { + self.array_map_mut(array_id).insert(index, value); + } + //Map the outputs into the array pub(crate) fn map_array(&mut self, a: ArrayId, outputs: &[Witness], ctx: &SsaContext) { let array = &ctx.mem[a]; - let address = array.adr; for i in 0..array.len { let var = if i < outputs.len() as u32 { InternalVar::from(outputs[i as usize]) } else { InternalVar::zero_expr() }; - self.inner.insert(address + i, var); + self.array_map_mut(array.id).insert(i, var); } } @@ -60,10 +75,8 @@ impl MemoryMap { return None; // IndexOutOfBoundsError } - let address_of_element = array.absolute_adr(offset); - // Check the memory_map to see if the element is there - if let Some(internal_var) = self.inner.get(&address_of_element) { + if let Some(internal_var) = self.array_map_mut(array.id).get(&offset) { return Some(internal_var.clone()); }; @@ -78,8 +91,4 @@ impl MemoryMap { Some(array_element) } - - pub(crate) fn insert(&mut self, key: u32, value: InternalVar) -> Option { - self.inner.insert(key, value) - } } diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/binary.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/binary.rs index 57bc0782d16..286ba988eec 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/binary.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/binary.rs @@ -1,7 +1,7 @@ use crate::{ ssa::{ acir_gen::{ - constraints, internal_var_cache::InternalVarCache, memory_map::MemoryMap, operations, + acir_mem::AcirMem, constraints, internal_var_cache::InternalVarCache, operations, InternalVar, }, context::SsaContext, @@ -30,7 +30,7 @@ pub(crate) fn evaluate( binary: &node::Binary, res_type: ObjectType, var_cache: &mut InternalVarCache, - memory_map: &mut MemoryMap, + memory_map: &mut AcirMem, evaluator: &mut Evaluator, ctx: &SsaContext, ) -> Option { diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/cmp.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/cmp.rs index 7a282fdf0f9..fdf411acb74 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/cmp.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/cmp.rs @@ -1,6 +1,6 @@ use crate::{ ssa::{ - acir_gen::{constraints, memory_map::MemoryMap, InternalVar}, + acir_gen::{acir_mem::AcirMem, constraints, InternalVar}, context::SsaContext, mem::{MemArray, Memory}, node::NodeId, @@ -26,7 +26,7 @@ use iter_extended::vecmap; // so in reality, the NEQ instruction will be done on the fields // of the struct pub(crate) fn evaluate_neq( - memory_map: &mut MemoryMap, + acir_mem: &mut AcirMem, lhs: NodeId, rhs: NodeId, l_c: Option, @@ -55,7 +55,7 @@ pub(crate) fn evaluate_neq( ) } - let mut x = InternalVar::from(array_eq(memory_map, array_a, array_b, evaluator)); + let mut x = InternalVar::from(array_eq(acir_mem, array_a, array_b, evaluator)); // TODO we need a witness because of the directive, but we should use an expression // TODO if we change the Invert directive to take an `Expression`, then we // TODO can get rid of this extra gate. @@ -89,7 +89,7 @@ pub(crate) fn evaluate_neq( } pub(crate) fn evaluate_eq( - memory_map: &mut MemoryMap, + memory_map: &mut AcirMem, lhs: NodeId, rhs: NodeId, l_c: Option, @@ -107,7 +107,7 @@ pub(crate) fn evaluate_eq( // // N.B. We assumes the lengths of a and b are the same but it is not checked inside the function. fn array_eq( - memory_map: &mut MemoryMap, + memory_map: &mut AcirMem, a: &MemArray, b: &MemArray, evaluator: &mut Evaluator, diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/intrinsics.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/intrinsics.rs index 7feac203bb3..942a00f0137 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/intrinsics.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/intrinsics.rs @@ -4,7 +4,7 @@ use crate::{ constraints::{bound_constraint_with_offset, to_radix_base}, expression_from_witness, operations::sort::evaluate_permutation, - InternalVar, InternalVarCache, MemoryMap, + AcirMem, InternalVar, InternalVarCache, }, builtin, context::SsaContext, @@ -32,7 +32,7 @@ pub(crate) fn evaluate( instruction: &Instruction, opcode: builtin::Opcode, var_cache: &mut InternalVarCache, - memory_map: &mut MemoryMap, + memory_map: &mut AcirMem, ctx: &SsaContext, evaluator: &mut Evaluator, ) -> Option { @@ -121,7 +121,7 @@ pub(crate) fn evaluate( // Transform the arguments of intrinsic functions into witnesses fn prepare_inputs( var_cache: &mut InternalVarCache, - memory_map: &mut MemoryMap, + memory_map: &mut AcirMem, arguments: &[NodeId], cfg: &SsaContext, evaluator: &mut Evaluator, @@ -137,7 +137,7 @@ fn prepare_inputs( fn resolve_node_id( node_id: &NodeId, var_cache: &mut InternalVarCache, - memory_map: &mut MemoryMap, + memory_map: &mut AcirMem, cfg: &SsaContext, evaluator: &mut Evaluator, ) -> Vec { @@ -179,7 +179,7 @@ fn resolve_node_id( fn resolve_array( array_id: ArrayId, - memory_map: &mut MemoryMap, + acir_mem: &mut AcirMem, cfg: &SsaContext, evaluator: &mut Evaluator, ) -> Vec { @@ -188,7 +188,7 @@ fn resolve_array( let array = &cfg.mem[array_id]; let num_bits = array.element_type.bits(); for i in 0..array.len { - let mut arr_element = memory_map + let mut arr_element = acir_mem .load_array_element_constant_index(array, i) .expect("array index out of bounds"); @@ -197,8 +197,7 @@ fn resolve_array( ); let func_input = FunctionInput { witness, num_bits }; - let address = array.adr + i; - memory_map.insert(address, arr_element); + acir_mem.insert(array.id, i, arr_element); inputs.push(func_input) } @@ -207,7 +206,7 @@ fn resolve_array( } fn prepare_outputs( - memory_map: &mut MemoryMap, + memory_map: &mut AcirMem, pointer: NodeId, output_nb: u32, ctx: &SsaContext, diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/load.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/load.rs index b12e9c487b6..c28e77e8220 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/load.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/load.rs @@ -1,6 +1,6 @@ use crate::{ ssa::{ - acir_gen::{internal_var_cache::InternalVarCache, memory_map::MemoryMap, InternalVar}, + acir_gen::{acir_mem::AcirMem, internal_var_cache::InternalVarCache, InternalVar}, context::SsaContext, mem::{self, ArrayId}, node::NodeId, @@ -11,7 +11,7 @@ use crate::{ pub(crate) fn evaluate( array_id: ArrayId, index: NodeId, - memory_map: &mut MemoryMap, + memory_map: &mut AcirMem, var_cache: &mut InternalVarCache, evaluator: &mut Evaluator, ctx: &SsaContext, diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs index 2fc08cdf486..96b637d99c2 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs @@ -1,7 +1,7 @@ use crate::{ errors::RuntimeErrorKind, ssa::{ - acir_gen::{internal_var_cache::InternalVarCache, memory_map::MemoryMap, InternalVar}, + acir_gen::{acir_mem::AcirMem, internal_var_cache::InternalVarCache, InternalVar}, context::SsaContext, mem::Memory, node::NodeId, @@ -11,7 +11,7 @@ use crate::{ pub(crate) fn evaluate( node_ids: &[NodeId], - memory_map: &mut MemoryMap, + memory_map: &mut AcirMem, var_cache: &mut InternalVarCache, evaluator: &mut Evaluator, ctx: &SsaContext, diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/store.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/store.rs index 1fd72793eb6..ead9429f642 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/store.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/store.rs @@ -1,6 +1,6 @@ use crate::{ ssa::{ - acir_gen::{internal_var_cache::InternalVarCache, memory_map::MemoryMap, InternalVar}, + acir_gen::{acir_mem::AcirMem, internal_var_cache::InternalVarCache, InternalVar}, context::SsaContext, mem::{self, ArrayId}, node::NodeId, @@ -12,7 +12,7 @@ pub(crate) fn evaluate( array_id: ArrayId, index: NodeId, value: NodeId, - memory_map: &mut MemoryMap, + acir_mem: &mut AcirMem, var_cache: &mut InternalVarCache, evaluator: &mut Evaluator, ctx: &SsaContext, @@ -24,8 +24,7 @@ pub(crate) fn evaluate( match index.to_const() { Some(index) => { let idx = mem::Memory::as_u32(index); - let absolute_adr = ctx.mem[array_id].absolute_adr(idx); - memory_map.insert(absolute_adr, value); + acir_mem.insert(array_id, idx, value); //we do not generate constraint, so no output. None } From 766b57d8f29a507eb545c209964ec7f7b6343618 Mon Sep 17 00:00:00 2001 From: Ethan-000 Date: Tue, 14 Feb 2023 13:04:14 +0000 Subject: [PATCH 4/4] chore: Add display for binaryop (#839) * add display for binaryop * remove display import --- crates/noirc_evaluator/src/ssa/context.rs | 29 +------------------- crates/noirc_evaluator/src/ssa/node.rs | 33 +++++++++++++++++++++++ 2 files changed, 34 insertions(+), 28 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/context.rs b/crates/noirc_evaluator/src/ssa/context.rs index 58f9d9d94cf..3a36ddcf9ab 100644 --- a/crates/noirc_evaluator/src/ssa/context.rs +++ b/crates/noirc_evaluator/src/ssa/context.rs @@ -157,35 +157,8 @@ impl SsaContext { fn binary_to_string(&self, binary: &node::Binary) -> String { let lhs = self.id_to_string(binary.lhs); let rhs = self.id_to_string(binary.rhs); - let op = match &binary.operator { - BinaryOp::Add => "add", - BinaryOp::SafeAdd => "safe_add", - BinaryOp::Sub { .. } => "sub", - BinaryOp::SafeSub { .. } => "safe_sub", - BinaryOp::Mul => "mul", - BinaryOp::SafeMul => "safe_mul", - BinaryOp::Udiv => "udiv", - BinaryOp::Sdiv => "sdiv", - BinaryOp::Urem => "urem", - BinaryOp::Srem => "srem", - BinaryOp::Div => "div", - BinaryOp::Eq => "eq", - BinaryOp::Ne => "ne", - BinaryOp::Ult => "ult", - BinaryOp::Ule => "ule", - BinaryOp::Slt => "slt", - BinaryOp::Sle => "sle", - BinaryOp::Lt => "lt", - BinaryOp::Lte => "lte", - BinaryOp::And => "and", - BinaryOp::Or => "or", - BinaryOp::Xor => "xor", - BinaryOp::Assign => "assign", - BinaryOp::Shl => "shl", - BinaryOp::Shr => "shr", - }; - format!("{op} {lhs}, {rhs}") + format!("{} {lhs}, {rhs}", binary.operator) } pub fn operation_to_string(&self, op: &Operation) -> String { diff --git a/crates/noirc_evaluator/src/ssa/node.rs b/crates/noirc_evaluator/src/ssa/node.rs index 5504325ed64..47b4f75fc1a 100644 --- a/crates/noirc_evaluator/src/ssa/node.rs +++ b/crates/noirc_evaluator/src/ssa/node.rs @@ -620,6 +620,39 @@ pub enum BinaryOp { Assign, } +impl std::fmt::Display for BinaryOp { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let op = match &self { + BinaryOp::Add => "add", + BinaryOp::SafeAdd => "safe_add", + BinaryOp::Sub { .. } => "sub", + BinaryOp::SafeSub { .. } => "safe_sub", + BinaryOp::Mul => "mul", + BinaryOp::SafeMul => "safe_mul", + BinaryOp::Udiv => "udiv", + BinaryOp::Sdiv => "sdiv", + BinaryOp::Urem => "urem", + BinaryOp::Srem => "srem", + BinaryOp::Div => "div", + BinaryOp::Eq => "eq", + BinaryOp::Ne => "ne", + BinaryOp::Ult => "ult", + BinaryOp::Ule => "ule", + BinaryOp::Slt => "slt", + BinaryOp::Sle => "sle", + BinaryOp::Lt => "lt", + BinaryOp::Lte => "lte", + BinaryOp::And => "and", + BinaryOp::Or => "or", + BinaryOp::Xor => "xor", + BinaryOp::Assign => "assign", + BinaryOp::Shl => "shl", + BinaryOp::Shr => "shr", + }; + write!(f, "{op}") + } +} + impl Binary { fn new(operator: BinaryOp, lhs: NodeId, rhs: NodeId) -> Binary { Binary { operator, lhs, rhs, predicate: None }