Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: pull out refactored methods from u128 branch #7385

Merged
merged 6 commits into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Seeds for failure cases proptest has generated in the past. It is
# automatically read and these particular cases re-run before any
# novel cases are generated.
#
# It is recommended to check this file in to source control so that
# everyone who runs the test benefits from these saved cases.
cc 5c73714aa60966f1e26e79441f4a3102fe768cc6c87c9421d44cbc4b9ffa8d66 # shrinks to bit_size = 0
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Seeds for failure cases proptest has generated in the past. It is
# automatically read and these particular cases re-run before any
# novel cases are generated.
#
# It is recommended to check this file in to source control so that
# everyone who runs the test benefits from these saved cases.
cc 696c50cf99099048e0e1ea8d77561cc6cf49a71b480a05e0ef437d57f97c454a # shrinks to input = 0, bit_size = 0
66 changes: 60 additions & 6 deletions compiler/noirc_evaluator/src/acir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1026,8 +1026,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
leading: AcirVar,
max_bit_size: u32,
) -> Result<AcirVar, RuntimeError> {
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(power_of_two::<F>(max_bit_size - 1));

let intermediate = self.sub_var(max_power_of_two, lhs)?;
let intermediate = self.mul_var(intermediate, leading)?;
Expand Down Expand Up @@ -1057,8 +1056,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
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(power_of_two::<F>(bit_size - 1));
let zero = self.add_constant(F::zero());
let one = self.add_constant(F::one());

Expand Down Expand Up @@ -1164,7 +1162,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
max_bit_size: u32,
) -> Result<AcirVar, RuntimeError> {
// 2^{rhs}
let divisor = self.add_constant(F::from(2_u128).pow(&F::from(rhs as u128)));
let divisor = self.add_constant(power_of_two::<F>(rhs));
let one = self.add_constant(F::one());

// Computes lhs = 2^{rhs} * q + r
Expand Down Expand Up @@ -1257,7 +1255,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
// 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(power_of_two::<F>(max_bits));
let diff = self.sub_var(lhs, rhs)?;
let comparison_evaluation = self.add_var(diff, two_max_bits)?;

Expand Down Expand Up @@ -1568,6 +1566,36 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
}
}

/// Returns an `F` representing the value `2**power`
///
/// # Panics
///
/// Panics if `2**power` exceeds `F::modulus()`.
pub(super) fn power_of_two<F: AcirField>(power: u32) -> F {
if power >= F::max_num_bits() {
panic!("Field cannot represent this power of two");
}
let full_bytes = power / 8;
let extra_bits = power % 8;
let most_significant_byte: u8 = match extra_bits % 8 {
0 => 0x01,
1 => 0x02,
2 => 0x04,
3 => 0x08,
4 => 0x10,
5 => 0x20,
6 => 0x40,
7 => 0x80,
_ => unreachable!("We cover the full range of x % 8"),
};

let bytes_be: Vec<u8> = std::iter::once(most_significant_byte)
.chain(std::iter::repeat(0).take(full_bytes as usize))
.collect();

F::from_be_bytes_reduce(&bytes_be)
}

