Skip to content

Commit

Permalink
chore: use Brillig opcode when possible for less-than operations on f…
Browse files Browse the repository at this point in the history
…ields
  • Loading branch information
dbanks12 committed Oct 24, 2024
1 parent 3a26a38 commit 4b43bd8
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ pub enum BlackBoxFunc {
/// - state: [(witness, 32); 8]
/// - output: [(witness, 32); 8]
Sha256Compression,

// Less than comparison between two field elements (only usable in unconstrained)
FieldLessThan,
}

impl std::fmt::Display for BlackBoxFunc {
Expand Down Expand Up @@ -218,6 +221,7 @@ impl BlackBoxFunc {
BlackBoxFunc::BigIntToLeBytes => "bigint_to_le_bytes",
BlackBoxFunc::Poseidon2Permutation => "poseidon2_permutation",
BlackBoxFunc::Sha256Compression => "sha256_compression",
BlackBoxFunc::FieldLessThan => "field_less_than",
}
}

Expand All @@ -244,6 +248,7 @@ impl BlackBoxFunc {
"bigint_to_le_bytes" => Some(BlackBoxFunc::BigIntToLeBytes),
"poseidon2_permutation" => Some(BlackBoxFunc::Poseidon2Permutation),
"sha256_compression" => Some(BlackBoxFunc::Sha256Compression),
"field_less_than" => Some(BlackBoxFunc::FieldLessThan),
_ => None,
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub trait BlackBoxFunctionSolver<F> {
_inputs: &[F],
_len: u32,
) -> Result<Vec<F>, BlackBoxResolutionError>;
fn field_less_than(&self, _input_x: &F, _input_y: &F) -> Result<bool, BlackBoxResolutionError>;
}

pub struct StubbedBlackBoxSolver;
Expand Down Expand Up @@ -83,4 +84,7 @@ impl<F> BlackBoxFunctionSolver<F> for StubbedBlackBoxSolver {
) -> Result<Vec<F>, BlackBoxResolutionError> {
Err(Self::fail(BlackBoxFunc::Poseidon2Permutation))
}
fn field_less_than(&self, _input_x: &F, _input_y: &F) -> Result<bool, BlackBoxResolutionError> {
Err(Self::fail(BlackBoxFunc::FieldLessThan))
}
}
11 changes: 11 additions & 0 deletions noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,15 @@ impl BlackBoxFunctionSolver<FieldElement> for Bn254BlackBoxSolver {
) -> Result<Vec<FieldElement>, BlackBoxResolutionError> {
poseidon2_permutation(inputs, len)
}

fn field_less_than(
&self,
_input_x: &FieldElement,
_input_y: &FieldElement,
) -> Result<bool, BlackBoxResolutionError> {
Err(BlackBoxResolutionError::Failed(
acir::BlackBoxFunc::FieldLessThan,
"field_less_than is not supported in acir, only in brillig/unconstrained".to_string(),
))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,19 @@ pub(crate) fn convert_black_box_call<F: AcirField + DebugToString, Registers: Re
unreachable!("ICE: AES128Encrypt expects three array arguments, one array result")
}
}
BlackBoxFunc::FieldLessThan => {
if let (
[BrilligVariable::SingleAddr(input_x), BrilligVariable::SingleAddr(input_y)],
[BrilligVariable::SingleAddr(output)],
) = (function_arguments, function_results)
{
brillig_context.field_less_than_instruction(*input_x, *input_y, *output);
} else {
unreachable!(
"ICE: FieldLessThan expects two register arguments one register result"
)
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,14 @@ pub(crate) mod tests {
) -> Result<Vec<FieldElement>, BlackBoxResolutionError> {
Ok(vec![0_u128.into(), 1_u128.into(), 2_u128.into(), 3_u128.into()])
}

fn field_less_than(
&self,
_input_x: &FieldElement,
_input_y: &FieldElement,
) -> Result<bool, BlackBoxResolutionError> {
Ok(true)
}
}

pub(crate) fn create_context(id: FunctionId) -> BrilligContext<FieldElement, Stack> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,26 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
self.binary(lhs, rhs, result, operation);
}

