Skip to content

Commit

Permalink
switch from foreign to builtin
Browse files Browse the repository at this point in the history
  • Loading branch information
sirasistant committed Oct 25, 2024
1 parent 8968d7d commit 6238efe
Show file tree
Hide file tree
Showing 17 changed files with 94 additions and 149 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,6 @@ 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 @@ -221,7 +218,6 @@ impl BlackBoxFunc {
BlackBoxFunc::BigIntToLeBytes => "bigint_to_le_bytes",
BlackBoxFunc::Poseidon2Permutation => "poseidon2_permutation",
BlackBoxFunc::Sha256Compression => "sha256_compression",
BlackBoxFunc::FieldLessThan => "field_less_than",
}
}

Expand All @@ -248,7 +244,6 @@ 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,7 +34,6 @@ 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 @@ -84,7 +83,4 @@ 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: 0 additions & 11 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,15 +74,4 @@ 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 @@ -417,19 +417,6 @@ 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 @@ -604,7 +604,31 @@ impl<'block> BrilligBlock<'block> {
// `Intrinsic::AsWitness` is used to provide hints to acir-gen on optimal expression splitting.
// It is then useless in the brillig runtime and so we can ignore it
Value::Intrinsic(Intrinsic::AsWitness) => (),
Value::Intrinsic(Intrinsic::FieldLessThan) => {
let lhs = self.convert_ssa_single_addr_value(arguments[0], dfg);
assert!(lhs.bit_size == FieldElement::max_num_bits());
let rhs = self.convert_ssa_single_addr_value(arguments[1], dfg);
assert!(rhs.bit_size == FieldElement::max_num_bits());

let results = dfg.instruction_results(instruction_id);
let destination = self
.variables
.define_variable(
self.function_context,
self.brillig_context,
results[0],
dfg,
)
.extract_single_addr();
assert!(destination.bit_size == 1);

self.brillig_context.binary_instruction(
lhs,
rhs,
destination,
BrilligBinaryOp::LessThan,
);
}
_ => {
unreachable!("unsupported function call type {:?}", dfg[*func])
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,6 @@ 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,26 +38,6 @@ 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,7 +343,6 @@ 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 @@ -667,8 +666,6 @@ 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 @@ -717,8 +714,6 @@ 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 @@ -2789,6 +2789,9 @@ impl<'a> Context<'a> {
Intrinsic::DerivePedersenGenerators => {
unreachable!("DerivePedersenGenerators can only be called with constants")
}
Intrinsic::FieldLessThan => {
unreachable!("FieldLessThan can only be called in unconstrained")
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ impl Context {
| Intrinsic::StaticAssert
| Intrinsic::StrAsBytes
| Intrinsic::ToBits(..)
| Intrinsic::ToRadix(..) => {
| Intrinsic::ToRadix(..)
| Intrinsic::FieldLessThan => {
self.value_sets.push(instruction_arguments_and_results);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ pub(crate) enum Intrinsic {
AsWitness,
IsUnconstrained,
DerivePedersenGenerators,
FieldLessThan,
}

impl std::fmt::Display for Intrinsic {
Expand Down Expand Up @@ -100,6 +101,7 @@ impl std::fmt::Display for Intrinsic {
Intrinsic::AsWitness => write!(f, "as_witness"),
Intrinsic::IsUnconstrained => write!(f, "is_unconstrained"),
Intrinsic::DerivePedersenGenerators => write!(f, "derive_pedersen_generators"),
Intrinsic::FieldLessThan => write!(f, "field_less_than"),
}
}
}
Expand Down Expand Up @@ -131,7 +133,8 @@ impl Intrinsic {
| Intrinsic::FromField
| Intrinsic::AsField
| Intrinsic::IsUnconstrained
| Intrinsic::DerivePedersenGenerators => false,
| Intrinsic::DerivePedersenGenerators
| Intrinsic::FieldLessThan => false,

// Some black box functions have side-effects
Intrinsic::BlackBox(func) => matches!(
Expand Down Expand Up @@ -169,6 +172,8 @@ impl Intrinsic {
"as_witness" => Some(Intrinsic::AsWitness),
"is_unconstrained" => Some(Intrinsic::IsUnconstrained),
"derive_pedersen_generators" => Some(Intrinsic::DerivePedersenGenerators),
"field_less_than" => Some(Intrinsic::FieldLessThan),

other => BlackBoxFunc::lookup(other).map(Intrinsic::BlackBox),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,16 @@ pub(super) fn simplify_call(
unreachable!("Derive Pedersen Generators must return an array");
}
}
Intrinsic::FieldLessThan => {
if let Some(constants) = constant_args {
let lhs = constants[0];
let rhs = constants[1];
let result = dfg.make_constant((lhs < rhs).into(), Type::bool());
SimplifyResult::SimplifiedTo(result)
} else {
SimplifyResult::None
}
}
}
}

Expand Down Expand Up @@ -572,7 +582,6 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ impl Context {
| Intrinsic::AsSlice
| Intrinsic::AsWitness
| Intrinsic::IsUnconstrained
| Intrinsic::DerivePedersenGenerators => false,
| Intrinsic::DerivePedersenGenerators
| Intrinsic::FieldLessThan => false,
},

// We must assume that functions contain a side effect as we cannot inspect more deeply.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ fn slice_capacity_change(
| Intrinsic::IsUnconstrained
| Intrinsic::DerivePedersenGenerators
| Intrinsic::ToBits(_)
| Intrinsic::ToRadix(_) => SizeChange::None,
| Intrinsic::ToRadix(_)
| Intrinsic::FieldLessThan => SizeChange::None,
}
}
Loading

0 comments on commit 6238efe

Please sign in to comment.