From 1a0582bfbbc14ee311da79da2b6518b122344b6a Mon Sep 17 00:00:00 2001 From: Tom French Date: Sun, 26 May 2024 05:34:07 +0100 Subject: [PATCH 1/8] chore: break up impls for more granular traits --- acvm-repo/acir/src/circuit/mod.rs | 4 +- .../acir/src/native_types/expression/mod.rs | 130 +++++------------- .../src/native_types/expression/ordering.rs | 8 +- acvm-repo/acir/src/native_types/witness.rs | 4 - .../acvm/src/compiler/transformers/csat.rs | 57 +++++++- acvm-repo/acvm/src/pwg/mod.rs | 2 +- acvm-repo/brillig_vm/src/memory.rs | 44 +++--- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 12 +- 8 files changed, 124 insertions(+), 137 deletions(-) diff --git a/acvm-repo/acir/src/circuit/mod.rs b/acvm-repo/acir/src/circuit/mod.rs index 7632afda423..7f3c1890717 100644 --- a/acvm-repo/acir/src/circuit/mod.rs +++ b/acvm-repo/acir/src/circuit/mod.rs @@ -204,7 +204,7 @@ impl FromStr for OpcodeLocation { } } -impl Circuit { +impl Circuit { pub fn num_vars(&self) -> u32 { self.current_witness_index + 1 } @@ -425,7 +425,7 @@ mod tests { }; let program = Program { functions: vec![circuit], unconstrained_functions: Vec::new() }; - fn read_write Deserialize<'a>>( + fn read_write Deserialize<'a>>( program: Program, ) -> (Program, Program) { let bytes = Program::serialize_program(&program); diff --git a/acvm-repo/acir/src/native_types/expression/mod.rs b/acvm-repo/acir/src/native_types/expression/mod.rs index b34862429e7..1feda5703c8 100644 --- a/acvm-repo/acir/src/native_types/expression/mod.rs +++ b/acvm-repo/acir/src/native_types/expression/mod.rs @@ -42,29 +42,12 @@ impl std::fmt::Display for Expression { } } -impl Expression { - // TODO: possibly remove, and move to noir repo. - pub const fn can_defer_constraint(&self) -> bool { - false - } - +impl Expression { /// Returns the number of multiplication terms pub fn num_mul_terms(&self) -> usize { self.mul_terms.len() } - pub fn from_field(q_c: F) -> Self { - Self { q_c, ..Default::default() } - } - - pub fn one() -> Self { - Self::from_field(F::one()) - } - - pub fn zero() -> Self { - Self::default() - } - /// Adds a new linear term to the `Expression`. pub fn push_addition_term(&mut self, coefficient: F, variable: Witness) { self.linear_combinations.push((coefficient, variable)); @@ -85,6 +68,19 @@ impl Expression { self.mul_terms.is_empty() && self.linear_combinations.is_empty() } + /// Returns a `FieldElement` if the expression represents a constant polynomial. + /// Otherwise returns `None`. + /// + /// Examples: + /// - f(x,y) = x would return `None` + /// - f(x,y) = x + 6 would return `None` + /// - f(x,y) = 2*y + 6 would return `None` + /// - f(x,y) = x + y would return `None` + /// - f(x,y) = 5 would return `FieldElement(5)` + pub fn to_const(&self) -> Option<&F> { + self.is_const().then_some(&self.q_c) + } + /// Returns `true` if highest degree term in the expression is one or less. /// /// - `mul_term` in an expression contains degree-2 terms @@ -122,21 +118,29 @@ impl Expression { self.is_linear() && self.linear_combinations.len() == 1 } + /// Sorts opcode in a deterministic order + /// XXX: We can probably make this more efficient by sorting on each phase. We only care if it is deterministic + pub fn sort(&mut self) { + self.mul_terms.sort_by(|a, b| a.1.cmp(&b.1).then(a.2.cmp(&b.2))); + self.linear_combinations.sort_by(|a, b| a.1.cmp(&b.1)); + } +} + +impl Expression { + pub fn from_field(q_c: F) -> Self { + Self { q_c, ..Default::default() } + } + + pub fn zero() -> Self { + Self::default() + } + pub fn is_zero(&self) -> bool { *self == Self::zero() } - /// Returns a `FieldElement` if the expression represents a constant polynomial. - /// Otherwise returns `None`. - /// - /// Examples: - /// - f(x,y) = x would return `None` - /// - f(x,y) = x + 6 would return `None` - /// - f(x,y) = 2*y + 6 would return `None` - /// - f(x,y) = x + y would return `None` - /// - f(x,y) = 5 would return `FieldElement(5)` - pub fn to_const(&self) -> Option { - self.is_const().then_some(self.q_c) + pub fn one() -> Self { + Self::from_field(F::one()) } /// Returns a `Witness` if the `Expression` can be represented as a degree-1 @@ -161,74 +165,6 @@ impl Expression { None } - /// Sorts opcode in a deterministic order - /// XXX: We can probably make this more efficient by sorting on each phase. We only care if it is deterministic - pub fn sort(&mut self) { - self.mul_terms.sort_by(|a, b| a.1.cmp(&b.1).then(a.2.cmp(&b.2))); - self.linear_combinations.sort_by(|a, b| a.1.cmp(&b.1)); - } - - /// Checks if this expression can fit into one arithmetic identity - /// TODO: This needs to be reworded, arithmetic identity only makes sense in the context - /// TODO of PLONK, whereas we want expressions to be generic. - /// TODO: We just need to reword it to say exactly what its doing and - /// TODO then reference the fact that this is what plonk will accept. - /// TODO alternatively, we can define arithmetic identity in the context of expressions - /// TODO and then reference that. - pub fn fits_in_one_identity(&self, width: usize) -> bool { - // A Polynomial with more than one mul term cannot fit into one opcode - if self.mul_terms.len() > 1 { - return false; - }; - // A Polynomial with more terms than fan-in cannot fit within a single opcode - if self.linear_combinations.len() > width { - return false; - } - - // A polynomial with no mul term and a fan-in that fits inside of the width can fit into a single opcode - if self.mul_terms.is_empty() { - return true; - } - - // A polynomial with width-2 fan-in terms and a single non-zero mul term can fit into one opcode - // Example: Axy + Dz . Notice, that the mul term places a constraint on the first two terms, but not the last term - // XXX: This would change if our arithmetic polynomial equation was changed to Axyz for example, but for now it is not. - if self.linear_combinations.len() <= (width - 2) { - return true; - } - - // We now know that we have a single mul term. We also know that the mul term must match up with two other terms - // A polynomial whose mul terms are non zero which do not match up with two terms in the fan-in cannot fit into one opcode - // An example of this is: Axy + Bx + Cy + ... - // Notice how the bivariate monomial xy has two univariate monomials with their respective coefficients - // XXX: note that if x or y is zero, then we could apply a further optimization, but this would be done in another algorithm. - // It would be the same as when we have zero coefficients - Can only work if wire is constrained to be zero publicly - let mul_term = &self.mul_terms[0]; - - // The coefficient should be non-zero, as this method is ran after the compiler removes all zero coefficient terms - assert_ne!(mul_term.0, F::zero()); - - let mut found_x = false; - let mut found_y = false; - - for term in self.linear_combinations.iter() { - let witness = &term.1; - let x = &mul_term.1; - let y = &mul_term.2; - if witness == x { - found_x = true; - }; - if witness == y { - found_y = true; - }; - if found_x & found_y { - break; - } - } - - found_x & found_y - } - /// Returns `self + k*b` pub fn add_mul(&self, k: F, b: &Self) -> Self { if k.is_zero() { diff --git a/acvm-repo/acir/src/native_types/expression/ordering.rs b/acvm-repo/acir/src/native_types/expression/ordering.rs index b0e5e88454f..f1d217b3bd9 100644 --- a/acvm-repo/acir/src/native_types/expression/ordering.rs +++ b/acvm-repo/acir/src/native_types/expression/ordering.rs @@ -1,5 +1,3 @@ -use acir_field::AcirField; - use crate::native_types::Witness; use std::cmp::Ordering; @@ -8,7 +6,7 @@ use super::Expression; // TODO: It's undecided whether `Expression` should implement `Ord/PartialOrd`. // This is currently used in ACVM in the compiler. -impl Ord for Expression { +impl Ord for Expression { fn cmp(&self, other: &Self) -> Ordering { let mut i1 = self.get_max_idx(); let mut i2 = other.get_max_idx(); @@ -25,7 +23,7 @@ impl Ord for Expression { } } -impl PartialOrd for Expression { +impl PartialOrd for Expression { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } @@ -37,7 +35,7 @@ struct WitnessIdx { second_term: bool, } -impl Expression { +impl Expression { fn get_max_idx(&self) -> WitnessIdx { WitnessIdx { linear: self.linear_combinations.len(), diff --git a/acvm-repo/acir/src/native_types/witness.rs b/acvm-repo/acir/src/native_types/witness.rs index 3e9beb510bc..a570968f948 100644 --- a/acvm-repo/acir/src/native_types/witness.rs +++ b/acvm-repo/acir/src/native_types/witness.rs @@ -17,10 +17,6 @@ impl Witness { // This is safe as long as the architecture is 32bits minimum self.0 as usize } - - pub const fn can_defer_constraint(&self) -> bool { - true - } } impl From for Witness { diff --git a/acvm-repo/acvm/src/compiler/transformers/csat.rs b/acvm-repo/acvm/src/compiler/transformers/csat.rs index 6cf74c04205..f2a3cc2c84e 100644 --- a/acvm-repo/acvm/src/compiler/transformers/csat.rs +++ b/acvm-repo/acvm/src/compiler/transformers/csat.rs @@ -326,7 +326,7 @@ impl CSatTransformer { // First check if this polynomial actually needs a partial opcode optimization // There is the chance that it fits perfectly within the assert-zero opcode - if opcode.fits_in_one_identity(self.width) { + if fits_in_one_identity(&opcode, self.width) { return opcode; } @@ -409,6 +409,61 @@ impl CSatTransformer { } } +/// Checks if this expression can fit into one arithmetic identity +fn fits_in_one_identity(expr: &Expression, width: usize) -> bool { + // A Polynomial with more than one mul term cannot fit into one opcode + if expr.mul_terms.len() > 1 { + return false; + }; + // A Polynomial with more terms than fan-in cannot fit within a single opcode + if expr.linear_combinations.len() > width { + return false; + } + + // A polynomial with no mul term and a fan-in that fits inside of the width can fit into a single opcode + if expr.mul_terms.is_empty() { + return true; + } + + // A polynomial with width-2 fan-in terms and a single non-zero mul term can fit into one opcode + // Example: Axy + Dz . Notice, that the mul term places a constraint on the first two terms, but not the last term + // XXX: This would change if our arithmetic polynomial equation was changed to Axyz for example, but for now it is not. + if expr.linear_combinations.len() <= (width - 2) { + return true; + } + + // We now know that we have a single mul term. We also know that the mul term must match up with two other terms + // A polynomial whose mul terms are non zero which do not match up with two terms in the fan-in cannot fit into one opcode + // An example of this is: Axy + Bx + Cy + ... + // Notice how the bivariate monomial xy has two univariate monomials with their respective coefficients + // XXX: note that if x or y is zero, then we could apply a further optimization, but this would be done in another algorithm. + // It would be the same as when we have zero coefficients - Can only work if wire is constrained to be zero publicly + let mul_term = &expr.mul_terms[0]; + + // The coefficient should be non-zero, as this method is ran after the compiler removes all zero coefficient terms + assert_ne!(mul_term.0, F::zero()); + + let mut found_x = false; + let mut found_y = false; + + for term in expr.linear_combinations.iter() { + let witness = &term.1; + let x = &mul_term.1; + let y = &mul_term.2; + if witness == x { + found_x = true; + }; + if witness == y { + found_y = true; + }; + if found_x & found_y { + break; + } + } + + found_x & found_y +} + #[cfg(test)] mod tests { use super::*; diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index da4510db63a..4f88e17d109 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -637,7 +637,7 @@ pub fn get_value( ) -> Result> { let expr = ExpressionSolver::evaluate(expr, initial_witness); match expr.to_const() { - Some(value) => Ok(value), + Some(value) => Ok(*value), None => Err(OpcodeResolutionError::OpcodeNotSolvable( OpcodeNotSolvable::MissingAssignment(any_witness_from_expression(&expr).unwrap().0), )), diff --git a/acvm-repo/brillig_vm/src/memory.rs b/acvm-repo/brillig_vm/src/memory.rs index 4092cd06ae0..cc33eb5d6cf 100644 --- a/acvm-repo/brillig_vm/src/memory.rs +++ b/acvm-repo/brillig_vm/src/memory.rs @@ -16,6 +16,29 @@ pub enum MemoryTypeError { MismatchedBitSize { value_bit_size: u32, expected_bit_size: u32 }, } +impl MemoryValue { + /// Builds a field-typed memory value. + pub fn new_field(value: F) -> Self { + MemoryValue::Field(value) + } + + /// Extracts the field element from the memory value, if it is typed as field element. + pub fn extract_field(&self) -> Option<&F> { + match self { + MemoryValue::Field(value) => Some(value), + _ => None, + } + } + + /// Extracts the integer from the memory value, if it is typed as integer. + pub fn extract_integer(&self) -> Option<(&BigUint, u32)> { + match self { + MemoryValue::Integer(value, bit_size) => Some((value, *bit_size)), + _ => None, + } + } +} + impl MemoryValue { /// Builds a memory value from a field element. pub fn new_from_field(value: F, bit_size: u32) -> Self { @@ -44,11 +67,6 @@ impl MemoryValue { Some(MemoryValue::new_from_field(value, bit_size)) } - /// Builds a field-typed memory value. - pub fn new_field(value: F) -> Self { - MemoryValue::Field(value) - } - /// Builds an integer-typed memory value. pub fn new_integer(value: BigUint, bit_size: u32) -> Self { assert!( @@ -58,22 +76,6 @@ impl MemoryValue { MemoryValue::Integer(value, bit_size) } - /// Extracts the field element from the memory value, if it is typed as field element. - pub fn extract_field(&self) -> Option<&F> { - match self { - MemoryValue::Field(value) => Some(value), - _ => None, - } - } - - /// Extracts the integer from the memory value, if it is typed as integer. - pub fn extract_integer(&self) -> Option<(&BigUint, u32)> { - match self { - MemoryValue::Integer(value, bit_size) => Some((value, *bit_size)), - _ => None, - } - } - /// Converts the memory value to a field element, independent of its type. pub fn to_field(&self) -> F { match self { diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 4a0f9f798ff..0c826cc8a38 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -261,7 +261,7 @@ impl AcirContext { // Check if a witness has been assigned this value already, if so reuse it. *self .constant_witnesses - .entry(constant) + .entry(*constant) .or_insert_with(|| self.acir_ir.get_or_create_witness(&expression)) } else { self.acir_ir.get_or_create_witness(&expression) @@ -724,7 +724,7 @@ impl AcirContext { // If `lhs` and `rhs` are known constants then we can calculate the result at compile time. // `rhs` must be non-zero. - (Some(lhs_const), Some(rhs_const), _) if rhs_const != FieldElement::zero() => { + (Some(lhs_const), Some(rhs_const), _) if !rhs_const.is_zero() => { let quotient = lhs_const.to_u128() / rhs_const.to_u128(); let remainder = lhs_const.to_u128() - quotient * rhs_const.to_u128(); @@ -734,7 +734,7 @@ impl AcirContext { } // If `rhs` is one then the division is a noop. - (_, Some(rhs_const), _) if rhs_const == FieldElement::one() => { + (_, Some(rhs_const), _) if rhs_const.is_one() => { return Ok((lhs, zero)); } @@ -1949,7 +1949,7 @@ impl From> for AcirVarData { fn from(expr: Expression) -> Self { // Prefer simpler variants if possible. if let Some(constant) = expr.to_const() { - AcirVarData::from(constant) + AcirVarData::from(*constant) } else if let Some(witness) = expr.to_witness() { AcirVarData::from(witness) } else { @@ -1979,12 +1979,12 @@ fn execute_brillig( for input in inputs { match input { BrilligInputs::Single(expr) => { - calldata.push(expr.to_const()?); + calldata.push(*expr.to_const()?); } BrilligInputs::Array(expr_arr) => { // Attempt to fetch all array input values for expr in expr_arr.iter() { - calldata.push(expr.to_const()?); + calldata.push(*expr.to_const()?); } } BrilligInputs::MemoryArray(_) => { From c463f2c5ccee96b5b99aa274910b2a9fba8527a6 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sun, 26 May 2024 18:15:54 +0100 Subject: [PATCH 2/8] chore: separate trait from field implementation --- acvm-repo/acir_field/src/field_element.rs | 544 +++++++++++++++++++++ acvm-repo/acir_field/src/generic_ark.rs | 565 +--------------------- acvm-repo/acir_field/src/lib.rs | 8 +- 3 files changed, 559 insertions(+), 558 deletions(-) create mode 100644 acvm-repo/acir_field/src/field_element.rs diff --git a/acvm-repo/acir_field/src/field_element.rs b/acvm-repo/acir_field/src/field_element.rs new file mode 100644 index 00000000000..245ce7af1b3 --- /dev/null +++ b/acvm-repo/acir_field/src/field_element.rs @@ -0,0 +1,544 @@ +use ark_ff::PrimeField; +use ark_ff::Zero; +use num_bigint::BigUint; +use serde::{Deserialize, Serialize}; +use std::borrow::Cow; +use std::ops::{Add, AddAssign, Div, Mul, Neg, Sub, SubAssign}; + +use crate::AcirField; + +// XXX: Switch out for a trait and proper implementations +// This implementation is in-efficient, can definitely remove hex usage and Iterator instances for trivial functionality +#[derive(Default, Clone, Copy, Eq, PartialOrd, Ord)] +pub struct FieldElement(F); + +impl std::fmt::Display for FieldElement { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + // First check if the number is zero + // + let number = BigUint::from_bytes_be(&self.to_be_bytes()); + if number == BigUint::zero() { + return write!(f, "0"); + } + // Check if the negative version is smaller to represent + // + let minus_number = BigUint::from_bytes_be(&(self.neg()).to_be_bytes()); + let (smaller_repr, is_negative) = + if minus_number.to_string().len() < number.to_string().len() { + (minus_number, true) + } else { + (number, false) + }; + if is_negative { + write!(f, "-")?; + } + + // Number of bits needed to represent the smaller representation + let num_bits = smaller_repr.bits(); + + // Check if the number represents a power of 2 + if smaller_repr.count_ones() == 1 { + let mut bit_index = 0; + for i in 0..num_bits { + if smaller_repr.bit(i) { + bit_index = i; + break; + } + } + return match bit_index { + 0 => write!(f, "1"), + 1 => write!(f, "2"), + 2 => write!(f, "4"), + 3 => write!(f, "8"), + _ => write!(f, "2{}", superscript(bit_index)), + }; + } + + // Check if number is a multiple of a power of 2. + // This is used because when computing the quotient + // we usually have numbers in the form 2^t * q + r + // We focus on 2^64, 2^32, 2^16, 2^8, 2^4 because + // they are common. We could extend this to a more + // general factorization strategy, but we pay in terms of CPU time + let mul_sign = "×"; + for power in [64, 32, 16, 8, 4] { + let power_of_two = BigUint::from(2_u128).pow(power); + if &smaller_repr % &power_of_two == BigUint::zero() { + return write!( + f, + "2{}{}{}", + superscript(power as u64), + mul_sign, + smaller_repr / &power_of_two, + ); + } + } + write!(f, "{smaller_repr}") + } +} + +impl std::fmt::Debug for FieldElement { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Display::fmt(self, f) + } +} + +impl std::hash::Hash for FieldElement { + fn hash(&self, state: &mut H) { + state.write(&self.to_be_bytes()); + } +} + +impl PartialEq for FieldElement { + fn eq(&self, other: &Self) -> bool { + self.to_be_bytes() == other.to_be_bytes() + } +} + +impl From for FieldElement { + fn from(mut a: i128) -> FieldElement { + let mut negative = false; + if a < 0 { + a = -a; + negative = true; + } + + let mut result = match F::from_str(&a.to_string()) { + Ok(result) => result, + Err(_) => panic!("Cannot convert i128 as a string to a field element"), + }; + + if negative { + result = -result; + } + FieldElement(result) + } +} + +impl Serialize for FieldElement { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.to_hex().serialize(serializer) + } +} + +impl<'de, T: ark_ff::PrimeField> Deserialize<'de> for FieldElement { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let s: Cow<'de, str> = Deserialize::deserialize(deserializer)?; + match Self::from_hex(&s) { + Some(value) => Ok(value), + None => Err(serde::de::Error::custom(format!("Invalid hex for FieldElement: {s}",))), + } + } +} + +impl From for FieldElement { + fn from(a: u128) -> FieldElement { + let result = match F::from_str(&a.to_string()) { + Ok(result) => result, + Err(_) => panic!("Cannot convert u128 as a string to a field element"), + }; + FieldElement(result) + } +} + +impl From for FieldElement { + fn from(a: usize) -> FieldElement { + FieldElement::from(a as u128) + } +} + +impl From for FieldElement { + fn from(boolean: bool) -> FieldElement { + if boolean { + FieldElement::one() + } else { + FieldElement::zero() + } + } +} + +impl FieldElement { + pub fn from_repr(field: F) -> Self { + Self(field) + } + + // XXX: This method is used while this field element + // implementation is not generic. + pub fn into_repr(self) -> F { + self.0 + } + + fn is_negative(&self) -> bool { + self.neg().num_bits() < self.num_bits() + } + + fn fits_in_u128(&self) -> bool { + self.num_bits() <= 128 + } + + /// Returns None, if the string is not a canonical + /// representation of a field element; less than the order + /// or if the hex string is invalid. + /// This method can be used for both hex and decimal representations. + pub fn try_from_str(input: &str) -> Option> { + if input.contains('x') { + return FieldElement::from_hex(input); + } + + let fr = F::from_str(input).ok()?; + Some(FieldElement(fr)) + } + + // mask_to methods will not remove any bytes from the field + // they are simply zeroed out + // Whereas truncate_to will remove those bits and make the byte array smaller + fn mask_to_be_bytes(&self, num_bits: u32) -> Vec { + let mut bytes = self.to_be_bytes(); + mask_vector_le(&mut bytes, num_bits as usize); + bytes + } + + fn bits(&self) -> Vec { + fn byte_to_bit(byte: u8) -> Vec { + let mut bits = Vec::with_capacity(8); + for index in (0..=7).rev() { + bits.push((byte & (1 << index)) >> index == 1); + } + bits + } + + let bytes = self.to_be_bytes(); + let mut bits = Vec::with_capacity(bytes.len() * 8); + for byte in bytes { + let _bits = byte_to_bit(byte); + bits.extend(_bits); + } + bits + } + + fn and_xor(&self, rhs: &FieldElement, num_bits: u32, is_xor: bool) -> FieldElement { + // XXX: Gadgets like SHA256 need to have their input be a multiple of 8 + // This is not a restriction caused by SHA256, as it works on bits + // but most backends assume bytes. + // We could implicitly pad, however this may not be intuitive for users. + // assert!( + // num_bits % 8 == 0, + // "num_bits is not a multiple of 8, it is {}", + // num_bits + // ); + + let lhs_bytes = self.mask_to_be_bytes(num_bits); + let rhs_bytes = rhs.mask_to_be_bytes(num_bits); + + let and_byte_arr: Vec<_> = lhs_bytes + .into_iter() + .zip(rhs_bytes) + .map(|(lhs, rhs)| if is_xor { lhs ^ rhs } else { lhs & rhs }) + .collect(); + + FieldElement::from_be_bytes_reduce(&and_byte_arr) + } +} + +impl AcirField for FieldElement { + fn one() -> FieldElement { + FieldElement(F::one()) + } + fn zero() -> FieldElement { + FieldElement(F::zero()) + } + + fn is_zero(&self) -> bool { + self == &Self::zero() + } + fn is_one(&self) -> bool { + self == &Self::one() + } + + fn pow(&self, exponent: &Self) -> Self { + FieldElement(self.0.pow(exponent.0.into_bigint())) + } + + /// Maximum number of bits needed to represent a field element + /// This is not the amount of bits being used to represent a field element + /// Example, you only need 254 bits to represent a field element in BN256 + /// But the representation uses 256 bits, so the top two bits are always zero + /// This method would return 254 + fn max_num_bits() -> u32 { + F::MODULUS_BIT_SIZE + } + + /// Maximum numbers of bytes needed to represent a field element + /// We are not guaranteed that the number of bits being used to represent a field element + /// will always be divisible by 8. If the case that it is not, we add one to the max number of bytes + /// For example, a max bit size of 254 would give a max byte size of 32. + fn max_num_bytes() -> u32 { + let num_bytes = Self::max_num_bits() / 8; + if Self::max_num_bits() % 8 == 0 { + num_bytes + } else { + num_bytes + 1 + } + } + + fn modulus() -> BigUint { + F::MODULUS.into() + } + + /// This is the number of bits required to represent this specific field element + fn num_bits(&self) -> u32 { + let bits = self.bits(); + // Iterate the number of bits and pop off all leading zeroes + let iter = bits.iter().skip_while(|x| !(**x)); + // Note: count will panic if it goes over usize::MAX. + // This may not be suitable for devices whose usize < u16 + iter.count() as u32 + } + + fn to_u128(self) -> u128 { + let bytes = self.to_be_bytes(); + u128::from_be_bytes(bytes[16..32].try_into().unwrap()) + } + + fn try_into_u128(self) -> Option { + self.fits_in_u128().then(|| self.to_u128()) + } + + fn to_i128(self) -> i128 { + let is_negative = self.is_negative(); + let bytes = if is_negative { self.neg() } else { self }.to_be_bytes(); + i128::from_be_bytes(bytes[16..32].try_into().unwrap()) * if is_negative { -1 } else { 1 } + } + + fn try_to_u64(&self) -> Option { + (self.num_bits() <= 64).then(|| self.to_u128() as u64) + } + + /// Computes the inverse or returns zero if the inverse does not exist + /// Before using this FieldElement, please ensure that this behavior is necessary + fn inverse(&self) -> FieldElement { + let inv = self.0.inverse().unwrap_or_else(F::zero); + FieldElement(inv) + } + + fn to_hex(self) -> String { + let mut bytes = Vec::new(); + self.0.serialize_uncompressed(&mut bytes).unwrap(); + bytes.reverse(); + hex::encode(bytes) + } + fn from_hex(hex_str: &str) -> Option> { + let value = hex_str.strip_prefix("0x").unwrap_or(hex_str); + // Values of odd length require an additional "0" prefix + let sanitized_value = + if value.len() % 2 == 0 { value.to_string() } else { format!("0{}", value) }; + let hex_as_bytes = hex::decode(sanitized_value).ok()?; + Some(FieldElement::from_be_bytes_reduce(&hex_as_bytes)) + } + + fn to_be_bytes(self) -> Vec { + // to_be_bytes! uses little endian which is why we reverse the output + // TODO: Add a little endian equivalent, so the caller can use whichever one + // TODO they desire + let mut bytes = Vec::new(); + self.0.serialize_uncompressed(&mut bytes).unwrap(); + bytes.reverse(); + bytes + } + + /// Converts bytes into a FieldElement and applies a + /// reduction if needed. + fn from_be_bytes_reduce(bytes: &[u8]) -> FieldElement { + FieldElement(F::from_be_bytes_mod_order(bytes)) + } + + /// Returns the closest number of bytes to the bits specified + /// This method truncates + fn fetch_nearest_bytes(&self, num_bits: usize) -> Vec { + fn nearest_bytes(num_bits: usize) -> usize { + ((num_bits + 7) / 8) * 8 + } + + let num_bytes = nearest_bytes(num_bits); + let num_elements = num_bytes / 8; + + let mut bytes = self.to_be_bytes(); + bytes.reverse(); // put it in big endian format. XXX(next refactor): we should be explicit about endianness. + + bytes[0..num_elements].to_vec() + } + + fn and(&self, rhs: &FieldElement, num_bits: u32) -> FieldElement { + self.and_xor(rhs, num_bits, false) + } + fn xor(&self, rhs: &FieldElement, num_bits: u32) -> FieldElement { + self.and_xor(rhs, num_bits, true) + } +} + +impl Neg for FieldElement { + type Output = FieldElement; + + fn neg(self) -> Self::Output { + FieldElement(-self.0) + } +} + +impl Mul for FieldElement { + type Output = FieldElement; + fn mul(mut self, rhs: FieldElement) -> Self::Output { + self.0.mul_assign(&rhs.0); + FieldElement(self.0) + } +} +impl Div for FieldElement { + type Output = FieldElement; + #[allow(clippy::suspicious_arithmetic_impl)] + fn div(self, rhs: FieldElement) -> Self::Output { + self * rhs.inverse() + } +} +impl Add for FieldElement { + type Output = FieldElement; + fn add(mut self, rhs: FieldElement) -> Self::Output { + self.0.add_assign(&rhs.0); + FieldElement(self.0) + } +} +impl AddAssign for FieldElement { + fn add_assign(&mut self, rhs: FieldElement) { + self.0.add_assign(&rhs.0); + } +} + +impl Sub for FieldElement { + type Output = FieldElement; + fn sub(mut self, rhs: FieldElement) -> Self::Output { + self.0.sub_assign(&rhs.0); + FieldElement(self.0) + } +} +impl SubAssign for FieldElement { + fn sub_assign(&mut self, rhs: FieldElement) { + self.0.sub_assign(&rhs.0); + } +} + +fn mask_vector_le(bytes: &mut [u8], num_bits: usize) { + // reverse to big endian format + bytes.reverse(); + + let mask_power = num_bits % 8; + let array_mask_index = num_bits / 8; + + for (index, byte) in bytes.iter_mut().enumerate() { + match index.cmp(&array_mask_index) { + std::cmp::Ordering::Less => { + // do nothing if the current index is less than + // the array index. + } + std::cmp::Ordering::Equal => { + let mask = 2u8.pow(mask_power as u32) - 1; + // mask the byte + *byte &= mask; + } + std::cmp::Ordering::Greater => { + // Anything greater than the array index + // will be set to zero + *byte = 0; + } + } + } + // reverse back to little endian + bytes.reverse(); +} + +// For pretty printing powers +fn superscript(n: u64) -> String { + if n == 0 { + "⁰".to_owned() + } else if n == 1 { + "¹".to_owned() + } else if n == 2 { + "²".to_owned() + } else if n == 3 { + "³".to_owned() + } else if n == 4 { + "⁴".to_owned() + } else if n == 5 { + "⁵".to_owned() + } else if n == 6 { + "⁶".to_owned() + } else if n == 7 { + "⁷".to_owned() + } else if n == 8 { + "⁸".to_owned() + } else if n == 9 { + "⁹".to_owned() + } else if n >= 10 { + superscript(n / 10) + &superscript(n % 10) + } else { + panic!("{}", n.to_string() + " can't be converted to superscript."); + } +} + +#[cfg(test)] +mod tests { + use super::{AcirField, FieldElement}; + + #[test] + fn and() { + let max = 10_000u32; + + let num_bits = (std::mem::size_of::() * 8) as u32 - max.leading_zeros(); + + for x in 0..max { + let x = FieldElement::::from(x as i128); + let res = x.and(&x, num_bits); + assert_eq!(res.to_be_bytes(), x.to_be_bytes()); + } + } + + #[test] + fn serialize_fixed_test_vectors() { + // Serialized field elements from of 0, -1, -2, -3 + let hex_strings = vec![ + "0000000000000000000000000000000000000000000000000000000000000000", + "30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000", + "30644e72e131a029b85045b68181585d2833e84879b9709143e1f593efffffff", + "30644e72e131a029b85045b68181585d2833e84879b9709143e1f593effffffe", + ]; + + for (i, string) in hex_strings.into_iter().enumerate() { + let minus_i_field_element = -FieldElement::::from(i as i128); + assert_eq!(minus_i_field_element.to_hex(), string); + } + } + + #[test] + fn deserialize_even_and_odd_length_hex() { + // Test cases of (odd, even) length hex strings + let hex_strings = + vec![("0x0", "0x00"), ("0x1", "0x01"), ("0x002", "0x0002"), ("0x00003", "0x000003")]; + for (i, case) in hex_strings.into_iter().enumerate() { + let i_field_element = FieldElement::::from(i as i128); + let odd_field_element = FieldElement::::from_hex(case.0).unwrap(); + let even_field_element = FieldElement::::from_hex(case.1).unwrap(); + + assert_eq!(i_field_element, odd_field_element); + assert_eq!(odd_field_element, even_field_element); + } + } + + #[test] + fn max_num_bits_smoke() { + let max_num_bits_bn254 = FieldElement::::max_num_bits(); + assert_eq!(max_num_bits_bn254, 254); + } +} diff --git a/acvm-repo/acir_field/src/generic_ark.rs b/acvm-repo/acir_field/src/generic_ark.rs index c1bc7971922..1e0215223ef 100644 --- a/acvm-repo/acir_field/src/generic_ark.rs +++ b/acvm-repo/acir_field/src/generic_ark.rs @@ -1,25 +1,21 @@ -use ark_ff::PrimeField; -use ark_ff::Zero; use num_bigint::BigUint; -use serde::{Deserialize, Serialize}; -use std::borrow::Cow; /// This trait is extremely unstable and WILL have breaking changes. pub trait AcirField: std::marker::Sized - + Display - + Debug + + std::fmt::Display + + std::fmt::Debug + Default + Clone + Copy - + Neg - + Add - + Sub - + Mul - + Div + + std::ops::Neg + + std::ops::Add + + std::ops::Sub + + std::ops::Mul + + std::ops::Div + + std::ops::AddAssign + + std::ops::SubAssign + PartialOrd - + AddAssign - + SubAssign + From + From // + From @@ -27,7 +23,7 @@ pub trait AcirField: // + From // + From + From - + Hash + + std::hash::Hash + std::cmp::Eq { fn one() -> Self; @@ -84,544 +80,3 @@ pub trait AcirField: fn and(&self, rhs: &Self, num_bits: u32) -> Self; fn xor(&self, rhs: &Self, num_bits: u32) -> Self; } - -// XXX: Switch out for a trait and proper implementations -// This implementation is in-efficient, can definitely remove hex usage and Iterator instances for trivial functionality -#[derive(Default, Clone, Copy, Eq, PartialOrd, Ord)] -pub struct FieldElement(F); - -impl std::fmt::Display for FieldElement { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - // First check if the number is zero - // - let number = BigUint::from_bytes_be(&self.to_be_bytes()); - if number == BigUint::zero() { - return write!(f, "0"); - } - // Check if the negative version is smaller to represent - // - let minus_number = BigUint::from_bytes_be(&(self.neg()).to_be_bytes()); - let (smaller_repr, is_negative) = - if minus_number.to_string().len() < number.to_string().len() { - (minus_number, true) - } else { - (number, false) - }; - if is_negative { - write!(f, "-")?; - } - - // Number of bits needed to represent the smaller representation - let num_bits = smaller_repr.bits(); - - // Check if the number represents a power of 2 - if smaller_repr.count_ones() == 1 { - let mut bit_index = 0; - for i in 0..num_bits { - if smaller_repr.bit(i) { - bit_index = i; - break; - } - } - return match bit_index { - 0 => write!(f, "1"), - 1 => write!(f, "2"), - 2 => write!(f, "4"), - 3 => write!(f, "8"), - _ => write!(f, "2{}", superscript(bit_index)), - }; - } - - // Check if number is a multiple of a power of 2. - // This is used because when computing the quotient - // we usually have numbers in the form 2^t * q + r - // We focus on 2^64, 2^32, 2^16, 2^8, 2^4 because - // they are common. We could extend this to a more - // general factorization strategy, but we pay in terms of CPU time - let mul_sign = "×"; - for power in [64, 32, 16, 8, 4] { - let power_of_two = BigUint::from(2_u128).pow(power); - if &smaller_repr % &power_of_two == BigUint::zero() { - return write!( - f, - "2{}{}{}", - superscript(power as u64), - mul_sign, - smaller_repr / &power_of_two, - ); - } - } - write!(f, "{smaller_repr}") - } -} - -impl std::fmt::Debug for FieldElement { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - std::fmt::Display::fmt(self, f) - } -} - -impl std::hash::Hash for FieldElement { - fn hash(&self, state: &mut H) { - state.write(&self.to_be_bytes()); - } -} - -impl PartialEq for FieldElement { - fn eq(&self, other: &Self) -> bool { - self.to_be_bytes() == other.to_be_bytes() - } -} - -impl From for FieldElement { - fn from(mut a: i128) -> FieldElement { - let mut negative = false; - if a < 0 { - a = -a; - negative = true; - } - - let mut result = match F::from_str(&a.to_string()) { - Ok(result) => result, - Err(_) => panic!("Cannot convert i128 as a string to a field element"), - }; - - if negative { - result = -result; - } - FieldElement(result) - } -} - -impl Serialize for FieldElement { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - self.to_hex().serialize(serializer) - } -} - -impl<'de, T: ark_ff::PrimeField> Deserialize<'de> for FieldElement { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - let s: Cow<'de, str> = Deserialize::deserialize(deserializer)?; - match Self::from_hex(&s) { - Some(value) => Ok(value), - None => Err(serde::de::Error::custom(format!("Invalid hex for FieldElement: {s}",))), - } - } -} - -impl From for FieldElement { - fn from(a: u128) -> FieldElement { - let result = match F::from_str(&a.to_string()) { - Ok(result) => result, - Err(_) => panic!("Cannot convert u128 as a string to a field element"), - }; - FieldElement(result) - } -} - -impl From for FieldElement { - fn from(a: usize) -> FieldElement { - FieldElement::from(a as u128) - } -} - -impl From for FieldElement { - fn from(boolean: bool) -> FieldElement { - if boolean { - FieldElement::one() - } else { - FieldElement::zero() - } - } -} - -impl FieldElement { - pub fn from_repr(field: F) -> Self { - Self(field) - } - - // XXX: This method is used while this field element - // implementation is not generic. - pub fn into_repr(self) -> F { - self.0 - } - - fn is_negative(&self) -> bool { - self.neg().num_bits() < self.num_bits() - } - - fn fits_in_u128(&self) -> bool { - self.num_bits() <= 128 - } - - /// Returns None, if the string is not a canonical - /// representation of a field element; less than the order - /// or if the hex string is invalid. - /// This method can be used for both hex and decimal representations. - pub fn try_from_str(input: &str) -> Option> { - if input.contains('x') { - return FieldElement::from_hex(input); - } - - let fr = F::from_str(input).ok()?; - Some(FieldElement(fr)) - } - - // mask_to methods will not remove any bytes from the field - // they are simply zeroed out - // Whereas truncate_to will remove those bits and make the byte array smaller - fn mask_to_be_bytes(&self, num_bits: u32) -> Vec { - let mut bytes = self.to_be_bytes(); - mask_vector_le(&mut bytes, num_bits as usize); - bytes - } - - fn bits(&self) -> Vec { - fn byte_to_bit(byte: u8) -> Vec { - let mut bits = Vec::with_capacity(8); - for index in (0..=7).rev() { - bits.push((byte & (1 << index)) >> index == 1); - } - bits - } - - let bytes = self.to_be_bytes(); - let mut bits = Vec::with_capacity(bytes.len() * 8); - for byte in bytes { - let _bits = byte_to_bit(byte); - bits.extend(_bits); - } - bits - } - - fn and_xor(&self, rhs: &FieldElement, num_bits: u32, is_xor: bool) -> FieldElement { - // XXX: Gadgets like SHA256 need to have their input be a multiple of 8 - // This is not a restriction caused by SHA256, as it works on bits - // but most backends assume bytes. - // We could implicitly pad, however this may not be intuitive for users. - // assert!( - // num_bits % 8 == 0, - // "num_bits is not a multiple of 8, it is {}", - // num_bits - // ); - - let lhs_bytes = self.mask_to_be_bytes(num_bits); - let rhs_bytes = rhs.mask_to_be_bytes(num_bits); - - let and_byte_arr: Vec<_> = lhs_bytes - .into_iter() - .zip(rhs_bytes) - .map(|(lhs, rhs)| if is_xor { lhs ^ rhs } else { lhs & rhs }) - .collect(); - - FieldElement::from_be_bytes_reduce(&and_byte_arr) - } -} - -impl AcirField for FieldElement { - fn one() -> FieldElement { - FieldElement(F::one()) - } - fn zero() -> FieldElement { - FieldElement(F::zero()) - } - - fn is_zero(&self) -> bool { - self == &Self::zero() - } - fn is_one(&self) -> bool { - self == &Self::one() - } - - fn pow(&self, exponent: &Self) -> Self { - FieldElement(self.0.pow(exponent.0.into_bigint())) - } - - /// Maximum number of bits needed to represent a field element - /// This is not the amount of bits being used to represent a field element - /// Example, you only need 254 bits to represent a field element in BN256 - /// But the representation uses 256 bits, so the top two bits are always zero - /// This method would return 254 - fn max_num_bits() -> u32 { - F::MODULUS_BIT_SIZE - } - - /// Maximum numbers of bytes needed to represent a field element - /// We are not guaranteed that the number of bits being used to represent a field element - /// will always be divisible by 8. If the case that it is not, we add one to the max number of bytes - /// For example, a max bit size of 254 would give a max byte size of 32. - fn max_num_bytes() -> u32 { - let num_bytes = Self::max_num_bits() / 8; - if Self::max_num_bits() % 8 == 0 { - num_bytes - } else { - num_bytes + 1 - } - } - - fn modulus() -> BigUint { - F::MODULUS.into() - } - - /// This is the number of bits required to represent this specific field element - fn num_bits(&self) -> u32 { - let bits = self.bits(); - // Iterate the number of bits and pop off all leading zeroes - let iter = bits.iter().skip_while(|x| !(**x)); - // Note: count will panic if it goes over usize::MAX. - // This may not be suitable for devices whose usize < u16 - iter.count() as u32 - } - - fn to_u128(self) -> u128 { - let bytes = self.to_be_bytes(); - u128::from_be_bytes(bytes[16..32].try_into().unwrap()) - } - - fn try_into_u128(self) -> Option { - self.fits_in_u128().then(|| self.to_u128()) - } - - fn to_i128(self) -> i128 { - let is_negative = self.is_negative(); - let bytes = if is_negative { self.neg() } else { self }.to_be_bytes(); - i128::from_be_bytes(bytes[16..32].try_into().unwrap()) * if is_negative { -1 } else { 1 } - } - - fn try_to_u64(&self) -> Option { - (self.num_bits() <= 64).then(|| self.to_u128() as u64) - } - - /// Computes the inverse or returns zero if the inverse does not exist - /// Before using this FieldElement, please ensure that this behavior is necessary - fn inverse(&self) -> FieldElement { - let inv = self.0.inverse().unwrap_or_else(F::zero); - FieldElement(inv) - } - - fn to_hex(self) -> String { - let mut bytes = Vec::new(); - self.0.serialize_uncompressed(&mut bytes).unwrap(); - bytes.reverse(); - hex::encode(bytes) - } - fn from_hex(hex_str: &str) -> Option> { - let value = hex_str.strip_prefix("0x").unwrap_or(hex_str); - // Values of odd length require an additional "0" prefix - let sanitized_value = - if value.len() % 2 == 0 { value.to_string() } else { format!("0{}", value) }; - let hex_as_bytes = hex::decode(sanitized_value).ok()?; - Some(FieldElement::from_be_bytes_reduce(&hex_as_bytes)) - } - - fn to_be_bytes(self) -> Vec { - // to_be_bytes! uses little endian which is why we reverse the output - // TODO: Add a little endian equivalent, so the caller can use whichever one - // TODO they desire - let mut bytes = Vec::new(); - self.0.serialize_uncompressed(&mut bytes).unwrap(); - bytes.reverse(); - bytes - } - - /// Converts bytes into a FieldElement and applies a - /// reduction if needed. - fn from_be_bytes_reduce(bytes: &[u8]) -> FieldElement { - FieldElement(F::from_be_bytes_mod_order(bytes)) - } - - /// Returns the closest number of bytes to the bits specified - /// This method truncates - fn fetch_nearest_bytes(&self, num_bits: usize) -> Vec { - fn nearest_bytes(num_bits: usize) -> usize { - ((num_bits + 7) / 8) * 8 - } - - let num_bytes = nearest_bytes(num_bits); - let num_elements = num_bytes / 8; - - let mut bytes = self.to_be_bytes(); - bytes.reverse(); // put it in big endian format. XXX(next refactor): we should be explicit about endianness. - - bytes[0..num_elements].to_vec() - } - - fn and(&self, rhs: &FieldElement, num_bits: u32) -> FieldElement { - self.and_xor(rhs, num_bits, false) - } - fn xor(&self, rhs: &FieldElement, num_bits: u32) -> FieldElement { - self.and_xor(rhs, num_bits, true) - } -} - -use std::fmt::Debug; -use std::fmt::Display; -use std::hash::Hash; -use std::ops::{Add, AddAssign, Div, Mul, Neg, Sub, SubAssign}; - -impl Neg for FieldElement { - type Output = FieldElement; - - fn neg(self) -> Self::Output { - FieldElement(-self.0) - } -} - -impl Mul for FieldElement { - type Output = FieldElement; - fn mul(mut self, rhs: FieldElement) -> Self::Output { - self.0.mul_assign(&rhs.0); - FieldElement(self.0) - } -} -impl Div for FieldElement { - type Output = FieldElement; - #[allow(clippy::suspicious_arithmetic_impl)] - fn div(self, rhs: FieldElement) -> Self::Output { - self * rhs.inverse() - } -} -impl Add for FieldElement { - type Output = FieldElement; - fn add(mut self, rhs: FieldElement) -> Self::Output { - self.0.add_assign(&rhs.0); - FieldElement(self.0) - } -} -impl AddAssign for FieldElement { - fn add_assign(&mut self, rhs: FieldElement) { - self.0.add_assign(&rhs.0); - } -} - -impl Sub for FieldElement { - type Output = FieldElement; - fn sub(mut self, rhs: FieldElement) -> Self::Output { - self.0.sub_assign(&rhs.0); - FieldElement(self.0) - } -} -impl SubAssign for FieldElement { - fn sub_assign(&mut self, rhs: FieldElement) { - self.0.sub_assign(&rhs.0); - } -} - -fn mask_vector_le(bytes: &mut [u8], num_bits: usize) { - // reverse to big endian format - bytes.reverse(); - - let mask_power = num_bits % 8; - let array_mask_index = num_bits / 8; - - for (index, byte) in bytes.iter_mut().enumerate() { - match index.cmp(&array_mask_index) { - std::cmp::Ordering::Less => { - // do nothing if the current index is less than - // the array index. - } - std::cmp::Ordering::Equal => { - let mask = 2u8.pow(mask_power as u32) - 1; - // mask the byte - *byte &= mask; - } - std::cmp::Ordering::Greater => { - // Anything greater than the array index - // will be set to zero - *byte = 0; - } - } - } - // reverse back to little endian - bytes.reverse(); -} - -// For pretty printing powers -fn superscript(n: u64) -> String { - if n == 0 { - "⁰".to_owned() - } else if n == 1 { - "¹".to_owned() - } else if n == 2 { - "²".to_owned() - } else if n == 3 { - "³".to_owned() - } else if n == 4 { - "⁴".to_owned() - } else if n == 5 { - "⁵".to_owned() - } else if n == 6 { - "⁶".to_owned() - } else if n == 7 { - "⁷".to_owned() - } else if n == 8 { - "⁸".to_owned() - } else if n == 9 { - "⁹".to_owned() - } else if n >= 10 { - superscript(n / 10) + &superscript(n % 10) - } else { - panic!("{}", n.to_string() + " can't be converted to superscript."); - } -} - -#[cfg(test)] -mod tests { - use super::{AcirField, FieldElement}; - - #[test] - fn and() { - let max = 10_000u32; - - let num_bits = (std::mem::size_of::() * 8) as u32 - max.leading_zeros(); - - for x in 0..max { - let x = FieldElement::::from(x as i128); - let res = x.and(&x, num_bits); - assert_eq!(res.to_be_bytes(), x.to_be_bytes()); - } - } - - #[test] - fn serialize_fixed_test_vectors() { - // Serialized field elements from of 0, -1, -2, -3 - let hex_strings = vec![ - "0000000000000000000000000000000000000000000000000000000000000000", - "30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000", - "30644e72e131a029b85045b68181585d2833e84879b9709143e1f593efffffff", - "30644e72e131a029b85045b68181585d2833e84879b9709143e1f593effffffe", - ]; - - for (i, string) in hex_strings.into_iter().enumerate() { - let minus_i_field_element = -FieldElement::::from(i as i128); - assert_eq!(minus_i_field_element.to_hex(), string); - } - } - - #[test] - fn deserialize_even_and_odd_length_hex() { - // Test cases of (odd, even) length hex strings - let hex_strings = - vec![("0x0", "0x00"), ("0x1", "0x01"), ("0x002", "0x0002"), ("0x00003", "0x000003")]; - for (i, case) in hex_strings.into_iter().enumerate() { - let i_field_element = FieldElement::::from(i as i128); - let odd_field_element = FieldElement::::from_hex(case.0).unwrap(); - let even_field_element = FieldElement::::from_hex(case.1).unwrap(); - - assert_eq!(i_field_element, odd_field_element); - assert_eq!(odd_field_element, even_field_element); - } - } - - #[test] - fn max_num_bits_smoke() { - let max_num_bits_bn254 = FieldElement::::max_num_bits(); - assert_eq!(max_num_bits_bn254, 254); - } -} diff --git a/acvm-repo/acir_field/src/lib.rs b/acvm-repo/acir_field/src/lib.rs index 7f06330ed8d..a4616078b22 100644 --- a/acvm-repo/acir_field/src/lib.rs +++ b/acvm-repo/acir_field/src/lib.rs @@ -5,19 +5,21 @@ use num_bigint::BigUint; use num_traits::Num; + +mod field_element; mod generic_ark; pub use generic_ark::AcirField; /// Temporarily exported generic field to aid migration to `AcirField` -pub use generic_ark::FieldElement as GenericFieldElement; +pub use field_element::FieldElement as GenericFieldElement; cfg_if::cfg_if! { if #[cfg(feature = "bls12_381")] { - pub type FieldElement = generic_ark::FieldElement; + pub type FieldElement = field_element::FieldElement; pub const CHOSEN_FIELD : FieldOptions = FieldOptions::BLS12_381; } else { - pub type FieldElement = generic_ark::FieldElement; + pub type FieldElement = field_element::FieldElement; pub const CHOSEN_FIELD : FieldOptions = FieldOptions::BN254; } } From ea135cac44b30220eb10cb8c15da4da8a76b7391 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 27 May 2024 16:02:12 +0000 Subject: [PATCH 3/8] feat: make brillig gen generic over field implementation --- .../src/brillig/brillig_gen.rs | 7 ++- .../brillig/brillig_gen/brillig_black_box.rs | 19 ++++---- .../src/brillig/brillig_gen/brillig_block.rs | 6 +-- .../brillig_gen/brillig_block_variables.rs | 13 +++--- .../brillig/brillig_gen/brillig_directive.rs | 15 +++---- .../src/brillig/brillig_gen/brillig_fn.rs | 2 +- .../brillig/brillig_gen/brillig_slice_ops.rs | 4 +- .../noirc_evaluator/src/brillig/brillig_ir.rs | 24 +++++----- .../src/brillig/brillig_ir/artifact.rs | 26 +++++------ .../src/brillig/brillig_ir/codegen_binary.rs | 8 ++-- .../src/brillig/brillig_ir/codegen_calls.rs | 7 +-- .../brillig_ir/codegen_control_flow.rs | 27 ++++++----- .../brillig/brillig_ir/codegen_intrinsic.rs | 12 +++-- .../src/brillig/brillig_ir/codegen_memory.rs | 5 ++- .../src/brillig/brillig_ir/codegen_stack.rs | 6 +-- .../src/brillig/brillig_ir/debug_show.rs | 4 +- .../src/brillig/brillig_ir/entry_point.rs | 45 +++++++++---------- .../src/brillig/brillig_ir/instructions.rs | 38 +++++++--------- .../src/brillig/brillig_ir/registers.rs | 2 +- compiler/noirc_evaluator/src/brillig/mod.rs | 11 +++-- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 2 +- .../ssa/acir_gen/acir_ir/generated_acir.rs | 4 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 10 ++--- 23 files changed, 153 insertions(+), 144 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs index 5576d673f1d..ee61a9d13d3 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs @@ -6,12 +6,17 @@ pub(crate) mod brillig_fn; pub(crate) mod brillig_slice_ops; mod variable_liveness; +use acvm::FieldElement; + use self::{brillig_block::BrilligBlock, brillig_fn::FunctionContext}; use super::brillig_ir::{artifact::BrilligArtifact, BrilligContext}; use crate::ssa::ir::function::Function; /// Converting an SSA function into Brillig bytecode. -pub(crate) fn convert_ssa_function(func: &Function, enable_debug_trace: bool) -> BrilligArtifact { +pub(crate) fn convert_ssa_function( + func: &Function, + enable_debug_trace: bool, +) -> BrilligArtifact { let mut brillig_context = BrilligContext::new(enable_debug_trace); let mut function_context = FunctionContext::new(func); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs index f56c5daf315..367cdbe4973 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs @@ -1,18 +1,19 @@ use acvm::{ acir::{brillig::BlackBoxOp, BlackBoxFunc}, - FieldElement, + AcirField, }; use crate::brillig::brillig_ir::{ brillig_variable::{BrilligVariable, BrilligVector, SingleAddrVariable}, + debug_show::DebugToString, BrilligBinaryOp, BrilligContext, }; /// Transforms SSA's black box function calls into the corresponding brillig instructions /// Extracting arguments and results from the SSA function call /// And making any necessary type conversions to adapt noir's blackbox calls to brillig's -pub(crate) fn convert_black_box_call( - brillig_context: &mut BrilligContext, +pub(crate) fn convert_black_box_call( + brillig_context: &mut BrilligContext, bb_func: &BlackBoxFunc, function_arguments: &[BrilligVariable], function_results: &[BrilligVariable], @@ -341,7 +342,7 @@ pub(crate) fn convert_black_box_call( let inputs_vector = convert_array_or_vector(brillig_context, inputs, bb_func); let modulus_vector = convert_array_or_vector(brillig_context, modulus, bb_func); let output_id = brillig_context.get_new_bigint_id(); - brillig_context.const_instruction(*output, FieldElement::from(output_id as u128)); + brillig_context.const_instruction(*output, F::from(output_id as u128)); brillig_context.black_box_op_instruction(BlackBoxOp::BigIntFromLeBytes { inputs: inputs_vector.to_heap_vector(), modulus: modulus_vector.to_heap_vector(), @@ -426,8 +427,8 @@ pub(crate) fn convert_black_box_call( } } -fn convert_array_or_vector( - brillig_context: &mut BrilligContext, +fn convert_array_or_vector( + brillig_context: &mut BrilligContext, array_or_vector: &BrilligVariable, bb_func: &BlackBoxFunc, ) -> BrilligVector { @@ -442,8 +443,8 @@ fn convert_array_or_vector( } } -fn prepare_bigint_output( - brillig_context: &mut BrilligContext, +fn prepare_bigint_output( + brillig_context: &mut BrilligContext, lhs_modulus: &SingleAddrVariable, rhs_modulus: &SingleAddrVariable, output: &SingleAddrVariable, @@ -465,6 +466,6 @@ fn prepare_bigint_output( brillig_context.deallocate_register(condition); // Set output id let output_id = brillig_context.get_new_bigint_id(); - brillig_context.const_instruction(*output, FieldElement::from(output_id as u128)); + brillig_context.const_instruction(*output, F::from(output_id as u128)); brillig_context.mov_instruction(modulus_id.address, lhs_modulus.address); } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 1fa4f41b29c..df7112d437d 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -33,7 +33,7 @@ pub(crate) struct BrilligBlock<'block> { /// The basic block that is being converted pub(crate) block_id: BasicBlockId, /// Context for creating brillig opcodes - pub(crate) brillig_context: &'block mut BrilligContext, + pub(crate) brillig_context: &'block mut BrilligContext, /// Tracks the available variable during the codegen of the block pub(crate) variables: BlockVariables, /// For each instruction, the set of values that are not used anymore after it. @@ -44,7 +44,7 @@ impl<'block> BrilligBlock<'block> { /// Converts an SSA Basic block into a sequence of Brillig opcodes pub(crate) fn compile( function_context: &'block mut FunctionContext, - brillig_context: &'block mut BrilligContext, + brillig_context: &'block mut BrilligContext, block_id: BasicBlockId, dfg: &DataFlowGraph, ) { @@ -944,7 +944,7 @@ impl<'block> BrilligBlock<'block> { } pub(crate) fn store_variable_in_array_with_ctx( - ctx: &mut BrilligContext, + ctx: &mut BrilligContext, destination_pointer: MemoryAddress, index_register: SingleAddrVariable, value_variable: BrilligVariable, diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs index fb9a8577d94..ffbca256d28 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs @@ -1,3 +1,4 @@ +use acvm::FieldElement; use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; use crate::{ @@ -50,7 +51,7 @@ impl BlockVariables { pub(crate) fn define_variable( &mut self, function_context: &mut FunctionContext, - brillig_context: &mut BrilligContext, + brillig_context: &mut BrilligContext, value_id: ValueId, dfg: &DataFlowGraph, ) -> BrilligVariable { @@ -70,7 +71,7 @@ impl BlockVariables { pub(crate) fn define_single_addr_variable( &mut self, function_context: &mut FunctionContext, - brillig_context: &mut BrilligContext, + brillig_context: &mut BrilligContext, value: ValueId, dfg: &DataFlowGraph, ) -> SingleAddrVariable { @@ -83,7 +84,7 @@ impl BlockVariables { &mut self, value_id: &ValueId, function_context: &mut FunctionContext, - brillig_context: &mut BrilligContext, + brillig_context: &mut BrilligContext, ) { assert!(self.available_variables.remove(value_id), "ICE: Variable is not available"); let variable = function_context @@ -122,7 +123,7 @@ impl BlockVariables { /// We keep constants block-local. pub(crate) fn allocate_constant( &mut self, - brillig_context: &mut BrilligContext, + brillig_context: &mut BrilligContext, value_id: ValueId, dfg: &DataFlowGraph, ) -> BrilligVariable { @@ -154,9 +155,9 @@ pub(crate) fn compute_array_length(item_typ: &CompositeType, elem_count: usize) } /// For a given value_id, allocates the necessary registers to hold it. -pub(crate) fn allocate_value( +pub(crate) fn allocate_value( value_id: ValueId, - brillig_context: &mut BrilligContext, + brillig_context: &mut BrilligContext, dfg: &DataFlowGraph, ) -> BrilligVariable { let typ = dfg.type_of_value(value_id); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs index 74319595795..691ad68351e 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs @@ -1,13 +1,12 @@ use acvm::{ acir::brillig::{BinaryFieldOp, BinaryIntOp, MemoryAddress, Opcode as BrilligOpcode}, acir::AcirField, - FieldElement, }; use crate::brillig::brillig_ir::artifact::GeneratedBrillig; /// Generates brillig bytecode which computes the inverse of its input if not null, and zero else. -pub(crate) fn directive_invert() -> GeneratedBrillig { +pub(crate) fn directive_invert() -> GeneratedBrillig { // We generate the following code: // fn invert(x : Field) -> Field { // 1/ x @@ -28,8 +27,8 @@ pub(crate) fn directive_invert() -> GeneratedBrillig { // Put value zero in register (2) BrilligOpcode::Const { destination: zero_const, - value: FieldElement::from(0_usize), - bit_size: FieldElement::max_num_bits(), + value: F::from(0_usize), + bit_size: F::max_num_bits(), }, BrilligOpcode::BinaryFieldOp { op: BinaryFieldOp::Equals, @@ -42,8 +41,8 @@ pub(crate) fn directive_invert() -> GeneratedBrillig { // Put value one in register (1) BrilligOpcode::Const { destination: one_const, - value: FieldElement::from(1_usize), - bit_size: FieldElement::max_num_bits(), + value: F::from(1_usize), + bit_size: F::max_num_bits(), }, // Divide 1 by the input, and set the result of the division into register (0) BrilligOpcode::BinaryFieldOp { @@ -68,13 +67,13 @@ pub(crate) fn directive_invert() -> GeneratedBrillig { /// (a/b, a-a/b*b) /// } /// ``` -pub(crate) fn directive_quotient(bit_size: u32) -> GeneratedBrillig { +pub(crate) fn directive_quotient(bit_size: u32) -> GeneratedBrillig { // `a` is (0) (i.e register index 0) // `b` is (1) // TODO: The only difference between these implementations is the integer version will truncate the input to the `bit_size` via cast. // Once we deduplicate brillig functions then we can modify this so that fields and integers share the same quotient function. - if bit_size >= FieldElement::max_num_bits() { + if bit_size >= F::max_num_bits() { // Field version GeneratedBrillig { byte_code: vec![ diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs index 000d1230ece..f0752c80c46 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs @@ -28,7 +28,7 @@ pub(crate) struct FunctionContext { } impl FunctionContext { - /// Creates a new function context. It will compute the liveness of every variable. + /// Creates a new function context. It will allocate parameters for all blocks and compute the liveness of every variable. pub(crate) fn new(function: &Function) -> Self { let id = function.id(); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs index 491086e8c0f..d17b15a13b5 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs @@ -372,7 +372,7 @@ mod tests { use crate::ssa::ir::map::Id; use crate::ssa::ssa_gen::Ssa; - fn create_test_environment() -> (Ssa, FunctionContext, BrilligContext) { + fn create_test_environment() -> (Ssa, FunctionContext, BrilligContext) { let mut builder = FunctionBuilder::new("main".to_string(), Id::test_new(0)); builder.set_runtime(RuntimeType::Brillig); @@ -385,7 +385,7 @@ mod tests { fn create_brillig_block<'a>( function_context: &'a mut FunctionContext, - brillig_context: &'a mut BrilligContext, + brillig_context: &'a mut BrilligContext, ) -> BrilligBlock<'a> { let variables = BlockVariables::default(); BrilligBlock { diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index 9341db2ead7..f4ba9d4fc37 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -25,11 +25,13 @@ mod instructions; pub(crate) use instructions::BrilligBinaryOp; -use self::{artifact::BrilligArtifact, registers::BrilligRegistersContext}; +use self::{ + artifact::BrilligArtifact, debug_show::DebugToString, registers::BrilligRegistersContext, +}; use crate::ssa::ir::dfg::CallStack; use acvm::{ acir::brillig::{MemoryAddress, Opcode as BrilligOpcode}, - FieldElement, + AcirField, }; use debug_show::DebugShow; @@ -76,8 +78,8 @@ impl ReservedRegisters { /// Brillig context object that is used while constructing the /// Brillig bytecode. -pub(crate) struct BrilligContext { - obj: BrilligArtifact, +pub(crate) struct BrilligContext { + obj: BrilligArtifact, /// Tracks register allocations registers: BrilligRegistersContext, /// Context label, must be unique with respect to the function @@ -93,9 +95,9 @@ pub(crate) struct BrilligContext { bigint_new_id: u32, } -impl BrilligContext { +impl BrilligContext { /// Initial context state - pub(crate) fn new(enable_debug_trace: bool) -> BrilligContext { + pub(crate) fn new(enable_debug_trace: bool) -> BrilligContext { BrilligContext { obj: BrilligArtifact::default(), registers: BrilligRegistersContext::new(), @@ -113,12 +115,12 @@ impl BrilligContext { result } /// Adds a brillig instruction to the brillig byte code - fn push_opcode(&mut self, opcode: BrilligOpcode) { + fn push_opcode(&mut self, opcode: BrilligOpcode) { self.obj.push_opcode(opcode); } /// Returns the artifact - pub(crate) fn artifact(self) -> BrilligArtifact { + pub(crate) fn artifact(self) -> BrilligArtifact { self.obj } @@ -200,17 +202,17 @@ pub(crate) mod tests { } } - pub(crate) fn create_context() -> BrilligContext { + pub(crate) fn create_context() -> BrilligContext { let mut context = BrilligContext::new(true); context.enter_context("test"); context } pub(crate) fn create_entry_point_bytecode( - context: BrilligContext, + context: BrilligContext, arguments: Vec, returns: Vec, - ) -> GeneratedBrillig { + ) -> GeneratedBrillig { let artifact = context.artifact(); let mut entry_point_artifact = BrilligContext::new_entry_point_artifact(arguments, returns, "test".to_string()); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index 99e922c1580..2d0bdb5955c 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -1,4 +1,4 @@ -use acvm::{acir::brillig::Opcode as BrilligOpcode, FieldElement}; +use acvm::acir::brillig::Opcode as BrilligOpcode; use std::collections::{BTreeMap, HashMap}; use crate::ssa::ir::dfg::CallStack; @@ -18,8 +18,8 @@ pub(crate) enum BrilligParameter { /// The result of compiling and linking brillig artifacts. /// This is ready to run bytecode with attached metadata. #[derive(Debug, Default)] -pub(crate) struct GeneratedBrillig { - pub(crate) byte_code: Vec>, +pub(crate) struct GeneratedBrillig { + pub(crate) byte_code: Vec>, pub(crate) locations: BTreeMap, pub(crate) assert_messages: BTreeMap, } @@ -27,8 +27,8 @@ pub(crate) struct GeneratedBrillig { #[derive(Default, Debug, Clone)] /// Artifacts resulting from the compilation of a function into brillig byte code. /// It includes the bytecode of the function and all the metadata that allows linking with other functions. -pub(crate) struct BrilligArtifact { - pub(crate) byte_code: Vec>, +pub(crate) struct BrilligArtifact { + pub(crate) byte_code: Vec>, /// A map of bytecode positions to assertion messages. /// Some error messages (compiler intrinsics) are not emitted via revert data, /// instead, they are handled externally so they don't add size to user programs. @@ -73,9 +73,9 @@ pub(crate) type JumpInstructionPosition = OpcodeLocation; /// to their position in the bytecode. pub(crate) type UnresolvedJumpLocation = Label; -impl BrilligArtifact { +impl BrilligArtifact { /// Resolves all jumps and generates the final bytecode - pub(crate) fn finish(mut self) -> GeneratedBrillig { + pub(crate) fn finish(mut self) -> GeneratedBrillig { self.resolve_jumps(); GeneratedBrillig { byte_code: self.byte_code, @@ -94,7 +94,7 @@ impl BrilligArtifact { /// This method will offset the positions in the Brillig artifact to /// account for the fact that it is being appended to the end of this /// Brillig artifact (self). - pub(crate) fn link_with(&mut self, obj: &BrilligArtifact) { + pub(crate) fn link_with(&mut self, obj: &BrilligArtifact) { // Add the unresolved jumps of the linked function to this artifact. self.add_unresolved_jumps_and_calls(obj); @@ -128,7 +128,7 @@ impl BrilligArtifact { } /// Adds unresolved jumps & function calls from another artifact offset by the current opcode count in the artifact. - fn add_unresolved_jumps_and_calls(&mut self, obj: &BrilligArtifact) { + fn add_unresolved_jumps_and_calls(&mut self, obj: &BrilligArtifact) { let offset = self.index_of_next_opcode(); for (jump_label, jump_location) in &obj.unresolved_jumps { self.unresolved_jumps.push((jump_label + offset, jump_location.clone())); @@ -154,7 +154,7 @@ impl BrilligArtifact { } /// Adds a brillig instruction to the brillig byte code - pub(crate) fn push_opcode(&mut self, opcode: BrilligOpcode) { + pub(crate) fn push_opcode(&mut self, opcode: BrilligOpcode) { if !self.call_stack.is_empty() { self.locations.insert(self.index_of_next_opcode(), self.call_stack.clone()); } @@ -164,7 +164,7 @@ impl BrilligArtifact { /// Adds a unresolved jump to be fixed at the end of bytecode processing. pub(crate) fn add_unresolved_jump( &mut self, - jmp_instruction: BrilligOpcode, + jmp_instruction: BrilligOpcode, destination: UnresolvedJumpLocation, ) { assert!( @@ -178,7 +178,7 @@ impl BrilligArtifact { /// Adds a unresolved external call that will be fixed once linking has been done. pub(crate) fn add_unresolved_external_call( &mut self, - call_instruction: BrilligOpcode, + call_instruction: BrilligOpcode, destination: UnresolvedJumpLocation, ) { // TODO: Add a check to ensure that the opcode is a call instruction @@ -188,7 +188,7 @@ impl BrilligArtifact { } /// Returns true if the opcode is a jump instruction - fn is_jmp_instruction(instruction: &BrilligOpcode) -> bool { + fn is_jmp_instruction(instruction: &BrilligOpcode) -> bool { matches!( instruction, BrilligOpcode::JumpIfNot { .. } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_binary.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_binary.rs index 4ef279bd532..a9c4f238491 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_binary.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_binary.rs @@ -1,8 +1,8 @@ -use acvm::{acir::brillig::MemoryAddress, FieldElement}; +use acvm::{acir::brillig::MemoryAddress, AcirField}; -use super::{instructions::BrilligBinaryOp, BrilligContext}; +use super::{debug_show::DebugToString, instructions::BrilligBinaryOp, BrilligContext}; -impl BrilligContext { +impl BrilligContext { /// Utility method to perform a binary instruction with a constant value in place pub(crate) fn codegen_usize_op_in_place( &mut self, @@ -21,7 +21,7 @@ impl BrilligContext { op: BrilligBinaryOp, constant: usize, ) { - let const_register = self.make_usize_constant_instruction(FieldElement::from(constant)); + let const_register = self.make_usize_constant_instruction(F::from(constant)); self.memory_op_instruction(operand, const_register.address, destination, op); // Mark as no longer used for this purpose, frees for reuse self.deallocate_single_addr(const_register); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_calls.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_calls.rs index db65849a6b8..2d93cf70181 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_calls.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_calls.rs @@ -1,10 +1,11 @@ -use acvm::acir::brillig::MemoryAddress; +use acvm::{acir::brillig::MemoryAddress, AcirField}; use super::{ - brillig_variable::BrilligVariable, BrilligBinaryOp, BrilligContext, ReservedRegisters, + brillig_variable::BrilligVariable, debug_show::DebugToString, BrilligBinaryOp, BrilligContext, + ReservedRegisters, }; -impl BrilligContext { +impl BrilligContext { /// Saves all of the registers that have been used up until this point. fn codegen_save_registers_of_vars(&mut self, vars: &[BrilligVariable]) -> Vec { // Save all of the used registers at this point in memory diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs index d9109646338..1a99154723b 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs @@ -1,12 +1,16 @@ -use acvm::acir::brillig::{HeapArray, MemoryAddress}; +use acvm::{ + acir::brillig::{HeapArray, MemoryAddress}, + AcirField, +}; use super::{ artifact::BrilligParameter, brillig_variable::{BrilligVariable, SingleAddrVariable}, + debug_show::DebugToString, BrilligBinaryOp, BrilligContext, ReservedRegisters, }; -impl BrilligContext { +impl BrilligContext { /// Codegens a return from the current function. /// /// For Brillig, the return is implicit, since there is no explicit return instruction. @@ -36,10 +40,11 @@ impl BrilligContext { /// This codegen will issue a loop that will iterate iteration_count times /// The body of the loop should be issued by the caller in the on_iteration closure. - pub(crate) fn codegen_loop(&mut self, iteration_count: MemoryAddress, on_iteration: F) - where - F: FnOnce(&mut BrilligContext, SingleAddrVariable), - { + pub(crate) fn codegen_loop( + &mut self, + iteration_count: MemoryAddress, + on_iteration: impl FnOnce(&mut BrilligContext, SingleAddrVariable), + ) { let iterator_register = self.make_usize_constant_instruction(0_u128.into()); let (loop_section, loop_label) = self.reserve_next_section_label(); @@ -87,7 +92,7 @@ impl BrilligContext { pub(crate) fn codegen_branch( &mut self, condition: MemoryAddress, - mut f: impl FnMut(&mut BrilligContext, bool), + mut f: impl FnMut(&mut BrilligContext, bool), ) { // Reserve 3 sections let (then_section, then_label) = self.reserve_next_section_label(); @@ -112,7 +117,7 @@ impl BrilligContext { pub(crate) fn codegen_if( &mut self, condition: MemoryAddress, - f: impl FnOnce(&mut BrilligContext), + f: impl FnOnce(&mut BrilligContext), ) { let (end_section, end_label) = self.reserve_next_section_label(); let (then_section, then_label) = self.reserve_next_section_label(); @@ -130,7 +135,7 @@ impl BrilligContext { pub(crate) fn codegen_if_not( &mut self, condition: MemoryAddress, - f: impl FnOnce(&mut BrilligContext), + f: impl FnOnce(&mut BrilligContext), ) { let (end_section, end_label) = self.reserve_next_section_label(); @@ -156,7 +161,7 @@ impl BrilligContext { let revert_data = HeapArray { pointer: ctx.allocate_register(), // + 1 due to the revert data id being the first item returned - size: BrilligContext::flattened_tuple_size(&revert_data_types) + 1, + size: Self::flattened_tuple_size(&revert_data_types) + 1, }; ctx.codegen_allocate_fixed_length_array(revert_data.pointer, revert_data.size); @@ -170,7 +175,7 @@ impl BrilligContext { for (revert_variable, revert_param) in revert_data_items.into_iter().zip(revert_data_types.into_iter()) { - let flattened_size = BrilligContext::flattened_size(&revert_param); + let flattened_size = Self::flattened_size(&revert_param); match revert_param { BrilligParameter::SingleAddr(_) => { ctx.store_instruction( diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_intrinsic.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_intrinsic.rs index 42f3b34aea0..b1cb2b19764 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_intrinsic.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_intrinsic.rs @@ -1,15 +1,15 @@ use acvm::{ acir::brillig::{BlackBoxOp, HeapArray}, acir::AcirField, - FieldElement, }; use super::{ brillig_variable::{BrilligVector, SingleAddrVariable}, + debug_show::DebugToString, BrilligContext, }; -impl BrilligContext { +impl BrilligContext { /// Codegens a truncation of a value to the given bit size pub(crate) fn codegen_truncate( &mut self, @@ -43,7 +43,7 @@ impl BrilligContext { big_endian: bool, limb_bit_size: u32, ) { - assert!(source_field.bit_size == FieldElement::max_num_bits()); + assert!(source_field.bit_size == F::max_num_bits()); self.usize_const_instruction(target_vector.size, limb_count.into()); self.usize_const_instruction(target_vector.rc, 1_usize.into()); @@ -55,12 +55,10 @@ impl BrilligContext { output: HeapArray { pointer: target_vector.pointer, size: limb_count }, }); - let limb_field = - SingleAddrVariable::new(self.allocate_register(), FieldElement::max_num_bits()); - + let limb_field = SingleAddrVariable::new(self.allocate_register(), F::max_num_bits()); let limb_casted = SingleAddrVariable::new(self.allocate_register(), limb_bit_size); - if limb_bit_size != FieldElement::max_num_bits() { + if limb_bit_size != F::max_num_bits() { self.codegen_loop(target_vector.size, |ctx, iterator_register| { // Read the limb ctx.codegen_array_get(target_vector.pointer, iterator_register, limb_field.address); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs index 15761113f51..81c1c3847b1 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs @@ -1,13 +1,14 @@ -use acvm::acir::brillig::MemoryAddress; +use acvm::{acir::brillig::MemoryAddress, AcirField}; use crate::brillig::brillig_ir::BrilligBinaryOp; use super::{ brillig_variable::{BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable}, + debug_show::DebugToString, BrilligContext, ReservedRegisters, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, }; -impl BrilligContext { +impl BrilligContext { /// Allocates an array of size `size` and stores the pointer to the array /// in `pointer_register` pub(crate) fn codegen_allocate_fixed_length_array( diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_stack.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_stack.rs index 1c30f0f848f..943b0b9d7a3 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_stack.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_stack.rs @@ -1,8 +1,8 @@ -use acvm::acir::brillig::MemoryAddress; +use acvm::{acir::brillig::MemoryAddress, AcirField}; -use super::BrilligContext; +use super::{debug_show::DebugToString, BrilligContext}; -impl BrilligContext { +impl BrilligContext { /// This function moves values from a set of registers to another set of registers. /// It first moves all sources to new allocated registers to avoid overwriting. pub(crate) fn codegen_mov_registers_to_registers( diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs index def91f82bfd..075a776572e 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs @@ -8,7 +8,7 @@ use acvm::{ }; /// Trait for converting values into debug-friendly strings. -trait DebugToString { +pub(crate) trait DebugToString { fn debug_to_string(&self) -> String; } @@ -169,7 +169,7 @@ impl DebugShow { } /// Stores the value of `constant` in the `result` register - pub(crate) fn const_instruction(&self, result: MemoryAddress, constant: FieldElement) { + pub(crate) fn const_instruction(&self, result: MemoryAddress, constant: F) { debug_println!(self.enable_debug_trace, " CONST {} = {}", result, constant); } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs index 9023183eb36..dc06c2fa0d7 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs @@ -1,21 +1,21 @@ use super::{ artifact::{BrilligArtifact, BrilligParameter}, brillig_variable::{BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable}, - debug_show::DebugShow, + debug_show::{DebugShow, DebugToString}, registers::BrilligRegistersContext, BrilligBinaryOp, BrilligContext, ReservedRegisters, }; -use acvm::{acir::brillig::MemoryAddress, acir::AcirField, FieldElement}; +use acvm::{acir::brillig::MemoryAddress, acir::AcirField}; pub(crate) const MAX_STACK_SIZE: usize = 2048; -impl BrilligContext { +impl BrilligContext { /// Creates an entry point artifact that will jump to the function label provided. pub(crate) fn new_entry_point_artifact( arguments: Vec, return_parameters: Vec, target_function: T, - ) -> BrilligArtifact { + ) -> BrilligArtifact { let mut context = BrilligContext { obj: BrilligArtifact::default(), registers: BrilligRegistersContext::new(), @@ -42,8 +42,8 @@ impl BrilligContext { arguments: &[BrilligParameter], return_parameters: &[BrilligParameter], ) { - let calldata_size = BrilligContext::flattened_tuple_size(arguments); - let return_data_size = BrilligContext::flattened_tuple_size(return_parameters); + let calldata_size = Self::flattened_tuple_size(arguments); + let return_data_size = Self::flattened_tuple_size(return_parameters); // Set initial value of stack pointer: MAX_STACK_SIZE + calldata_size + return_data_size self.const_instruction( @@ -74,7 +74,7 @@ impl BrilligContext { let pointer_to_the_array_in_calldata = self.make_usize_constant_instruction(current_calldata_pointer.into()); let rc_register = self.make_usize_constant_instruction(1_usize.into()); - let flattened_size = BrilligContext::flattened_size(argument); + let flattened_size = Self::flattened_size(argument); let var = BrilligVariable::BrilligArray(BrilligArray { pointer: pointer_to_the_array_in_calldata.address, size: flattened_size, @@ -88,7 +88,7 @@ impl BrilligContext { let pointer_to_the_array_in_calldata = self.make_usize_constant_instruction(current_calldata_pointer.into()); - let flattened_size = BrilligContext::flattened_size(argument); + let flattened_size = Self::flattened_size(argument); let size_register = self.make_usize_constant_instruction(flattened_size.into()); let rc_register = self.make_usize_constant_instruction(1_usize.into()); @@ -121,7 +121,7 @@ impl BrilligContext { BrilligVariable::BrilligVector(vector), BrilligParameter::Slice(item_type, item_count), ) => { - let flattened_size = BrilligContext::flattened_size(argument); + let flattened_size = Self::flattened_size(argument); let deflattened_address = self.deflatten_array(item_type, flattened_size, vector.pointer); @@ -139,7 +139,7 @@ impl BrilligContext { } fn copy_and_cast_calldata(&mut self, arguments: &[BrilligParameter]) { - let calldata_size = BrilligContext::flattened_tuple_size(arguments); + let calldata_size = Self::flattened_tuple_size(arguments); self.calldata_copy_instruction(MemoryAddress(MAX_STACK_SIZE), calldata_size, 0); fn flat_bit_sizes(param: &BrilligParameter) -> Box + '_> { @@ -154,7 +154,7 @@ impl BrilligContext { for (i, bit_size) in arguments.iter().flat_map(flat_bit_sizes).enumerate() { // Calldatacopy tags everything with field type, so when downcast when necessary - if bit_size < FieldElement::max_num_bits() { + if bit_size < F::max_num_bits() { self.cast_instruction( SingleAddrVariable::new(MemoryAddress(MAX_STACK_SIZE + i), bit_size), SingleAddrVariable::new_field(MemoryAddress(MAX_STACK_SIZE + i)), @@ -169,7 +169,7 @@ impl BrilligContext { BrilligParameter::SingleAddr(_) => 1, BrilligParameter::Array(item_types, item_count) | BrilligParameter::Slice(item_types, item_count) => { - let item_size: usize = item_types.iter().map(BrilligContext::flattened_size).sum(); + let item_size: usize = item_types.iter().map(Self::flattened_size).sum(); item_count * item_size } } @@ -177,7 +177,7 @@ impl BrilligContext { /// Computes the size of a parameter if it was flattened pub(super) fn flattened_tuple_size(tuple: &[BrilligParameter]) -> usize { - tuple.iter().map(BrilligContext::flattened_size).sum() + tuple.iter().map(Self::flattened_size).sum() } /// Computes the size of a parameter if it was flattened @@ -193,12 +193,12 @@ impl BrilligContext { item_count: usize, flattened_array_pointer: MemoryAddress, ) -> MemoryAddress { - if BrilligContext::has_nested_arrays(item_type) { + if Self::has_nested_arrays(item_type) { let movement_register = self.allocate_register(); let deflattened_array_pointer = self.allocate_register(); let target_item_size = item_type.len(); - let source_item_size = BrilligContext::flattened_tuple_size(item_type); + let source_item_size = Self::flattened_tuple_size(item_type); self.codegen_allocate_fixed_length_array( deflattened_array_pointer, @@ -276,7 +276,7 @@ impl BrilligContext { .into_iter() .for_each(|register| self.deallocate_register(register)); - source_offset += BrilligContext::flattened_size(subitem); + source_offset += Self::flattened_size(subitem); } BrilligParameter::Slice(..) => unreachable!("ICE: Cannot deflatten slices"), } @@ -328,8 +328,8 @@ impl BrilligContext { .collect(); // Now, we deflatten the return data - let calldata_size = BrilligContext::flattened_tuple_size(arguments); - let return_data_size = BrilligContext::flattened_tuple_size(return_parameters); + let calldata_size = Self::flattened_tuple_size(arguments); + let return_data_size = Self::flattened_tuple_size(return_parameters); // Return data has a reserved space after calldata let return_data_offset = MAX_STACK_SIZE + calldata_size; @@ -357,7 +357,7 @@ impl BrilligContext { ); self.deallocate_single_addr(pointer_to_return_data); - return_data_index += BrilligContext::flattened_size(return_param); + return_data_index += Self::flattened_size(return_param); } BrilligParameter::Slice(..) => { unreachable!("ICE: Cannot return slices from brillig entrypoints") @@ -376,12 +376,11 @@ impl BrilligContext { flattened_array_pointer: MemoryAddress, deflattened_array_pointer: MemoryAddress, ) { - if BrilligContext::has_nested_arrays(item_type) { + if Self::has_nested_arrays(item_type) { let movement_register = self.allocate_register(); let source_item_size = item_type.len(); - let target_item_size: usize = - item_type.iter().map(BrilligContext::flattened_size).sum(); + let target_item_size: usize = item_type.iter().map(Self::flattened_size).sum(); for item_index in 0..item_count { let source_item_base_index = item_index * source_item_size; @@ -462,7 +461,7 @@ impl BrilligContext { .into_iter() .for_each(|register| self.deallocate_register(register)); - target_offset += BrilligContext::flattened_size(subitem); + target_offset += Self::flattened_size(subitem); } BrilligParameter::Slice(..) => unreachable!("ICE: Cannot flatten slices"), } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs index 03a9216b73a..a614f93fa30 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs @@ -10,12 +10,13 @@ use acvm::{ use super::{ artifact::UnresolvedJumpLocation, brillig_variable::{BrilligArray, BrilligVector, SingleAddrVariable}, + debug_show::DebugToString, BrilligContext, ReservedRegisters, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, }; /// Low level instructions of the brillig IR, used by the brillig ir codegens and brillig_gen /// Printed using debug_slow -impl BrilligContext { +impl BrilligContext { /// Processes a binary instruction according `operation`. /// /// This method will compute lhs rhs @@ -42,8 +43,7 @@ impl BrilligContext { ) { self.debug_show.not_instruction(input.address, input.bit_size, result.address); // Compile !x as ((-1) - x) - let u_max = FieldElement::from(2_i128).pow(&FieldElement::from(input.bit_size as i128)) - - FieldElement::one(); + let u_max = F::from(2_u128).pow(&F::from(input.bit_size as u128)) - F::one(); let max = self.make_constant(u_max, input.bit_size); self.binary(max, input, result, BrilligBinaryOp::Sub); @@ -63,7 +63,7 @@ impl BrilligContext { SingleAddrVariable::new_usize(rhs), SingleAddrVariable::new( destination, - BrilligContext::binary_result_bit_size(op, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE), + Self::binary_result_bit_size(op, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE), ), op, ); @@ -77,8 +77,7 @@ impl BrilligContext { operation: BrilligBinaryOp, ) { let is_field_op = lhs.bit_size == FieldElement::max_num_bits(); - let expected_result_bit_size = - BrilligContext::binary_result_bit_size(operation, lhs.bit_size); + let expected_result_bit_size = Self::binary_result_bit_size(operation, lhs.bit_size); assert!( result.bit_size == expected_result_bit_size, "Expected result bit size to be {}, got {} for operation {:?}", @@ -213,7 +212,7 @@ impl BrilligContext { /// Adds a unresolved `Jump` to the bytecode. fn add_unresolved_jump( &mut self, - jmp_instruction: BrilligOpcode, + jmp_instruction: BrilligOpcode, destination: UnresolvedJumpLocation, ) { self.obj.add_unresolved_jump(jmp_instruction, destination); @@ -369,12 +368,12 @@ impl BrilligContext { } /// Stores the value of `constant` in the `result` register - pub(crate) fn const_instruction(&mut self, result: SingleAddrVariable, constant: FieldElement) { + pub(crate) fn const_instruction(&mut self, result: SingleAddrVariable, constant: F) { self.debug_show.const_instruction(result.address, constant); self.constant(result, constant); } - fn constant(&mut self, result: SingleAddrVariable, constant: FieldElement) { + fn constant(&mut self, result: SingleAddrVariable, constant: F) { assert!( result.bit_size >= constant.num_bits(), "Constant {} does not fit in bit size {}", @@ -382,10 +381,10 @@ impl BrilligContext { result.bit_size ); if result.bit_size > 128 && constant.num_bits() > 128 { - let high = FieldElement::from_be_bytes_reduce( + let high = F::from_be_bytes_reduce( constant.to_be_bytes().get(0..16).expect("FieldElement::to_be_bytes() too short!"), ); - let low = FieldElement::from(constant.to_u128()); + let low = F::from(constant.to_u128()); let high_register = SingleAddrVariable::new(self.allocate_register(), 254); let low_register = SingleAddrVariable::new(self.allocate_register(), 254); let intermediate_register = SingleAddrVariable::new(self.allocate_register(), 254); @@ -393,7 +392,7 @@ impl BrilligContext { self.constant(low_register, low); // I want to multiply high by 2^128, but I can't get that big constant in. // So I'll multiply by 2^64 twice. - self.constant(intermediate_register, FieldElement::from(1_u128 << 64)); + self.constant(intermediate_register, F::from(1_u128 << 64)); self.binary(high_register, intermediate_register, high_register, BrilligBinaryOp::Mul); self.binary(high_register, intermediate_register, high_register, BrilligBinaryOp::Mul); // Now we can add. @@ -411,18 +410,14 @@ impl BrilligContext { } } - pub(crate) fn usize_const_instruction( - &mut self, - result: MemoryAddress, - constant: FieldElement, - ) { + pub(crate) fn usize_const_instruction(&mut self, result: MemoryAddress, constant: F) { self.const_instruction(SingleAddrVariable::new_usize(result), constant); } /// Returns a register which holds the value of a constant pub(crate) fn make_constant_instruction( &mut self, - constant: FieldElement, + constant: F, bit_size: u32, ) -> SingleAddrVariable { let var = SingleAddrVariable::new(self.allocate_register(), bit_size); @@ -430,17 +425,14 @@ impl BrilligContext { var } - fn make_constant(&mut self, constant: FieldElement, bit_size: u32) -> SingleAddrVariable { + fn make_constant(&mut self, constant: F, bit_size: u32) -> SingleAddrVariable { let var = SingleAddrVariable::new(self.allocate_register(), bit_size); self.constant(var, constant); var } /// Returns a register which holds the value of an usize constant - pub(crate) fn make_usize_constant_instruction( - &mut self, - constant: FieldElement, - ) -> SingleAddrVariable { + pub(crate) fn make_usize_constant_instruction(&mut self, constant: F) -> SingleAddrVariable { let register = self.allocate_register(); self.usize_const_instruction(register, constant); SingleAddrVariable::new_usize(register) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs index f756f06aa69..ae506462b25 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs @@ -82,7 +82,7 @@ impl BrilligRegistersContext { } } -impl BrilligContext { +impl BrilligContext { /// Returns the i'th register after the reserved ones pub(crate) fn register(&self, i: usize) -> MemoryAddress { MemoryAddress::from(ReservedRegisters::NUM_RESERVED_REGISTERS + i) diff --git a/compiler/noirc_evaluator/src/brillig/mod.rs b/compiler/noirc_evaluator/src/brillig/mod.rs index eda1ac97c1e..70c0959517b 100644 --- a/compiler/noirc_evaluator/src/brillig/mod.rs +++ b/compiler/noirc_evaluator/src/brillig/mod.rs @@ -1,6 +1,8 @@ pub(crate) mod brillig_gen; pub(crate) mod brillig_ir; +use acvm::FieldElement; + use self::{ brillig_gen::{brillig_fn::FunctionContext, convert_ssa_function}, brillig_ir::artifact::{BrilligArtifact, Label}, @@ -16,7 +18,7 @@ use std::collections::{BTreeSet, HashMap}; #[derive(Default)] pub struct Brillig { /// Maps SSA function labels to their brillig artifact - ssa_function_to_brillig: HashMap, + ssa_function_to_brillig: HashMap>, } impl Brillig { @@ -27,7 +29,10 @@ impl Brillig { } /// Finds a brillig function artifact by its function label - pub(crate) fn find_by_function_label(&self, function_label: Label) -> Option<&BrilligArtifact> { + pub(crate) fn find_by_function_label( + &self, + function_label: Label, + ) -> Option<&BrilligArtifact> { self.ssa_function_to_brillig.iter().find_map(|(function_id, obj)| { if FunctionContext::function_id_to_function_label(*function_id) == function_label { Some(obj) @@ -39,7 +44,7 @@ impl Brillig { } impl std::ops::Index for Brillig { - type Output = BrilligArtifact; + type Output = BrilligArtifact; fn index(&self, id: FunctionId) -> &Self::Output { &self.ssa_function_to_brillig[&id] } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 0c826cc8a38..d902bf7f688 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -1550,7 +1550,7 @@ impl AcirContext { pub(crate) fn brillig_call( &mut self, predicate: AcirVar, - generated_brillig: &GeneratedBrillig, + generated_brillig: &GeneratedBrillig, inputs: Vec, outputs: Vec, attempt_execution: bool, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 6c79c0a228d..33367809980 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -83,7 +83,7 @@ pub(crate) enum BrilligStdlibFunc { } impl BrilligStdlibFunc { - pub(crate) fn get_generated_brillig(&self) -> GeneratedBrillig { + pub(crate) fn get_generated_brillig(&self) -> GeneratedBrillig { match self { BrilligStdlibFunc::Inverse => brillig_directive::directive_invert(), BrilligStdlibFunc::Quotient(bit_size) => { @@ -597,7 +597,7 @@ impl GeneratedAcir { pub(crate) fn brillig_call( &mut self, predicate: Option>, - generated_brillig: &GeneratedBrillig, + generated_brillig: &GeneratedBrillig, inputs: Vec>, outputs: Vec, brillig_function_index: u32, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 13677506d0b..bb1b9e3ccfd 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -50,7 +50,7 @@ struct SharedContext { /// Final list of Brillig functions which will be part of the final program /// This is shared across `Context` structs as we want one list of Brillig /// functions across all ACIR artifacts - generated_brillig: Vec, + generated_brillig: Vec>, /// Maps SSA function index -> Final generated Brillig artifact index. /// There can be Brillig functions specified in SSA which do not act as @@ -78,7 +78,7 @@ impl SharedContext { self.brillig_generated_func_pointers.get(&(func_id, arguments)) } - fn generated_brillig(&self, func_pointer: usize) -> &GeneratedBrillig { + fn generated_brillig(&self, func_pointer: usize) -> &GeneratedBrillig { &self.generated_brillig[func_pointer] } @@ -87,7 +87,7 @@ impl SharedContext { func_id: FunctionId, arguments: Vec, generated_pointer: u32, - code: GeneratedBrillig, + code: GeneratedBrillig, ) { self.brillig_generated_func_pointers.insert((func_id, arguments), generated_pointer); self.generated_brillig.push(code); @@ -127,7 +127,7 @@ impl SharedContext { generated_pointer: u32, func_id: FunctionId, opcode_location: OpcodeLocation, - code: GeneratedBrillig, + code: GeneratedBrillig, ) { self.brillig_stdlib_func_pointer.insert(brillig_stdlib_func, generated_pointer); self.add_call_to_resolve(func_id, (opcode_location, generated_pointer)); @@ -922,7 +922,7 @@ impl<'a> Context<'a> { func: &Function, arguments: Vec, brillig: &Brillig, - ) -> Result { + ) -> Result, InternalError> { // Create the entry point artifact let mut entry_point = BrilligContext::new_entry_point_artifact( arguments, From 5c349626c16d4fc2d606f88505adb63819634a40 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 27 May 2024 16:14:08 +0000 Subject: [PATCH 4/8] chore: generic `GeneratedAcir` --- compiler/noirc_evaluator/src/ssa.rs | 2 +- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 18 +++--- .../ssa/acir_gen/acir_ir/generated_acir.rs | 60 +++++++++---------- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 12 ++-- 4 files changed, 46 insertions(+), 46 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index d38601bfc1b..7da63bd8f11 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -205,7 +205,7 @@ pub struct SsaCircuitArtifact { } fn convert_generated_acir_into_circuit( - mut generated_acir: GeneratedAcir, + mut generated_acir: GeneratedAcir, func_sig: FunctionSignature, recursive: bool, debug_variables: DebugVariables, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index d902bf7f688..a55b6983c53 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -48,12 +48,12 @@ impl AcirType { } /// Returns the bit size of the underlying type - pub(crate) fn bit_size(&self) -> u32 { + pub(crate) fn bit_size(&self) -> u32 { match self { AcirType::NumericType(numeric_type) => match numeric_type { NumericType::Signed { bit_size } => *bit_size, NumericType::Unsigned { bit_size } => *bit_size, - NumericType::NativeField => FieldElement::max_num_bits(), + NumericType::NativeField => F::max_num_bits(), }, AcirType::Array(_, _) => unreachable!("cannot fetch bit size of array type"), } @@ -121,7 +121,7 @@ pub(crate) struct AcirContext { /// For example, If one was to add two Variables together, /// then the `acir_ir` will be populated to assert this /// addition. - acir_ir: GeneratedAcir, + acir_ir: GeneratedAcir, /// The BigIntContext, used to generate identifiers for BigIntegers big_int_ctx: BigIntContext, @@ -418,7 +418,7 @@ impl AcirContext { return Ok(lhs); } - let bit_size = typ.bit_size(); + let bit_size = typ.bit_size::(); if bit_size == 1 { // Operands are booleans. // @@ -452,7 +452,7 @@ impl AcirContext { return Ok(zero); } - let bit_size = typ.bit_size(); + let bit_size = typ.bit_size::(); if bit_size == 1 { // Operands are booleans. self.mul_var(lhs, rhs) @@ -480,7 +480,7 @@ impl AcirContext { return Ok(lhs); } - let bit_size = typ.bit_size(); + let bit_size = typ.bit_size::(); if bit_size == 1 { // Operands are booleans // a + b - ab @@ -695,7 +695,7 @@ impl AcirContext { /// Adds a new variable that is constrained to be the logical NOT of `x`. pub(crate) fn not_var(&mut self, x: AcirVar, typ: AcirType) -> Result { - let bit_size = typ.bit_size(); + let bit_size = typ.bit_size::(); // Subtracting from max flips the bits let max = self.add_constant((1_u128 << bit_size) - 1); self.sub_var(max, x) @@ -1419,7 +1419,7 @@ impl AcirContext { // constants too. let witness_var = self.get_or_create_witness_var(input)?; let witness = self.var_to_witness(witness_var)?; - let num_bits = typ.bit_size(); + let num_bits = typ.bit_size::(); single_val_witnesses.push(FunctionInput { witness, num_bits }); } witnesses.push(single_val_witnesses); @@ -1529,7 +1529,7 @@ impl AcirContext { mut self, inputs: Vec, warnings: Vec, - ) -> GeneratedAcir { + ) -> GeneratedAcir { self.acir_ir.input_witnesses = inputs; self.acir_ir.warnings = warnings; self.acir_ir diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 33367809980..6f98e41ee6b 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -32,7 +32,7 @@ pub(crate) const PLACEHOLDER_BRILLIG_INDEX: u32 = 0; #[derive(Debug, Default)] /// The output of the Acir-gen pass, which should only be produced for entry point Acir functions -pub(crate) struct GeneratedAcir { +pub(crate) struct GeneratedAcir { /// The next witness index that may be declared. /// If witness index is `None` then we have not yet created a witness /// and thus next witness index that be declared is zero. @@ -42,7 +42,7 @@ pub(crate) struct GeneratedAcir { current_witness_index: Option, /// The opcodes of which the compiled ACIR will comprise. - opcodes: Vec>, + opcodes: Vec>, /// All witness indices that comprise the final return value of the program /// @@ -61,7 +61,7 @@ pub(crate) struct GeneratedAcir { pub(crate) call_stack: CallStack, /// Correspondence between an opcode index and the error message associated with it. - pub(crate) assertion_payloads: BTreeMap>, + pub(crate) assertion_payloads: BTreeMap>, pub(crate) warnings: Vec, @@ -93,25 +93,25 @@ impl BrilligStdlibFunc { } } -impl GeneratedAcir { +impl GeneratedAcir { /// Returns the current witness index. pub(crate) fn current_witness_index(&self) -> Witness { Witness(self.current_witness_index.unwrap_or(0)) } /// Adds a new opcode into ACIR. - pub(crate) fn push_opcode(&mut self, opcode: AcirOpcode) { + pub(crate) fn push_opcode(&mut self, opcode: AcirOpcode) { self.opcodes.push(opcode); if !self.call_stack.is_empty() { self.locations.insert(self.last_acir_opcode_location(), self.call_stack.clone()); } } - pub(crate) fn opcodes(&self) -> &[AcirOpcode] { + pub(crate) fn opcodes(&self) -> &[AcirOpcode] { &self.opcodes } - pub(crate) fn take_opcodes(&mut self) -> Vec> { + pub(crate) fn take_opcodes(&mut self) -> Vec> { std::mem::take(&mut self.opcodes) } @@ -130,7 +130,7 @@ impl GeneratedAcir { /// /// If `expr` can be represented as a `Witness` then this function will return it, /// else a new opcode will be added to create a `Witness` that is equal to `expr`. - pub(crate) fn get_or_create_witness(&mut self, expr: &Expression) -> Witness { + pub(crate) fn get_or_create_witness(&mut self, expr: &Expression) -> Witness { match expr.to_witness() { Some(witness) => witness, None => self.create_witness_for_expression(expr), @@ -145,7 +145,7 @@ impl GeneratedAcir { /// which has degree-1 in order to be able to continue the multiplication chain. pub(crate) fn create_witness_for_expression( &mut self, - expression: &Expression, + expression: &Expression, ) -> Witness { let fresh_witness = self.next_witness_index(); @@ -171,15 +171,15 @@ impl GeneratedAcir { } } -impl GeneratedAcir { +impl GeneratedAcir { /// Calls a black box function and returns the output /// of said blackbox function. pub(crate) fn call_black_box( &mut self, func_name: BlackBoxFunc, inputs: &[Vec], - constant_inputs: Vec, - constant_outputs: Vec, + constant_inputs: Vec, + constant_outputs: Vec, output_count: usize, ) -> Result, InternalError> { let input_count = inputs.iter().fold(0usize, |sum, val| sum + val.len()); @@ -396,7 +396,7 @@ impl GeneratedAcir { /// Only radix that are a power of two are supported pub(crate) fn radix_le_decompose( &mut self, - input_expr: &Expression, + input_expr: &Expression, radix: u32, limb_count: u32, bit_size: u32, @@ -422,7 +422,7 @@ impl GeneratedAcir { self.range_constraint(*limb_witness, bit_size)?; composed_limbs = composed_limbs.add_mul( - FieldElement::from_be_bytes_reduce(&radix_pow.to_bytes_be()), + F::from_be_bytes_reduce(&radix_pow.to_bytes_be()), &Expression::from(*limb_witness), ); @@ -442,7 +442,7 @@ impl GeneratedAcir { /// /// Safety: It is the callers responsibility to ensure that the /// resulting `Witness` is constrained to be the inverse. - pub(crate) fn brillig_inverse(&mut self, expr: Expression) -> Witness { + pub(crate) fn brillig_inverse(&mut self, expr: Expression) -> Witness { // Create the witness for the result let inverted_witness = self.next_witness_index(); @@ -466,7 +466,7 @@ impl GeneratedAcir { /// /// If `expr` is not zero, then the constraint system will /// fail upon verification. - pub(crate) fn assert_is_zero(&mut self, expr: Expression) { + pub(crate) fn assert_is_zero(&mut self, expr: Expression) { self.push_opcode(AcirOpcode::AssertZero(expr)); } @@ -475,8 +475,8 @@ impl GeneratedAcir { /// - `0` otherwise pub(crate) fn is_equal( &mut self, - lhs: &Expression, - rhs: &Expression, + lhs: &Expression, + rhs: &Expression, ) -> Witness { let t = lhs - rhs; @@ -535,13 +535,13 @@ impl GeneratedAcir { /// By setting `z` to be `0`, we can make `y` equal to `1`. /// This is easily observed: `y = 1 - t * 0` /// Now since `y` is one, this means that `t` needs to be zero, or else `y * t == 0` will fail. - fn is_zero(&mut self, t_expr: &Expression) -> Witness { + fn is_zero(&mut self, t_expr: &Expression) -> Witness { // We're checking for equality with zero so we can negate the expression without changing the result. // This is useful as it will sometimes allow us to simplify an expression down to a witness. let t_witness = if let Some(witness) = t_expr.to_witness() { witness } else { - let negated_expr = t_expr * -FieldElement::one(); + let negated_expr = t_expr * -F::one(); self.get_or_create_witness(&negated_expr) }; @@ -553,17 +553,17 @@ impl GeneratedAcir { // Add constraint y == 1 - tz => y + tz - 1 == 0 let y_is_boolean_constraint = Expression { - mul_terms: vec![(FieldElement::one(), t_witness, z)], - linear_combinations: vec![(FieldElement::one(), y)], - q_c: -FieldElement::one(), + mul_terms: vec![(F::one(), t_witness, z)], + linear_combinations: vec![(F::one(), y)], + q_c: -F::one(), }; self.assert_is_zero(y_is_boolean_constraint); // Add constraint that y * t == 0; let ty_zero_constraint = Expression { - mul_terms: vec![(FieldElement::one(), t_witness, y)], + mul_terms: vec![(F::one(), t_witness, y)], linear_combinations: vec![], - q_c: FieldElement::zero(), + q_c: F::zero(), }; self.assert_is_zero(ty_zero_constraint); @@ -579,9 +579,9 @@ impl GeneratedAcir { ) -> Result<(), RuntimeError> { // We class this as an error because users should instead // do `as Field`. - if num_bits >= FieldElement::max_num_bits() { + if num_bits >= F::max_num_bits() { return Err(RuntimeError::InvalidRangeConstraint { - num_bits: FieldElement::max_num_bits(), + num_bits: F::max_num_bits(), call_stack: self.call_stack.clone(), }); }; @@ -596,9 +596,9 @@ impl GeneratedAcir { pub(crate) fn brillig_call( &mut self, - predicate: Option>, - generated_brillig: &GeneratedBrillig, - inputs: Vec>, + predicate: Option>, + generated_brillig: &GeneratedBrillig, + inputs: Vec>, outputs: Vec, brillig_function_index: u32, stdlib_func: Option, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index bb1b9e3ccfd..54c64994e27 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -279,7 +279,7 @@ impl AcirValue { } pub(crate) type Artifacts = - (Vec, Vec>, BTreeMap); + (Vec>, Vec>, BTreeMap); impl Ssa { #[tracing::instrument(level = "trace", skip_all)] @@ -341,7 +341,7 @@ impl Ssa { } } -fn generate_distinct_return_witnesses(acir: &mut GeneratedAcir) { +fn generate_distinct_return_witnesses(acir: &mut GeneratedAcir) { // Create a witness for each return witness we have to guarantee that the return witnesses match the standard // layout for serializing those types as if they were being passed as inputs. // @@ -386,7 +386,7 @@ impl<'a> Context<'a> { ssa: &Ssa, function: &Function, brillig: &Brillig, - ) -> Result, RuntimeError> { + ) -> Result>, RuntimeError> { match function.runtime() { RuntimeType::Acir(inline_type) => { match inline_type { @@ -418,7 +418,7 @@ impl<'a> Context<'a> { main_func: &Function, ssa: &Ssa, brillig: &Brillig, - ) -> Result { + ) -> Result, RuntimeError> { let dfg = &main_func.dfg; let entry_block = &dfg[main_func.entry_block()]; let input_witness = self.convert_ssa_block_params(entry_block.parameters(), dfg)?; @@ -437,7 +437,7 @@ impl<'a> Context<'a> { mut self, main_func: &Function, brillig: &Brillig, - ) -> Result { + ) -> Result, RuntimeError> { let dfg = &main_func.dfg; let inputs = try_vecmap(dfg[main_func.entry_block()].parameters(), |param_id| { @@ -1869,7 +1869,7 @@ impl<'a> Context<'a> { } let binary_type = AcirType::from(binary_type); - let bit_count = binary_type.bit_size(); + let bit_count = binary_type.bit_size::(); let num_type = binary_type.to_numeric_type(); let result = match binary.operator { BinaryOp::Add => self.acir_context.add_var(lhs, rhs), From b390b7147f35e38b1e5084de5b2e16e3e3fb18b9 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 27 May 2024 16:32:59 +0000 Subject: [PATCH 5/8] chore: make `AcirContext` generic --- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 172 +++++++++--------- .../src/ssa/acir_gen/acir_ir/big_int.rs | 16 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 2 +- 3 files changed, 93 insertions(+), 97 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index a55b6983c53..bdea9aebb8e 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -20,7 +20,6 @@ use acvm::{ native_types::{Expression, Witness}, BlackBoxFunc, }, - FieldElement, }; use fxhash::FxHashMap as HashMap; use iter_extended::{try_vecmap, vecmap}; @@ -106,13 +105,13 @@ impl From for AcirType { /// Context object which holds the relationship between /// `Variables`(AcirVar) and types such as `Expression` and `Witness` /// which are placed into ACIR. -pub(crate) struct AcirContext { +pub(crate) struct AcirContext { /// Two-way map that links `AcirVar` to `AcirVarData`. /// /// The vars object is an instance of the `TwoWayMap`, which provides a bidirectional mapping between `AcirVar` and `AcirVarData`. - vars: HashMap, + vars: HashMap>, - constant_witnesses: HashMap, + constant_witnesses: HashMap, /// An in-memory representation of ACIR. /// @@ -121,13 +120,13 @@ pub(crate) struct AcirContext { /// For example, If one was to add two Variables together, /// then the `acir_ir` will be populated to assert this /// addition. - acir_ir: GeneratedAcir, + acir_ir: GeneratedAcir, /// The BigIntContext, used to generate identifiers for BigIntegers big_int_ctx: BigIntContext, } -impl AcirContext { +impl AcirContext { pub(crate) fn current_witness_index(&self) -> Witness { self.acir_ir.current_witness_index() } @@ -148,7 +147,7 @@ impl AcirContext { } /// Adds a constant to the context and assigns a Variable to represent it - pub(crate) fn add_constant(&mut self, constant: impl Into) -> AcirVar { + pub(crate) fn add_constant(&mut self, constant: impl Into) -> AcirVar { let constant_data = AcirVarData::Const(constant.into()); self.add_data(constant_data) } @@ -156,7 +155,7 @@ impl AcirContext { /// Returns the constant represented by the given variable. /// /// Panics: if the variable does not represent a constant. - pub(crate) fn constant(&self, var: AcirVar) -> FieldElement { + pub(crate) fn constant(&self, var: AcirVar) -> &F { self.vars[&var].as_constant().expect("ICE - expected the variable to be a constant value") } @@ -273,7 +272,7 @@ impl AcirContext { pub(crate) fn var_to_expression( &self, var: AcirVar, - ) -> Result, InternalError> { + ) -> Result, InternalError> { let var_data = match self.vars.get(&var) { Some(var_data) => var_data, None => { @@ -408,7 +407,7 @@ impl AcirContext { if lhs_expr == rhs_expr { // x ^ x == 0 - let zero = self.add_constant(FieldElement::zero()); + let zero = self.add_constant(F::zero()); return Ok(zero); } else if lhs_expr.is_zero() { // 0 ^ x == x @@ -418,14 +417,14 @@ impl AcirContext { return Ok(lhs); } - let bit_size = typ.bit_size::(); + let bit_size = typ.bit_size::(); if bit_size == 1 { // Operands are booleans. // // a ^ b == a + b - 2*a*b let prod = self.mul_var(lhs, rhs)?; let sum = self.add_var(lhs, rhs)?; - self.add_mul_var(sum, -FieldElement::from(2_i128), prod) + self.add_mul_var(sum, -F::from(2_u128), prod) } else { let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)]; let outputs = self.black_box_function(BlackBoxFunc::XOR, inputs, 1)?; @@ -448,11 +447,11 @@ impl AcirContext { return Ok(lhs); } else if lhs_expr.is_zero() || rhs_expr.is_zero() { // x & 0 == 0 and 0 & x == 0 - let zero = self.add_constant(FieldElement::zero()); + let zero = self.add_constant(F::zero()); return Ok(zero); } - let bit_size = typ.bit_size::(); + let bit_size = typ.bit_size::(); if bit_size == 1 { // Operands are booleans. self.mul_var(lhs, rhs) @@ -480,7 +479,7 @@ impl AcirContext { return Ok(lhs); } - let bit_size = typ.bit_size::(); + let bit_size = typ.bit_size::(); if bit_size == 1 { // Operands are booleans // a + b - ab @@ -502,7 +501,7 @@ impl AcirContext { &mut self, lhs: AcirVar, rhs: AcirVar, - assert_message: Option>, + assert_message: Option>, ) -> Result<(), RuntimeError> { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -531,7 +530,7 @@ impl AcirContext { pub(crate) fn vars_to_expressions_or_memory( &self, values: &[AcirValue], - ) -> Result>, RuntimeError> { + ) -> Result>, RuntimeError> { let mut result = Vec::with_capacity(values.len()); for value in values { match value { @@ -597,7 +596,7 @@ impl AcirContext { (AcirVarData::Const(constant), _) | (_, AcirVarData::Const(constant)) if constant.is_zero() => { - self.add_constant(FieldElement::zero()) + self.add_constant(F::zero()) } (AcirVarData::Const(lhs_constant), AcirVarData::Const(rhs_constant)) => { @@ -615,7 +614,7 @@ impl AcirContext { } (AcirVarData::Witness(lhs_witness), AcirVarData::Witness(rhs_witness)) => { let mut expr = Expression::default(); - expr.push_multiplication_term(FieldElement::one(), lhs_witness, rhs_witness); + expr.push_multiplication_term(F::one(), lhs_witness, rhs_witness); self.add_data(AcirVarData::Expr(expr)) } (AcirVarData::Expr(expression), AcirVarData::Witness(witness)) @@ -684,7 +683,7 @@ impl AcirContext { fn add_mul_var( &mut self, lhs: AcirVar, - k: FieldElement, + k: F, rhs: AcirVar, ) -> Result { let k_var = self.add_constant(k); @@ -695,7 +694,7 @@ impl AcirContext { /// Adds a new variable that is constrained to be the logical NOT of `x`. pub(crate) fn not_var(&mut self, x: AcirVar, typ: AcirType) -> Result { - let bit_size = typ.bit_size::(); + let bit_size = typ.bit_size::(); // Subtracting from max flips the bits let max = self.add_constant((1_u128 << bit_size) - 1); self.sub_var(max, x) @@ -709,8 +708,8 @@ impl AcirContext { bit_size: u32, predicate: AcirVar, ) -> Result<(AcirVar, AcirVar), RuntimeError> { - let zero = self.add_constant(FieldElement::zero()); - let one = self.add_constant(FieldElement::one()); + let zero = self.add_constant(F::zero()); + let one = self.add_constant(F::one()); let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -829,7 +828,7 @@ impl AcirContext { // Avoids overflow: 'q*b+r < 2^max_q_bits*2^max_rhs_bits' let mut avoid_overflow = false; - if max_q_bits + max_rhs_bits >= FieldElement::max_num_bits() - 1 { + if max_q_bits + max_rhs_bits >= F::max_num_bits() - 1 { // q*b+r can overflow; we avoid this when b is constant if rhs_expr.is_const() { avoid_overflow = true; @@ -843,16 +842,16 @@ impl AcirContext { if avoid_overflow { // we compute q0 = p/rhs let rhs_big = BigUint::from_bytes_be(&rhs_const.to_be_bytes()); - let q0_big = FieldElement::modulus() / &rhs_big; - let q0 = FieldElement::from_be_bytes_reduce(&q0_big.to_bytes_be()); + let q0_big = F::modulus() / &rhs_big; + let q0 = F::from_be_bytes_reduce(&q0_big.to_bytes_be()); let q0_var = self.add_constant(q0); // when q == q0, b*q+r can overflow so we need to bound r to avoid the overflow. let size_predicate = self.eq_var(q0_var, quotient_var)?; let predicate = self.mul_var(size_predicate, predicate)?; // Ensure that there is no overflow, under q == q0 predicate - let max_r_big = FieldElement::modulus() - q0_big * rhs_big; - let max_r = FieldElement::from_be_bytes_reduce(&max_r_big.to_bytes_be()); + let max_r_big = F::modulus() - q0_big * rhs_big; + let max_r = F::from_be_bytes_reduce(&max_r_big.to_bytes_be()); let max_r_var = self.add_constant(max_r); let max_r_predicate = self.mul_var(predicate, max_r_var)?; @@ -897,7 +896,7 @@ impl AcirContext { } assert!( - bits < FieldElement::max_num_bits(), + bits < F::max_num_bits(), "range check with bit size of the prime field is not implemented yet" ); @@ -921,7 +920,7 @@ impl AcirContext { // however, since it is a constant, we can compute it's actual bit size let r_bit_size = bit_size_u128(r); // witness = lhs_offset + r - assert!(bits + r_bit_size < FieldElement::max_num_bits()); //we need to ensure lhs_offset + r does not overflow + assert!(bits + r_bit_size < F::max_num_bits()); //we need to ensure lhs_offset + r does not overflow let r_var = self.add_constant(r); let aor = self.add_var(lhs_offset, r_var)?; @@ -946,13 +945,13 @@ impl AcirContext { max_bit_size: u32, ) -> Result { let max_power_of_two = self.add_constant( - FieldElement::from(2_i128).pow(&FieldElement::from(max_bit_size as i128 - 1)), + F::from(2_u128).pow(&F::from(max_bit_size as u128 - 1)), ); let intermediate = self.sub_var(max_power_of_two, lhs)?; let intermediate = self.mul_var(intermediate, leading)?; - self.add_mul_var(lhs, FieldElement::from(2_i128), intermediate) + self.add_mul_var(lhs, F::from(2_u128), intermediate) } /// Returns the quotient and remainder such that lhs = rhs * quotient + remainder @@ -977,9 +976,9 @@ impl AcirContext { // 2^{max_bit size-1} let max_power_of_two = self.add_constant( - FieldElement::from(2_i128).pow(&FieldElement::from(bit_size as i128 - 1)), + F::from(2_u128).pow(&F::from(bit_size as u128 - 1)), ); - let one = self.add_constant(FieldElement::one()); + let one = self.add_constant(F::one()); // Get the sign bit of rhs by computing rhs / max_power_of_two let (rhs_leading, _) = self.euclidean_division_var(rhs, max_power_of_two, bit_size, one)?; @@ -1071,8 +1070,8 @@ impl AcirContext { ) -> Result { // 2^{rhs} let divisor = - self.add_constant(FieldElement::from(2_u128).pow(&FieldElement::from(rhs as u128))); - let one = self.add_constant(FieldElement::one()); + self.add_constant(F::from(2_u128).pow(&F::from(rhs as u128))); + let one = self.add_constant(F::one()); // Computes lhs = 2^{rhs} * q + r let (_, remainder) = self.euclidean_division_var(lhs, divisor, max_bit_size, one)?; @@ -1092,8 +1091,8 @@ impl AcirContext { rhs: AcirVar, bit_count: u32, ) -> Result { - let pow_last = self.add_constant(FieldElement::from(1_u128 << (bit_count - 1))); - let pow = self.add_constant(FieldElement::from(1_u128 << (bit_count))); + let pow_last = self.add_constant(F::from(1_u128 << (bit_count - 1))); + let pow = self.add_constant(F::from(1_u128 << (bit_count))); // We check whether the inputs have same sign or not by computing the XOR of their bit sign @@ -1162,10 +1161,10 @@ impl AcirContext { // Ensure that 2^{max_bits + 1} is less than the field size // // TODO: perhaps this should be a user error, instead of an assert - assert!(max_bits + 1 < FieldElement::max_num_bits()); + assert!(max_bits + 1 < F::max_num_bits()); let two_max_bits = self - .add_constant(FieldElement::from(2_i128).pow(&FieldElement::from(max_bits as i128))); + .add_constant(F::from(2_u128).pow(&F::from(max_bits as u128))); let diff = self.sub_var(lhs, rhs)?; let comparison_evaluation = self.add_var(diff, two_max_bits)?; @@ -1213,7 +1212,7 @@ impl AcirContext { // compute less than. let comparison = self.more_than_eq_var(lhs, rhs, bit_size)?; - let one = self.add_constant(FieldElement::one()); + let one = self.add_constant(F::one()); self.sub_var(one, comparison) // comparison_negated } @@ -1250,7 +1249,7 @@ impl AcirContext { } }; - (vec![domain_constant], Vec::new()) + (vec![*domain_constant], Vec::new()) } BlackBoxFunc::Poseidon2Permutation => { // The last argument is the state length, which must be a constant @@ -1275,7 +1274,7 @@ impl AcirContext { } }; - (vec![state_len], Vec::new()) + (vec![*state_len], Vec::new()) } BlackBoxFunc::BigIntAdd | BlackBoxFunc::BigIntSub @@ -1296,7 +1295,7 @@ impl AcirContext { output_count = 0; let mut field_inputs = Vec::new(); for i in const_inputs { - field_inputs.push(i?); + field_inputs.push(*i?); } if field_inputs[1] != field_inputs[3] { return Err(RuntimeError::BigIntModulus { call_stack: self.get_call_stack() }); @@ -1305,7 +1304,7 @@ impl AcirContext { let result_id = self.big_int_ctx.new_big_int(field_inputs[1]); ( vec![field_inputs[0], field_inputs[2]], - vec![result_id.bigint_id(), result_id.modulus_id()], + vec![result_id.bigint_id::(), result_id.modulus_id::()], ) } BlackBoxFunc::BigIntToLeBytes => { @@ -1322,10 +1321,10 @@ impl AcirContext { inputs = Vec::new(); let mut field_inputs = Vec::new(); for i in const_inputs { - field_inputs.push(i?); + field_inputs.push(*i?); } let bigint = self.big_int_ctx.get(field_inputs[0]); - let modulus = self.big_int_ctx.modulus(bigint.modulus_id()); + let modulus = self.big_int_ctx.modulus(bigint.modulus_id::()); let bytes_len = ((modulus - BigUint::from(1_u32)).bits() - 1) / 8 + 1; output_count = bytes_len as usize; assert!(bytes_len == 32); @@ -1338,7 +1337,7 @@ impl AcirContext { match inputs.pop().expect(invalid_input) { AcirValue::Array(values) => { for value in values { - modulus.push(self.vars[&value.into_var()?].as_constant().ok_or( + modulus.push(*self.vars[&value.into_var()?].as_constant().ok_or( RuntimeError::InternalError(InternalError::NotAConstant { name: "big integer".to_string(), call_stack: self.get_call_stack(), @@ -1359,8 +1358,8 @@ impl AcirContext { let modulus_id = self.big_int_ctx.get_or_insert_modulus(big_modulus); let result_id = - self.big_int_ctx.new_big_int(FieldElement::from(modulus_id as u128)); - (modulus, vec![result_id.bigint_id(), result_id.modulus_id()]) + self.big_int_ctx.new_big_int(F::from(modulus_id as u128)); + (modulus, vec![result_id.bigint_id::(), result_id.modulus_id::()]) } BlackBoxFunc::AES128Encrypt => { let invalid_input = "aes128_encrypt - operation requires a plaintext to encrypt"; @@ -1375,7 +1374,7 @@ impl AcirContext { } }?; output_count = input_size + (16 - input_size % 16); - (vec![], vec![FieldElement::from(output_count as u128)]) + (vec![], vec![F::from(output_count as u128)]) } _ => (vec![], vec![]), }; @@ -1419,7 +1418,7 @@ impl AcirContext { // constants too. let witness_var = self.get_or_create_witness_var(input)?; let witness = self.var_to_witness(witness_var)?; - let num_bits = typ.bit_size::(); + let num_bits = typ.bit_size::(); single_val_witnesses.push(FunctionInput { witness, num_bits }); } witnesses.push(single_val_witnesses); @@ -1529,7 +1528,7 @@ impl AcirContext { mut self, inputs: Vec, warnings: Vec, - ) -> GeneratedAcir { + ) -> GeneratedAcir { self.acir_ir.input_witnesses = inputs; self.acir_ir.warnings = warnings; self.acir_ir @@ -1540,7 +1539,7 @@ impl AcirContext { /// Variable can be seen as an index into the context. /// We use a two-way map so that it is efficient to lookup /// either the key or the value. - fn add_data(&mut self, data: AcirVarData) -> AcirVar { + fn add_data(&mut self, data: AcirVarData) -> AcirVar { let id = AcirVar(self.vars.len()); self.vars.insert(id, data); id @@ -1550,7 +1549,7 @@ impl AcirContext { pub(crate) fn brillig_call( &mut self, predicate: AcirVar, - generated_brillig: &GeneratedBrillig, + generated_brillig: &GeneratedBrillig, inputs: Vec, outputs: Vec, attempt_execution: bool, @@ -1558,14 +1557,14 @@ impl AcirContext { brillig_function_index: u32, brillig_stdlib_func: Option, ) -> Result, RuntimeError> { - let brillig_inputs: Vec> = + let brillig_inputs: Vec> = try_vecmap(inputs, |i| -> Result<_, InternalError> { match i { AcirValue::Var(var, _) => { Ok(BrilligInputs::Single(self.var_to_expression(var)?)) } AcirValue::Array(vars) => { - let mut var_expressions: Vec> = Vec::new(); + let mut var_expressions: Vec> = Vec::new(); for var in vars { self.brillig_array_input(&mut var_expressions, var)?; } @@ -1616,8 +1615,8 @@ impl AcirContext { brillig_stdlib_func, ); - fn range_constraint_value( - context: &mut AcirContext, + fn range_constraint_value( + context: &mut AcirContext, value: &AcirValue, ) -> Result<(), RuntimeError> { match value { @@ -1652,7 +1651,7 @@ impl AcirContext { fn brillig_array_input( &mut self, - var_expressions: &mut Vec>, + var_expressions: &mut Vec>, input: AcirValue, ) -> Result<(), InternalError> { match input { @@ -1711,8 +1710,8 @@ impl AcirContext { fn execute_brillig( &mut self, - code: &[BrilligOpcode], - inputs: &[BrilligInputs], + code: &[BrilligOpcode], + inputs: &[BrilligInputs], outputs_types: &[AcirType], ) -> Option> { let mut memory = (execute_brillig(code, inputs)?).into_iter(); @@ -1737,7 +1736,7 @@ impl AcirContext { &mut self, element_types: &[AcirType], size: usize, - memory_iter: &mut impl Iterator>, + memory_iter: &mut impl Iterator>, ) -> AcirValue { let mut array_values = im::Vector::new(); for _ in 0..size { @@ -1818,7 +1817,7 @@ impl AcirContext { ) -> Result<(), InternalError> { let initialized_values = match optional_value { None => { - let zero = self.add_constant(FieldElement::zero()); + let zero = self.add_constant(F::zero()); let zero_witness = self.var_to_witness(zero)?; vec![zero_witness; len] } @@ -1890,13 +1889,13 @@ impl AcirContext { /// Enum representing the possible values that a /// Variable can be given. #[derive(Debug, Eq, Clone)] -enum AcirVarData { +enum AcirVarData { Witness(Witness), - Expr(Expression), - Const(FieldElement), + Expr(Expression), + Const(F), } -impl PartialEq for AcirVarData { +impl PartialEq for AcirVarData { fn eq(&self, other: &Self) -> bool { match (self, other) { (Self::Witness(l0), Self::Witness(r0)) => l0 == r0, @@ -1908,23 +1907,26 @@ impl PartialEq for AcirVarData { } // TODO: check/test this hash impl -impl std::hash::Hash for AcirVarData { +impl std::hash::Hash for AcirVarData { fn hash(&self, state: &mut H) { core::mem::discriminant(self).hash(state); } } -impl AcirVarData { +impl AcirVarData { /// Returns a FieldElement, if the underlying `AcirVarData` /// represents a constant. - pub(crate) fn as_constant(&self) -> Option { + pub(crate) fn as_constant(&self) -> Option<&F> { if let AcirVarData::Const(field) = self { - return Some(*field); + return Some(field); } None } +} + +impl AcirVarData { /// Converts all enum variants to an Expression. - pub(crate) fn to_expression(&self) -> Cow> { + pub(crate) fn to_expression(&self) -> Cow> { match self { AcirVarData::Witness(witness) => Cow::Owned(Expression::from(*witness)), AcirVarData::Expr(expr) => Cow::Borrowed(expr), @@ -1933,23 +1935,17 @@ impl AcirVarData { } } -impl From for AcirVarData { - fn from(constant: FieldElement) -> Self { - AcirVarData::Const(constant) - } -} - -impl From for AcirVarData { +impl From for AcirVarData { fn from(witness: Witness) -> Self { AcirVarData::Witness(witness) } } -impl From> for AcirVarData { - fn from(expr: Expression) -> Self { +impl From> for AcirVarData { + fn from(expr: Expression) -> Self { // Prefer simpler variants if possible. if let Some(constant) = expr.to_const() { - AcirVarData::from(*constant) + AcirVarData::Const(*constant) } else if let Some(witness) = expr.to_witness() { AcirVarData::from(witness) } else { @@ -1967,12 +1963,12 @@ pub(crate) struct AcirVar(usize); /// Returns the finished state of the Brillig VM if execution can complete. /// /// Returns `None` if complete execution of the Brillig bytecode is not possible. -fn execute_brillig( - code: &[BrilligOpcode], - inputs: &[BrilligInputs], -) -> Option>> { +fn execute_brillig( + code: &[BrilligOpcode], + inputs: &[BrilligInputs], +) -> Option>> { // Set input values - let mut calldata: Vec = Vec::new(); + let mut calldata: Vec = Vec::new(); // Each input represents a constant or array of constants. // Iterate over each input and push it into registers and/or memory. diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/big_int.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/big_int.rs index b9c596d80c7..30297b42ecf 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/big_int.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/big_int.rs @@ -1,4 +1,4 @@ -use acvm::{acir::AcirField, FieldElement}; +use acvm::acir::AcirField; use num_bigint::BigUint; /// Represents a bigint value in the form (id, modulus) where @@ -11,12 +11,12 @@ pub(crate) struct BigIntId { } impl BigIntId { - pub(crate) fn bigint_id(&self) -> FieldElement { - FieldElement::from(self.bigint_id as u128) + pub(crate) fn bigint_id>(&self) -> F { + F::from(self.bigint_id as u128) } - pub(crate) fn modulus_id(&self) -> FieldElement { - FieldElement::from(self.modulus_id as u128) + pub(crate) fn modulus_id>(&self) -> F { + F::from(self.modulus_id as u128) } } @@ -29,7 +29,7 @@ pub(crate) struct BigIntContext { impl BigIntContext { /// Creates a new BigIntId for the given modulus identifier and returns it. - pub(crate) fn new_big_int(&mut self, modulus_id: FieldElement) -> BigIntId { + pub(crate) fn new_big_int(&mut self, modulus_id: F) -> BigIntId { let id = self.big_integers.len() as u32; let result = BigIntId { bigint_id: id, modulus_id: modulus_id.to_u128() as u32 }; self.big_integers.push(result); @@ -37,12 +37,12 @@ impl BigIntContext { } /// Returns the modulus corresponding to the given modulus index - pub(crate) fn modulus(&self, idx: FieldElement) -> BigUint { + pub(crate) fn modulus(&self, idx: F) -> BigUint { self.modulus[idx.to_u128() as usize].clone() } /// Returns the BigIntId corresponding to the given identifier - pub(crate) fn get(&self, id: FieldElement) -> BigIntId { + pub(crate) fn get(&self, id: F) -> BigIntId { self.big_integers[id.to_u128() as usize] } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 54c64994e27..4a3b44e98e6 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -155,7 +155,7 @@ struct Context<'a> { current_side_effects_enabled_var: AcirVar, /// Manages and builds the `AcirVar`s to which the converted SSA values refer. - acir_context: AcirContext, + acir_context: AcirContext, /// Track initialized acir dynamic arrays /// From 62c1d7b33250cbe8b96abc8291812cc6a404c33d Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 27 May 2024 16:36:20 +0000 Subject: [PATCH 6/8] chore: make `SharedContext` generic --- .../src/ssa/acir_gen/acir_ir/generated_acir.rs | 3 +-- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 16 ++++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 6f98e41ee6b..8d08e127143 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -19,7 +19,6 @@ use acvm::acir::{ use acvm::{ acir::AcirField, acir::{circuit::directives::Directive, native_types::Expression}, - FieldElement, }; use iter_extended::vecmap; use num_bigint::BigUint; @@ -83,7 +82,7 @@ pub(crate) enum BrilligStdlibFunc { } impl BrilligStdlibFunc { - pub(crate) fn get_generated_brillig(&self) -> GeneratedBrillig { + pub(crate) fn get_generated_brillig(&self) -> GeneratedBrillig { match self { BrilligStdlibFunc::Inverse => brillig_directive::directive_invert(), BrilligStdlibFunc::Quotient(bit_size) => { diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 4a3b44e98e6..d34ca1e5120 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -46,11 +46,11 @@ use im::Vector; use iter_extended::{try_vecmap, vecmap}; #[derive(Default)] -struct SharedContext { +struct SharedContext { /// Final list of Brillig functions which will be part of the final program /// This is shared across `Context` structs as we want one list of Brillig /// functions across all ACIR artifacts - generated_brillig: Vec>, + generated_brillig: Vec>, /// Maps SSA function index -> Final generated Brillig artifact index. /// There can be Brillig functions specified in SSA which do not act as @@ -69,7 +69,7 @@ struct SharedContext { brillig_stdlib_calls_to_resolve: HashMap>, } -impl SharedContext { +impl SharedContext { fn generated_brillig_pointer( &self, func_id: FunctionId, @@ -78,7 +78,7 @@ impl SharedContext { self.brillig_generated_func_pointers.get(&(func_id, arguments)) } - fn generated_brillig(&self, func_pointer: usize) -> &GeneratedBrillig { + fn generated_brillig(&self, func_pointer: usize) -> &GeneratedBrillig { &self.generated_brillig[func_pointer] } @@ -87,7 +87,7 @@ impl SharedContext { func_id: FunctionId, arguments: Vec, generated_pointer: u32, - code: GeneratedBrillig, + code: GeneratedBrillig, ) { self.brillig_generated_func_pointers.insert((func_id, arguments), generated_pointer); self.generated_brillig.push(code); @@ -127,7 +127,7 @@ impl SharedContext { generated_pointer: u32, func_id: FunctionId, opcode_location: OpcodeLocation, - code: GeneratedBrillig, + code: GeneratedBrillig, ) { self.brillig_stdlib_func_pointer.insert(brillig_stdlib_func, generated_pointer); self.add_call_to_resolve(func_id, (opcode_location, generated_pointer)); @@ -193,7 +193,7 @@ struct Context<'a> { data_bus: DataBus, /// Contains state that is generated and also used across ACIR functions - shared_context: &'a mut SharedContext, + shared_context: &'a mut SharedContext, } #[derive(Clone)] @@ -363,7 +363,7 @@ fn generate_distinct_return_witnesses(acir: &mut GeneratedAcir) } impl<'a> Context<'a> { - fn new(shared_context: &'a mut SharedContext) -> Context<'a> { + fn new(shared_context: &'a mut SharedContext) -> Context<'a> { let mut acir_context = AcirContext::default(); let current_side_effects_enabled_var = acir_context.add_constant(FieldElement::one()); From e27af67b2d8ab89492eff8921f14be9e36ac061e Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 29 May 2024 13:41:23 +0100 Subject: [PATCH 7/8] chore: fmt --- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 31 ++++++------------- .../ssa/acir_gen/acir_ir/generated_acir.rs | 11 ++----- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 7 +++-- 3 files changed, 16 insertions(+), 33 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index bdea9aebb8e..cd51a73cdb8 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -269,10 +269,7 @@ impl AcirContext { } /// Converts an [`AcirVar`] to an [`Expression`] - pub(crate) fn var_to_expression( - &self, - var: AcirVar, - ) -> Result, InternalError> { + pub(crate) fn var_to_expression(&self, var: AcirVar) -> Result, InternalError> { let var_data = match self.vars.get(&var) { Some(var_data) => var_data, None => { @@ -680,12 +677,7 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the expression `lhs + k * rhs` - fn add_mul_var( - &mut self, - lhs: AcirVar, - k: F, - rhs: AcirVar, - ) -> Result { + fn add_mul_var(&mut self, lhs: AcirVar, k: F, rhs: AcirVar) -> Result { let k_var = self.add_constant(k); let intermediate = self.mul_var(k_var, rhs)?; @@ -944,9 +936,8 @@ impl AcirContext { leading: AcirVar, max_bit_size: u32, ) -> Result { - let max_power_of_two = self.add_constant( - F::from(2_u128).pow(&F::from(max_bit_size as u128 - 1)), - ); + let max_power_of_two = + self.add_constant(F::from(2_u128).pow(&F::from(max_bit_size as u128 - 1))); let intermediate = self.sub_var(max_power_of_two, lhs)?; let intermediate = self.mul_var(intermediate, leading)?; @@ -975,9 +966,8 @@ impl AcirContext { assert_ne!(bit_size, 0, "signed integer should have at least one bit"); // 2^{max_bit size-1} - let max_power_of_two = self.add_constant( - F::from(2_u128).pow(&F::from(bit_size as u128 - 1)), - ); + let max_power_of_two = + self.add_constant(F::from(2_u128).pow(&F::from(bit_size as u128 - 1))); let one = self.add_constant(F::one()); // Get the sign bit of rhs by computing rhs / max_power_of_two @@ -1069,8 +1059,7 @@ impl AcirContext { max_bit_size: u32, ) -> Result { // 2^{rhs} - let divisor = - self.add_constant(F::from(2_u128).pow(&F::from(rhs as u128))); + let divisor = self.add_constant(F::from(2_u128).pow(&F::from(rhs as u128))); let one = self.add_constant(F::one()); // Computes lhs = 2^{rhs} * q + r @@ -1163,8 +1152,7 @@ impl AcirContext { // TODO: perhaps this should be a user error, instead of an assert assert!(max_bits + 1 < F::max_num_bits()); - let two_max_bits = self - .add_constant(F::from(2_u128).pow(&F::from(max_bits as u128))); + let two_max_bits = self.add_constant(F::from(2_u128).pow(&F::from(max_bits as u128))); let diff = self.sub_var(lhs, rhs)?; let comparison_evaluation = self.add_var(diff, two_max_bits)?; @@ -1357,8 +1345,7 @@ impl AcirContext { output_count = 0; let modulus_id = self.big_int_ctx.get_or_insert_modulus(big_modulus); - let result_id = - self.big_int_ctx.new_big_int(F::from(modulus_id as u128)); + let result_id = self.big_int_ctx.new_big_int(F::from(modulus_id as u128)); (modulus, vec![result_id.bigint_id::(), result_id.modulus_id::()]) } BlackBoxFunc::AES128Encrypt => { diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 8d08e127143..a18b34616e5 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -142,10 +142,7 @@ impl GeneratedAcir { /// This means you cannot multiply an infinite amount of `Expression`s together. /// Once the `Expression` goes over degree-2, then it needs to be reduced to a `Witness` /// which has degree-1 in order to be able to continue the multiplication chain. - pub(crate) fn create_witness_for_expression( - &mut self, - expression: &Expression, - ) -> Witness { + pub(crate) fn create_witness_for_expression(&mut self, expression: &Expression) -> Witness { let fresh_witness = self.next_witness_index(); // Create a constraint that sets them to be equal to each other @@ -472,11 +469,7 @@ impl GeneratedAcir { /// Returns a `Witness` that is constrained to be: /// - `1` if `lhs == rhs` /// - `0` otherwise - pub(crate) fn is_equal( - &mut self, - lhs: &Expression, - rhs: &Expression, - ) -> Witness { + pub(crate) fn is_equal(&mut self, lhs: &Expression, rhs: &Expression) -> Witness { let t = lhs - rhs; self.is_zero(&t) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index d34ca1e5120..876c0ed6992 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -278,8 +278,11 @@ impl AcirValue { } } -pub(crate) type Artifacts = - (Vec>, Vec>, BTreeMap); +pub(crate) type Artifacts = ( + Vec>, + Vec>, + BTreeMap, +); impl Ssa { #[tracing::instrument(level = "trace", skip_all)] From eb253447efde3aa4e530661c04211db590bafb64 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 12 Jun 2024 14:58:32 +0100 Subject: [PATCH 8/8] Apply suggestions from code review --- acvm-repo/acir_field/src/field_element.rs | 1 - .../src/brillig/brillig_gen/brillig_directive.rs | 2 +- .../noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/acvm-repo/acir_field/src/field_element.rs b/acvm-repo/acir_field/src/field_element.rs index 6cefddd5fe6..3c16276adc9 100644 --- a/acvm-repo/acir_field/src/field_element.rs +++ b/acvm-repo/acir_field/src/field_element.rs @@ -286,7 +286,6 @@ impl AcirField for FieldElement { (self.num_bits() <= 64).then(|| self.to_u128() as u64) } - fn try_to_u32(&self) -> Option { (self.num_bits() <= 32).then(|| self.to_u128() as u32) } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs index 691ad68351e..ae159f2c45c 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs @@ -41,7 +41,7 @@ pub(crate) fn directive_invert() -> GeneratedBrillig { // Put value one in register (1) BrilligOpcode::Const { destination: one_const, - value: F::from(1_usize), + value: F::one(), bit_size: F::max_num_bits(), }, // Divide 1 by the input, and set the result of the division into register (0) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index c0d24205b69..56b869fbf6b 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -971,7 +971,6 @@ impl AcirContext { let zero = self.add_constant(F::zero()); let one = self.add_constant(F::one()); - // Get the sign bit of rhs by computing rhs / max_power_of_two let (rhs_leading, _) = self.euclidean_division_var(rhs, max_power_of_two, bit_size, one)?;