/// Procresses a field less than instruction.
///
/// Just performs an assertion on bit size and then forwards to binary_instruction().
pub(crate) fn field_less_than_instruction(
&mut self,
lhs: SingleAddrVariable,
rhs: SingleAddrVariable,
result: SingleAddrVariable,
) {
let max_field_size = FieldElement::max_num_bits();
assert!(
lhs.bit_size == max_field_size && rhs.bit_size == max_field_size,
"Expected bit sizes lhs and rhs to be {}, got {} and {} for 'field less than' operation",
lhs.bit_size,
rhs.bit_size,
max_field_size,
);
self.binary_instruction(lhs, rhs, result, BrilligBinaryOp::LessThan);
}

/// Processes a not instruction.
///
/// Not is computed using a subtraction operation as there is no native not instruction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ impl<F: AcirField> GeneratedAcir<F> {
.expect("Compiler should generate correct size inputs"),
outputs: outputs.try_into().expect("Compiler should generate correct size outputs"),
},
BlackBoxFunc::FieldLessThan => panic!("FieldLessThan is not supported in ACIR"),
};

self.push_opcode(AcirOpcode::BlackBoxFuncCall(black_box_func_call));
Expand Down Expand Up @@ -666,6 +667,8 @@ fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Option<usize> {

// FromLeBytes takes a variable array of bytes as input
BlackBoxFunc::BigIntFromLeBytes => None,

BlackBoxFunc::FieldLessThan => panic!("FieldLessThan is not supported in ACIR"),
}
}

Expand Down Expand Up @@ -714,6 +717,8 @@ fn black_box_expected_output_size(name: BlackBoxFunc) -> Option<usize> {

// AES encryption returns a variable number of outputs
BlackBoxFunc::AES128Encrypt => None,

BlackBoxFunc::FieldLessThan => panic!("FieldLessThan is not supported in ACIR"),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@ fn simplify_black_box_func(
}
BlackBoxFunc::Sha256Compression => SimplifyResult::None, //TODO(Guillaume)
BlackBoxFunc::AES128Encrypt => SimplifyResult::None,
BlackBoxFunc::FieldLessThan => panic!("FieldLessThan is not supported in ACIR"),
}
}

Expand Down
34 changes: 21 additions & 13 deletions noir/noir-repo/noir_stdlib/src/field/bn254.nr
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::runtime::is_unconstrained;
use crate::field::bn254::field_less_than;

// The low and high decomposition of the field modulus
global PLO: Field = 53438638232309528389504892708671455233;
Expand All @@ -25,23 +26,30 @@ pub(crate) unconstrained fn decompose_hint(x: Field) -> (Field, Field) {
compute_decomposition(x)
}

#[foreign(field_less_than)]
pub fn field_less_than(_x: Field, _y: Field) -> bool {}

fn compute_lt(x: Field, y: Field, num_bytes: u32) -> bool {
let x_bytes: [u8; 32] = x.to_le_bytes();
let y_bytes: [u8; 32] = y.to_le_bytes();
let mut x_is_lt = false;
let mut done = false;
for i in 0..num_bytes {
if (!done) {
let x_byte = x_bytes[num_bytes - 1 - i];
let y_byte = y_bytes[num_bytes - 1 - i];
let bytes_match = x_byte == y_byte;
if !bytes_match {
x_is_lt = x_byte < y_byte;
done = true;
if is_unconstrained() {
field_less_than(x, y)
} else {
let x_bytes: [u8; 32] = x.to_le_bytes();
let y_bytes: [u8; 32] = y.to_le_bytes();
let mut x_is_lt = false;
let mut done = false;
for i in 0..num_bytes {
if (!done) {
let x_byte = x_bytes[num_bytes - 1 - i];
let y_byte = y_bytes[num_bytes - 1 - i];
let bytes_match = x_byte == y_byte;
if !bytes_match {
x_is_lt = x_byte < y_byte;
done = true;
}
}
}
x_is_lt
}
x_is_lt
}

fn compute_lte(x: Field, y: Field, num_bytes: u32) -> bool {
Expand Down
8 changes: 8 additions & 0 deletions noir/noir-repo/tooling/lsp/src/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,12 @@ impl BlackBoxFunctionSolver<acvm::FieldElement> for WrapperSolver {
) -> Result<Vec<acvm::FieldElement>, acvm::BlackBoxResolutionError> {
self.0.poseidon2_permutation(inputs, len)
}

fn field_less_than(
&self,
_input_x: &acvm::FieldElement,
_input_y: &acvm::FieldElement,
) -> Result<bool, acvm::BlackBoxResolutionError> {
self.0.field_less_than(_input_x, _input_y)
}
}

0 comments on commit 4b43bd8

Please sign in to comment.