/// Enum representing the possible values that a
/// Variable can be given.
#[derive(Debug, Eq, Clone)]
Expand Down Expand Up @@ -1656,3 +1684,29 @@ fn fits_in_one_identity<F: AcirField>(expr: &Expression<F>, width: ExpressionWid
/// A Reference to an `AcirVarData`
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub(crate) struct AcirVar(usize);

#[cfg(test)]
mod test {
use acvm::{AcirField, FieldElement};
use proptest::prelude::*;

use super::power_of_two;

#[test]
#[should_panic = "Field cannot represent this power of two"]
fn power_of_two_panics_on_overflow() {
power_of_two::<FieldElement>(FieldElement::max_num_bits());
}

proptest! {
#[test]
fn power_of_two_agrees_with_generic_impl(bit_size in (0..=128u32)) {
let power_of_two_general =
FieldElement::from(2_u128).pow(&FieldElement::from(bit_size));
let power_of_two_opt: FieldElement = power_of_two(bit_size);

prop_assert_eq!(power_of_two_opt, power_of_two_general);
}

}
}
6 changes: 3 additions & 3 deletions compiler/noirc_evaluator/src/acir/generated_acir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,13 +356,13 @@ impl<F: AcirField> GeneratedAcir<F> {
limb_count: u32,
bit_size: u32,
) -> Result<Vec<Witness>, RuntimeError> {
let radix_big = BigUint::from(radix);
let radix_range = BigUint::from(2u128)..=BigUint::from(256u128);
let radix_range = 2..=256;
assert!(
radix_range.contains(&radix_big),
radix_range.contains(&radix),
"ICE: Radix must be in the range 2..=256, but found: {:?}",
radix
);
let radix_big = BigUint::from(radix);
assert_eq!(
BigUint::from(2u128).pow(bit_size),
radix_big,
Expand Down
5 changes: 3 additions & 2 deletions compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ use crate::ssa::{
},
ssa_gen::Ssa,
};
use acir_variable::{AcirContext, AcirType, AcirVar};
use acir_variable::{power_of_two, AcirContext, AcirType, AcirVar};
use generated_acir::BrilligStdlibFunc;
pub(crate) use generated_acir::GeneratedAcir;
use noirc_frontend::hir_def::types::Type as HirType;
Expand Down Expand Up @@ -2122,7 +2122,8 @@ impl<'a> Context<'a> {
) {
// Subtractions must first have the integer modulus added before truncation can be
// applied. This is done in order to prevent underflow.
let integer_modulus = self.acir_context.add_constant(2_u128.pow(bit_size));
let integer_modulus = power_of_two::<FieldElement>(bit_size);
let integer_modulus = self.acir_context.add_constant(integer_modulus);
var = self.acir_context.add_var(var, integer_modulus)?;
}
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use binary::truncate_field;
use serde::{Deserialize, Serialize};
use std::hash::{Hash, Hasher};

Expand Down Expand Up @@ -425,7 +426,7 @@
// These can fail.
Constrain(..) | ConstrainNotEqual(..) | RangeCheck { .. } => true,

// This should never be side-effectful

Check warning on line 429 in compiler/noirc_evaluator/src/ssa/ir/instruction.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (effectful)
MakeArray { .. } | Noop => false,

// Some binary math can overflow or underflow
Expand Down Expand Up @@ -968,9 +969,8 @@
return SimplifiedTo(*value);
}
if let Some((numeric_constant, typ)) = dfg.get_numeric_constant_with_type(*value) {
let integer_modulus = 2_u128.pow(*bit_size);
let truncated = numeric_constant.to_u128() % integer_modulus;
SimplifiedTo(dfg.make_constant(truncated.into(), typ))
let truncated_field = truncate_field(numeric_constant, *bit_size);
SimplifiedTo(dfg.make_constant(truncated_field, typ))
} else if let Value::Instruction { instruction, .. } = &dfg[dfg.resolve(*value)] {
match &dfg[*instruction] {
Instruction::Truncate { bit_size: src_bit_size, .. } => {
Expand Down
57 changes: 53 additions & 4 deletions compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@
Or,
/// Bitwise xor (^)
Xor,
/// Bitshift left (<<)

Check warning on line 42 in compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Bitshift)
Shl,
/// Bitshift right (>>)

Check warning on line 44 in compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Bitshift)
Shr,
}

Expand Down Expand Up @@ -107,7 +107,7 @@
}

let operator = if lhs_type == NumericType::NativeField {
// Unchecked operations between fields or bools don't make sense, so we convert those to non-unchecked

Check warning on line 110 in compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bools)
// to reduce noise and confusion in the generated SSA.
match operator {
BinaryOp::Add { unchecked: true } => BinaryOp::Add { unchecked: false },
Expand All @@ -116,7 +116,7 @@
_ => operator,
}
} else if lhs_type == NumericType::bool() {
// Unchecked mul between bools doesn't make sense, so we convert that to non-unchecked

Check warning on line 119 in compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bools)
if let BinaryOp::Mul { unchecked: true } = operator {
BinaryOp::Mul { unchecked: false }
} else {
Expand Down Expand Up @@ -289,7 +289,7 @@
}
if lhs_type == NumericType::bool() {
// Boolean AND is equivalent to multiplication, which is a cheaper operation.
// (mul unchecked because these are bools so it doesn't matter really)

Check warning on line 292 in compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bools)
let instruction =
Instruction::binary(BinaryOp::Mul { unchecked: true }, lhs, rhs);
return SimplifyResult::SimplifiedToInstruction(instruction);
Expand Down Expand Up @@ -441,7 +441,7 @@
}
let result = function(lhs, rhs)?;
// Check for overflow
if result >= 1 << bit_size {
if result != 0 && result.ilog2() >= bit_size {
return None;
}
result.into()
Expand Down Expand Up @@ -489,6 +489,7 @@
let signed_int = if is_positive {
unsigned_int as i128
} else {
assert!(bit_size < 128);
let x = (1u128 << bit_size) - unsigned_int;
-(x as i128)
};
Expand All @@ -501,14 +502,45 @@
FieldElement::from(int)
} else {
// We add an offset of `bit_size` bits to shift the negative values into the range [2^(bitsize-1), 2^bitsize)
assert!(bit_size < 128);
let offset_int = (1i128 << bit_size) + int;
FieldElement::from(offset_int)
}
}

