Skip to content

Commit

Permalink
Merge branch 'master' into tf/simplify-reports
Browse files Browse the repository at this point in the history
* master:
  fix(performance): Remove redundant slice access check from brillig (#7434)
  chore(docs): updating tutorials and other nits to beta.2 (#7405)
  feat: LSP hover for integer literals (#7368)
  feat(experimental): Compile match expressions (#7312)
  feat(acir_field): Add little-endian byte serialization for FieldElement (#7258)
  feat: allow unquoting TraitConstraint in trait impl position (#7395)
  feat(brillig): Hoist shared constants across functions to the global space (#7216)
  feat(LSP): auto-import via visible reexport (#7409)
  fix(brillig): Brillig entry point analysis and function specialization through duplication (#7277)
  chore: redo typo PR by maximevtush (#7425)
  fix(ssa): Accurately mark binary ops for hoisting and check Div/Mod against induction variable lower bound (#7396)
  feat!: remove bigint from stdlib (#7411)
  • Loading branch information
TomAFrench committed Feb 19, 2025
2 parents e272db3 + 49a095d commit 48de313
Show file tree
Hide file tree
Showing 97 changed files with 4,946 additions and 2,197 deletions.
4 changes: 2 additions & 2 deletions .github/benchmark_projects.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ projects:
path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset
num_runs: 5
timeout: 250
compilation-timeout: 7
compilation-timeout: 8
execution-timeout: 0.35
compilation-memory-limit: 750
execution-memory-limit: 300
Expand Down Expand Up @@ -74,7 +74,7 @@ projects:
num_runs: 1
timeout: 60
compilation-timeout: 100
execution-timeout: 35
execution-timeout: 40
compilation-memory-limit: 7000
execution-memory-limit: 1500
rollup-merge:
Expand Down
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.

61 changes: 57 additions & 4 deletions acvm-repo/acir_field/src/field_element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,15 @@ impl<F: PrimeField> AcirField for FieldElement<F> {
}

fn to_be_bytes(self) -> Vec<u8> {
// 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 = self.to_le_bytes();
bytes.reverse();
bytes
}

/// Converts the field element to a vector of bytes in little-endian order
fn to_le_bytes(self) -> Vec<u8> {
let mut bytes = Vec::new();
self.0.serialize_uncompressed(&mut bytes).unwrap();
bytes.reverse();
bytes
}

Expand All @@ -289,6 +292,12 @@ impl<F: PrimeField> AcirField for FieldElement<F> {
FieldElement(F::from_be_bytes_mod_order(bytes))
}

/// Converts bytes in little-endian order into a FieldElement and applies a
/// reduction if needed.
fn from_le_bytes_reduce(bytes: &[u8]) -> FieldElement<F> {
FieldElement(F::from_le_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<u8> {
Expand Down Expand Up @@ -405,6 +414,50 @@ mod tests {
assert_eq!(max_num_bits_bn254, 254);
}

proptest! {
#[test]
fn test_endianness_prop(value in any::<u64>()) {
let field = FieldElement::<ark_bn254::Fr>::from(value);
// Test serialization consistency
let le_bytes = field.to_le_bytes();
let be_bytes = field.to_be_bytes();

let mut reversed_le = le_bytes.clone();
reversed_le.reverse();
prop_assert_eq!(&be_bytes, &reversed_le, "BE bytes should be reverse of LE bytes");

// Test deserialization consistency
let from_le = FieldElement::from_le_bytes_reduce(&le_bytes);
let from_be = FieldElement::from_be_bytes_reduce(&be_bytes);
prop_assert_eq!(from_le, from_be, "Deserialization should be consistent between LE and BE");
prop_assert_eq!(from_le, field, "Deserialized value should match original");
}
}

#[test]
fn test_endianness() {
let field = FieldElement::<ark_bn254::Fr>::from(0x1234_5678_u32);
let le_bytes = field.to_le_bytes();
let be_bytes = field.to_be_bytes();

// Check that the bytes are reversed between BE and LE
let mut reversed_le = le_bytes.clone();
reversed_le.reverse();
assert_eq!(&be_bytes, &reversed_le);

// Verify we can reconstruct the same field element from either byte order
let from_le = FieldElement::from_le_bytes_reduce(&le_bytes);
let from_be = FieldElement::from_be_bytes_reduce(&be_bytes);
assert_eq!(from_le, from_be);
assert_eq!(from_le, field);

// Additional test with a larger number to ensure proper byte handling
let large_field = FieldElement::<ark_bn254::Fr>::from(0x0123_4567_89AB_CDEF_u64);
let large_le = large_field.to_le_bytes();
let reconstructed = FieldElement::from_le_bytes_reduce(&large_le);
assert_eq!(reconstructed, large_field);
}

proptest! {
// This currently panics due to the fact that we allow inputs which are greater than the field modulus,
// automatically reducing them to fit within the canonical range.
Expand Down
6 changes: 6 additions & 0 deletions acvm-repo/acir_field/src/generic_ark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ pub trait AcirField:
/// Converts bytes into a FieldElement and applies a reduction if needed.
fn from_be_bytes_reduce(bytes: &[u8]) -> Self;

/// Converts bytes in little-endian order into a FieldElement and applies a reduction if needed.
fn from_le_bytes_reduce(bytes: &[u8]) -> Self;

/// Converts the field element to a vector of bytes in little-endian order
fn to_le_bytes(self) -> Vec<u8>;

/// Returns the closest number of bytes to the bits specified
/// This method truncates
fn fetch_nearest_bytes(&self, num_bits: usize) -> Vec<u8>;
Expand Down
6 changes: 4 additions & 2 deletions compiler/noirc_evaluator/src/brillig/brillig_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pub(crate) mod brillig_block_variables;
pub(crate) mod brillig_fn;
pub(crate) mod brillig_globals;
pub(crate) mod brillig_slice_ops;
mod constant_allocation;
pub(crate) mod constant_allocation;
mod variable_liveness;

use acvm::FieldElement;
Expand All @@ -20,14 +20,15 @@ use super::{
};
use crate::{
errors::InternalError,
ssa::ir::{call_stack::CallStack, function::Function},
ssa::ir::{call_stack::CallStack, function::Function, types::NumericType},
};

/// Converting an SSA function into Brillig bytecode.
pub(crate) fn convert_ssa_function(
func: &Function,
options: &BrilligOptions,
globals: &HashMap<ValueId, BrilligVariable>,
hoisted_global_constants: &HashMap<(FieldElement, NumericType), BrilligVariable>,
) -> BrilligArtifact<FieldElement> {
let mut brillig_context = BrilligContext::new(options);

Expand All @@ -44,6 +45,7 @@ pub(crate) fn convert_ssa_function(
block,
&func.dfg,
globals,
hoisted_global_constants,
);
}

Expand Down
55 changes: 50 additions & 5 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ use acvm::{acir::AcirField, FieldElement};
use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
use iter_extended::vecmap;
use num_bigint::BigUint;
use std::collections::BTreeSet;
use std::sync::Arc;

use super::brillig_black_box::convert_black_box_call;
use super::brillig_block_variables::BlockVariables;
use super::brillig_block_variables::{allocate_value_with_type, BlockVariables};
use super::brillig_fn::FunctionContext;
use super::brillig_globals::HoistedConstantsToBrilligGlobals;
use super::constant_allocation::InstructionLocation;

/// Generate the compilation artifacts for compiling a function into brillig bytecode.
Expand All @@ -45,6 +47,7 @@ pub(crate) struct BrilligBlock<'block, Registers: RegisterAllocator> {
pub(crate) last_uses: HashMap<InstructionId, HashSet<ValueId>>,

pub(crate) globals: &'block HashMap<ValueId, BrilligVariable>,
pub(crate) hoisted_global_constants: &'block HoistedConstantsToBrilligGlobals,

pub(crate) building_globals: bool,
}
Expand All @@ -57,11 +60,17 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
block_id: BasicBlockId,
dfg: &DataFlowGraph,
globals: &'block HashMap<ValueId, BrilligVariable>,
hoisted_global_constants: &'block HoistedConstantsToBrilligGlobals,
) {
let live_in = function_context.liveness.get_live_in(&block_id);

let mut live_in_no_globals = HashSet::default();
for value in live_in {
if let Value::NumericConstant { constant, typ } = dfg[*value] {
if hoisted_global_constants.contains_key(&(constant, typ)) {
continue;
}
}
if !dfg.is_global(*value) {
live_in_no_globals.insert(*value);
}
Expand All @@ -85,6 +94,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
variables,
last_uses,
globals,
hoisted_global_constants,
building_globals: false,
};

Expand All @@ -95,7 +105,8 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
&mut self,
globals: &DataFlowGraph,
used_globals: &HashSet<ValueId>,
) {
hoisted_global_constants: &BTreeSet<(FieldElement, NumericType)>,
) -> HashMap<(FieldElement, NumericType), BrilligVariable> {
for (id, value) in globals.values_iter() {
if !used_globals.contains(&id) {
continue;
Expand All @@ -114,6 +125,16 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
}
}
}

let mut new_hoisted_constants = HashMap::default();
for (constant, typ) in hoisted_global_constants.iter().copied() {
let new_variable = allocate_value_with_type(self.brillig_context, Type::Numeric(typ));
self.brillig_context.const_instruction(new_variable.extract_single_addr(), constant);
if new_hoisted_constants.insert((constant, typ), new_variable).is_some() {
unreachable!("ICE: ({constant:?}, {typ:?}) was already in cache");
}
}
new_hoisted_constants
}

fn convert_block(&mut self, dfg: &DataFlowGraph) {
Expand Down Expand Up @@ -746,7 +767,10 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {

let index_variable = self.convert_ssa_single_addr_value(*index, dfg);

if !dfg.is_safe_index(*index, *array) {
// Slice access checks are generated separately against the slice's dynamic length field.
if matches!(dfg.type_of_value(*array), Type::Array(..))
&& !dfg.is_safe_index(*index, *array)
{
self.validate_array_index(array_variable, index_variable);
}

Expand Down Expand Up @@ -774,7 +798,10 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
dfg,
);

if !dfg.is_safe_index(*index, *array) {
// Slice access checks are generated separately against the slice's dynamic length field.
if matches!(dfg.type_of_value(*array), Type::Array(..))
&& !dfg.is_safe_index(*index, *array)
{
self.validate_array_index(source_variable, index_register);
}

Expand Down Expand Up @@ -950,7 +977,8 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {

for dead_variable in dead_variables {
// Globals are reserved throughout the entirety of the program
if !dfg.is_global(*dead_variable) {
let not_hoisted_global = self.get_hoisted_global(dfg, *dead_variable).is_none();
if !dfg.is_global(*dead_variable) && not_hoisted_global {
self.variables.remove_variable(
dead_variable,
self.function_context,
Expand Down Expand Up @@ -1668,6 +1696,10 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
let value_id = dfg.resolve(value_id);
let value = &dfg[value_id];

if let Some(variable) = self.get_hoisted_global(dfg, value_id) {
return variable;
}

match value {
Value::Global(_) => {
unreachable!("Expected global value to be resolve to its inner value");
Expand Down Expand Up @@ -2008,6 +2040,19 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
}
}
}

fn get_hoisted_global(
&self,
dfg: &DataFlowGraph,
value_id: ValueId,
) -> Option<BrilligVariable> {
if let Value::NumericConstant { constant, typ } = &dfg[value_id] {
if let Some(variable) = self.hoisted_global_constants.get(&(*constant, *typ)) {
return Some(*variable);
}
}
None
}
}

/// Returns the type of the operation considering the types of the operands
Expand Down
Loading

0 comments on commit 48de313

Please sign in to comment.