Skip to content

Commit

Permalink
feat: Add FieldElement::from<usize> implementation (#3647)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

While reviewing #3617 it struck me how very verbose it is to create
constants in acirgen, if we want to represent an array index we need to
do every conversion from `usize -> u128 -> FieldElement -> AcirVar`.

This PR shortens this by having `add_constant` accept an `impl
Into<FieldElement>` as well as implementing `FieldElement::from<usize>`.
We can then feed in usizes directly to `add_constant`.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
TomAFrench authored Nov 30, 2023
1 parent 3680f52 commit 8b7c5aa
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 23 deletions.
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 @@ -143,6 +143,12 @@ impl<F: PrimeField> From<u128> for FieldElement<F> {
}
}

impl<F: PrimeField> From<usize> for FieldElement<F> {
fn from(a: usize) -> FieldElement<F> {
FieldElement::from(a as u128)
}
}

impl<F: PrimeField> From<bool> for FieldElement<F> {
fn from(boolean: bool) -> FieldElement<F> {
if boolean {
Expand Down
25 changes: 11 additions & 14 deletions compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ impl AcirContext {
}

/// Adds a constant to the context and assigns a Variable to represent it
pub(crate) fn add_constant(&mut self, constant: FieldElement) -> AcirVar {
let constant_data = AcirVarData::Const(constant);
pub(crate) fn add_constant(&mut self, constant: impl Into<FieldElement>) -> AcirVar {
let constant_data = AcirVarData::Const(constant.into());
self.add_data(constant_data)
}

Expand Down Expand Up @@ -353,7 +353,7 @@ impl AcirContext {

// Check to see if equality can be determined at compile-time.
if diff_expr.is_const() {
return Ok(self.add_constant(diff_expr.is_zero().into()));
return Ok(self.add_constant(diff_expr.is_zero()));
}

let is_equal_witness = self.acir_ir.is_equal(&lhs_expr, &rhs_expr);
Expand Down Expand Up @@ -410,7 +410,7 @@ impl AcirContext {
// max - ((max - a) AND (max -b))
// Subtracting from max flips the bits, so this is effectively:
// (NOT a) NAND (NOT b)
let max = self.add_constant(FieldElement::from((1_u128 << bit_size) - 1));
let max = self.add_constant((1_u128 << bit_size) - 1);
let a = self.sub_var(max, lhs)?;
let b = self.sub_var(max, rhs)?;
let inputs = vec![AcirValue::Var(a, typ.clone()), AcirValue::Var(b, typ)];
Expand Down Expand Up @@ -553,7 +553,7 @@ impl AcirContext {
pub(crate) fn not_var(&mut self, x: AcirVar, typ: AcirType) -> Result<AcirVar, RuntimeError> {
let bit_size = typ.bit_size();
// Subtracting from max flips the bits
let max = self.add_constant(FieldElement::from((1_u128 << bit_size) - 1));
let max = self.add_constant((1_u128 << bit_size) - 1);
self.sub_var(max, x)
}

Expand All @@ -580,8 +580,8 @@ impl AcirContext {
let quotient = lhs_const.to_u128() / rhs_const.to_u128();
let remainder = lhs_const.to_u128() - quotient * rhs_const.to_u128();

let quotient_var = self.add_constant(FieldElement::from(quotient));
let remainder_var = self.add_constant(FieldElement::from(remainder));
let quotient_var = self.add_constant(quotient);
let remainder_var = self.add_constant(remainder);
return Ok((quotient_var, remainder_var));
}

Expand Down Expand Up @@ -778,7 +778,7 @@ impl AcirContext {
// witness = lhs_offset + r
assert!(bits + r_bit_size < FieldElement::max_num_bits()); //we need to ensure lhs_offset + r does not overflow

let r_var = self.add_constant(r.into());
let r_var = self.add_constant(r);
let aor = self.add_var(lhs_offset, r_var)?;
// lhs_offset<=rhs_offset <=> lhs_offset + r < rhs_offset + r = 2^bit_size <=> witness < 2^bit_size
self.range_constrain_var(aor, &NumericType::Unsigned { bit_size }, None)?;
Expand Down Expand Up @@ -1150,10 +1150,7 @@ impl AcirContext {
// `Intrinsic::ToRadix` returns slices which are represented
// by tuples with the structure (length, slice contents)
Ok(vec![
AcirValue::Var(
self.add_constant(FieldElement::from(limb_vars.len() as u128)),
AcirType::field(),
),
AcirValue::Var(self.add_constant(limb_vars.len()), AcirType::field()),
AcirValue::Array(limb_vars.into()),
])
}
Expand All @@ -1166,7 +1163,7 @@ impl AcirContext {
limb_count_var: AcirVar,
result_element_type: AcirType,
) -> Result<Vec<AcirValue>, RuntimeError> {
let two_var = self.add_constant(FieldElement::from(2_u128));
let two_var = self.add_constant(2_u128);
self.radix_decompose(endian, input_var, two_var, limb_count_var, result_element_type)
}

Expand Down Expand Up @@ -1274,7 +1271,7 @@ impl AcirContext {
AcirValue::DynamicArray(AcirDynamicArray { block_id, len, .. }) => {
for i in 0..len {
// We generate witnesses corresponding to the array values
let index_var = self.add_constant(FieldElement::from(i as u128));
let index_var = self.add_constant(i);

let value_read_var = self.read_from_memory(block_id, &index_var)?;
let value_read = AcirValue::Var(value_read_var, AcirType::field());
Expand Down
17 changes: 8 additions & 9 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,7 @@ impl Context {

let mut read_dynamic_array_index =
|block_id: BlockId, array_index: usize| -> Result<AcirVar, InternalError> {
let index_var =
self.acir_context.add_constant(FieldElement::from(array_index as u128));
let index_var = self.acir_context.add_constant(array_index);

self.acir_context.read_from_memory(block_id, &index_var)
};
Expand Down Expand Up @@ -852,7 +851,7 @@ impl Context {
);

let values = try_vecmap(0..*len, |i| {
let index_var = self.acir_context.add_constant(FieldElement::from(i as u128));
let index_var = self.acir_context.add_constant(i);

let read = self.acir_context.read_from_memory(*block_id, &index_var)?;
Ok::<AcirValue, RuntimeError>(AcirValue::Var(read, AcirType::field()))
Expand Down Expand Up @@ -1076,7 +1075,7 @@ impl Context {
}
AcirValue::DynamicArray(AcirDynamicArray { block_id: inner_block_id, len, .. }) => {
let values = try_vecmap(0..len, |i| {
let index_var = self.acir_context.add_constant(FieldElement::from(i as u128));
let index_var = self.acir_context.add_constant(i);

let read = self.acir_context.read_from_memory(inner_block_id, &index_var)?;
Ok::<AcirValue, RuntimeError>(AcirValue::Var(read, AcirType::field()))
Expand Down Expand Up @@ -1235,7 +1234,7 @@ impl Context {

// The final array should will the flattened index at each outer array index
let init_values = vecmap(flat_elem_type_sizes, |type_size| {
let var = self.acir_context.add_constant(FieldElement::from(type_size as u128));
let var = self.acir_context.add_constant(type_size);
AcirValue::Var(var, AcirType::field())
});
let element_type_sizes_len = init_values.len();
Expand Down Expand Up @@ -1292,7 +1291,7 @@ impl Context {
array_len: usize,
) -> Result<(), RuntimeError> {
let init_values = try_vecmap(0..array_len, |i| {
let index_var = self.acir_context.add_constant(FieldElement::from(i as u128));
let index_var = self.acir_context.add_constant(i);

let read = self.acir_context.read_from_memory(source, &index_var)?;
Ok::<AcirValue, RuntimeError>(AcirValue::Var(read, AcirType::field()))
Expand Down Expand Up @@ -1760,8 +1759,8 @@ impl Context {
Intrinsic::ArrayLen => {
let len = match self.convert_value(arguments[0], dfg) {
AcirValue::Var(_, _) => unreachable!("Non-array passed to array.len() method"),
AcirValue::Array(values) => (values.len() as u128).into(),
AcirValue::DynamicArray(array) => (array.len as u128).into(),
AcirValue::Array(values) => values.len(),
AcirValue::DynamicArray(array) => array.len,
};
Ok(vec![AcirValue::Var(self.acir_context.add_constant(len), AcirType::field())])
}
Expand Down Expand Up @@ -1978,7 +1977,7 @@ impl Context {
AcirValue::DynamicArray(AcirDynamicArray { block_id, len, .. }) => {
for i in 0..len {
// We generate witnesses corresponding to the array values
let index_var = self.acir_context.add_constant(FieldElement::from(i as u128));
let index_var = self.acir_context.add_constant(i);

let value_read_var =
self.acir_context.read_from_memory(block_id, &index_var)?;
Expand Down

0 comments on commit 8b7c5aa

Please sign in to comment.