/// Truncates `int` to fit within `bit_size` bits.
fn truncate(int: u128, bit_size: u32) -> u128 {
let max = 1 << bit_size;
int % max
if bit_size == 128 {
int
} else {
let max = 1 << bit_size;
int % max
}
}

pub(crate) fn truncate_field<F: AcirField>(int: F, bit_size: u32) -> F {
if bit_size == 0 {
return F::zero();
}
let num_bytes = bit_size.div_ceil(8);
let mut be_bytes: Vec<u8> =
int.to_be_bytes().into_iter().rev().take(num_bytes as usize).rev().collect();

// We need to apply a mask to the largest byte to handle non-divisible bit sizes.
let mask = match bit_size % 8 {
0 => 0xff,
1 => 0x01,
2 => 0x03,
3 => 0x07,
4 => 0x0f,
5 => 0x1f,
6 => 0x3f,
7 => 0x7f,
_ => unreachable!("We cover the full range of x % 8"),
};
be_bytes[0] &= mask;

F::from_be_bytes_reduce(&be_bytes)
}

impl BinaryOp {
Expand Down Expand Up @@ -588,8 +620,12 @@
use proptest::prelude::*;

use super::{
convert_signed_integer_to_field_element, try_convert_field_element_to_signed_integer,
convert_signed_integer_to_field_element, truncate_field,
try_convert_field_element_to_signed_integer,
};
use acvm::{AcirField, FieldElement};
use num_bigint::BigUint;
use num_traits::One;

proptest! {
#[test]
Expand All @@ -601,5 +637,18 @@

prop_assert_eq!(int, recovered_int);
}

#[test]
fn truncate_field_agrees_with_bigint_modulo(input: u128, bit_size in (0..=253u32)) {
let field = FieldElement::from(input);
let truncated_as_field = truncate_field(field, bit_size);

let integer_modulus = BigUint::from(2_u128).pow(bit_size);
let truncated_as_bigint = BigUint::from(input)
.modpow(&BigUint::one(), &integer_modulus);

Check warning on line 648 in compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (modpow)
let truncated_as_bigint = FieldElement::from_be_bytes_reduce(&truncated_as_bigint.to_bytes_be());
prop_assert_eq!(truncated_as_field, truncated_as_bigint);
}

}
}
1 change: 1 addition & 0 deletions compiler/noirc_printable_type/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ acvm.workspace = true
serde.workspace = true

[dev-dependencies]
proptest.workspace = true
24 changes: 23 additions & 1 deletion compiler/noirc_printable_type/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,13 @@
output.push_str(&format_field_string(*f));
}
(PrintableValue::Field(f), PrintableType::UnsignedInteger { width }) => {
let uint_cast = f.to_u128() & ((1 << width) - 1); // Retain the lower 'width' bits
// Retain the lower 'width' bits
debug_assert!(*width <= 128, "We don't currently support uints larger than u128");

Check warning on line 101 in compiler/noirc_printable_type/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (uints)
let mut uint_cast = f.to_u128();
if *width != 128 {
uint_cast &= (1 << width) - 1;
};

output.push_str(&uint_cast.to_string());
}
(PrintableValue::Field(f), PrintableType::SignedInteger { width }) => {
Expand Down Expand Up @@ -167,8 +173,8 @@

(PrintableValue::Vec { array_elements, .. }, PrintableType::Tuple { types }) => {
output.push('(');
let mut elems = array_elements.iter().zip(types).peekable();

Check warning on line 176 in compiler/noirc_printable_type/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (elems)
while let Some((value, typ)) = elems.next() {

Check warning on line 177 in compiler/noirc_printable_type/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (elems)
output.push_str(
&PrintableValueDisplay::Plain(value.clone(), typ.clone()).to_string(),
);
Expand Down Expand Up @@ -267,6 +273,10 @@
mod tests {
use acvm::FieldElement;

use proptest::prelude::*;

use crate::to_string;

use super::{PrintableType, PrintableValue, PrintableValueDisplay};

#[test]
Expand Down Expand Up @@ -299,4 +309,16 @@
PrintableValueDisplay::<FieldElement>::FmtString(template.to_string(), values);
assert_eq!(display.to_string(), expected);
}

proptest! {
#[test]
fn handles_decoding_u128_values(uint_value: u128) {
let value = PrintableValue::Field(FieldElement::from(uint_value));
let typ = PrintableType::UnsignedInteger { width: 128 };

let value_as_string = to_string(&value, &typ).unwrap();
// We want to match rust's stringification.
prop_assert_eq!(value_as_string, uint_value.to_string());
}
}
}
Loading