Skip to content

Commit

Permalink
Merge branch 'master' into tf/clean-up-else-conditions
Browse files Browse the repository at this point in the history
* master:
  feat: simplify constant MSM calls in SSA (#6547)
  chore(test): Remove duplicate brillig tests (#6523)
  chore: restructure `noirc_evaluator` crate (#6534)
  • Loading branch information
TomAFrench committed Nov 19, 2024
2 parents 97d7167 + f291e37 commit f473d1b
Show file tree
Hide file tree
Showing 51 changed files with 168 additions and 809 deletions.
1 change: 0 additions & 1 deletion acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ pub fn multi_scalar_mul(
scalars_hi: &[FieldElement],
) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> {
if points.len() != 3 * scalars_lo.len() || scalars_lo.len() != scalars_hi.len() {
dbg!(&points.len(), &scalars_lo.len(), &scalars_hi.len());
return Err(BlackBoxResolutionError::Failed(
BlackBoxFunc::MultiScalarMul,
"Points and scalars must have the same length".to_string(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,34 +1,35 @@
use super::big_int::BigIntContext;
use super::generated_acir::{BrilligStdlibFunc, GeneratedAcir, PLACEHOLDER_BRILLIG_INDEX};
use crate::brillig::brillig_gen::brillig_directive;
use crate::brillig::brillig_ir::artifact::GeneratedBrillig;
use crate::errors::{InternalBug, InternalError, RuntimeError, SsaReport};
use crate::ssa::acir_gen::{AcirDynamicArray, AcirValue};
use crate::ssa::ir::dfg::CallStack;
use crate::ssa::ir::types::Type as SsaType;
use crate::ssa::ir::{instruction::Endian, types::NumericType};
use acvm::acir::circuit::brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs};
use acvm::acir::circuit::opcodes::{
AcirFunctionId, BlockId, BlockType, ConstantOrWitnessEnum, MemOp,
};
use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, ExpressionWidth, Opcode};
use acvm::brillig_vm::{MemoryValue, VMStatus, VM};
use acvm::BlackBoxFunctionSolver;
use acvm::{
acir::AcirField,
acir::{
brillig::Opcode as BrilligOpcode,
circuit::opcodes::FunctionInput,
circuit::{
brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs},
opcodes::{
AcirFunctionId, BlockId, BlockType, ConstantOrWitnessEnum, FunctionInput, MemOp,
},
AssertionPayload, ExpressionOrMemory, ExpressionWidth, Opcode,
},
native_types::{Expression, Witness},
BlackBoxFunc,
AcirField, BlackBoxFunc,
},
brillig_vm::{MemoryValue, VMStatus, VM},
BlackBoxFunctionSolver,
};
use fxhash::FxHashMap as HashMap;
use iter_extended::{try_vecmap, vecmap};
use num_bigint::BigUint;
use std::cmp::Ordering;
use std::{borrow::Cow, hash::Hash};

use crate::brillig::brillig_ir::artifact::GeneratedBrillig;
use crate::errors::{InternalBug, InternalError, RuntimeError, SsaReport};
use crate::ssa::ir::{
dfg::CallStack, instruction::Endian, types::NumericType, types::Type as SsaType,
};

use super::big_int::BigIntContext;
use super::generated_acir::{BrilligStdlibFunc, GeneratedAcir, PLACEHOLDER_BRILLIG_INDEX};
use super::{brillig_directive, AcirDynamicArray, AcirValue};

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
/// High level Type descriptor for Variables.
///
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
//! `GeneratedAcir` is constructed as part of the `acir_gen` pass to accumulate all of the ACIR
//! program as it is being converted from SSA form.
use std::{collections::BTreeMap, u32};
use std::collections::BTreeMap;

use crate::{
brillig::{brillig_gen::brillig_directive, brillig_ir::artifact::GeneratedBrillig},
errors::{InternalError, RuntimeError, SsaReport},
ssa::ir::dfg::CallStack,
};
use acvm::acir::{
circuit::{
brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs},
Expand All @@ -21,6 +16,13 @@ use acvm::{
acir::{circuit::directives::Directive, native_types::Expression},
};

use super::brillig_directive;
use crate::{
brillig::brillig_ir::artifact::GeneratedBrillig,
errors::{InternalError, RuntimeError, SsaReport},
ssa::ir::dfg::CallStack,
};

use iter_extended::vecmap;
use noirc_errors::debug_info::ProcedureDebugId;
use num_bigint::BigUint;
Expand Down Expand Up @@ -153,7 +155,7 @@ impl<F: AcirField> GeneratedAcir<F> {
/// This means you cannot multiply an infinite amount of `Expression`s together.
/// Once the `Expression` goes over degree-2, then it needs to be reduced to a `Witness`
/// which has degree-1 in order to be able to continue the multiplication chain.
pub(crate) fn create_witness_for_expression(&mut self, expression: &Expression<F>) -> Witness {
fn create_witness_for_expression(&mut self, expression: &Expression<F>) -> Witness {
let fresh_witness = self.next_witness_index();

// Create a constraint that sets them to be equal to each other
Expand Down
Original file line number Diff line number Diff line change
@@ -1,46 +1,57 @@
//! This file holds the pass to convert from Noir's SSA IR to ACIR.
mod acir_ir;
use fxhash::FxHashMap as HashMap;
use im::Vector;
use std::collections::{BTreeMap, HashSet};
use std::fmt::Debug;

use self::acir_ir::acir_variable::{AcirContext, AcirType, AcirVar};
use self::acir_ir::generated_acir::BrilligStdlibFunc;
use super::function_builder::data_bus::DataBus;
use super::ir::dfg::CallStack;
use super::ir::function::FunctionId;
use super::ir::instruction::{ConstrainError, ErrorType};
use super::ir::printer::try_to_extract_string_from_error_payload;
use super::{
use acvm::acir::{
circuit::{
brillig::{BrilligBytecode, BrilligFunctionId},
opcodes::{AcirFunctionId, BlockType},
AssertionPayload, ErrorSelector, ExpressionWidth, OpcodeLocation,
},
native_types::Witness,
BlackBoxFunc,
};
use acvm::{acir::circuit::opcodes::BlockId, acir::AcirField, FieldElement};
use bn254_blackbox_solver::Bn254BlackBoxSolver;
use iter_extended::{try_vecmap, vecmap};
use noirc_frontend::monomorphization::ast::InlineType;

mod acir_variable;
mod big_int;
mod brillig_directive;
mod generated_acir;

use crate::brillig::{
brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext,
brillig_ir::{
artifact::{BrilligParameter, GeneratedBrillig},
BrilligContext,
},
Brillig,
};
use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
use crate::ssa::{
function_builder::data_bus::DataBus,
ir::{
dfg::DataFlowGraph,
function::{Function, RuntimeType},
dfg::{CallStack, DataFlowGraph},
function::{Function, FunctionId, RuntimeType},
instruction::{
Binary, BinaryOp, Instruction, InstructionId, Intrinsic, TerminatorInstruction,
Binary, BinaryOp, ConstrainError, ErrorType, Instruction, InstructionId, Intrinsic,
TerminatorInstruction,
},
map::Id,
printer::try_to_extract_string_from_error_payload,
types::{NumericType, Type},
value::{Value, ValueId},
},
ssa_gen::Ssa,
};
use crate::brillig::brillig_ir::artifact::{BrilligParameter, GeneratedBrillig};
use crate::brillig::brillig_ir::BrilligContext;
use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig};
use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
pub(crate) use acir_ir::generated_acir::GeneratedAcir;
use acvm::acir::circuit::opcodes::{AcirFunctionId, BlockType};
use bn254_blackbox_solver::Bn254BlackBoxSolver;
use noirc_frontend::monomorphization::ast::InlineType;

use acvm::acir::circuit::brillig::{BrilligBytecode, BrilligFunctionId};
use acvm::acir::circuit::{AssertionPayload, ErrorSelector, ExpressionWidth, OpcodeLocation};
use acvm::acir::native_types::Witness;
use acvm::acir::BlackBoxFunc;
use acvm::{acir::circuit::opcodes::BlockId, acir::AcirField, FieldElement};
use fxhash::FxHashMap as HashMap;
use im::Vector;
use iter_extended::{try_vecmap, vecmap};
use acir_variable::{AcirContext, AcirType, AcirVar};
use generated_acir::BrilligStdlibFunc;
pub(crate) use generated_acir::GeneratedAcir;

#[derive(Default)]
struct SharedContext<F> {
Expand Down Expand Up @@ -2823,22 +2834,6 @@ impl<'a> Context<'a> {
Ok(())
}

/// Given an array value, return the numerical type of its element.
/// Panics if the given value is not an array or has a non-numeric element type.
fn array_element_type(dfg: &DataFlowGraph, value: ValueId) -> AcirType {
match dfg.type_of_value(value) {
Type::Array(elements, _) => {
assert_eq!(elements.len(), 1);
(&elements[0]).into()
}
Type::Slice(elements) => {
assert_eq!(elements.len(), 1);
(&elements[0]).into()
}
_ => unreachable!("Expected array type"),
}
}

/// Convert a Vec<AcirVar> into a Vec<AcirValue> using the given result ids.
/// If the type of a result id is an array, several acir vars are collected into
/// a single AcirValue::Array of the same length.
Expand Down Expand Up @@ -2929,9 +2924,9 @@ mod test {
use std::collections::BTreeMap;

use crate::{
acir::BrilligStdlibFunc,
brillig::Brillig,
ssa::{
acir_gen::acir_ir::generated_acir::BrilligStdlibFunc,
function_builder::FunctionBuilder,
ir::{function::FunctionId, instruction::BinaryOp, map::Id, types::Type},
},
Expand Down
1 change: 0 additions & 1 deletion compiler/noirc_evaluator/src/brillig/brillig_gen.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
pub(crate) mod brillig_black_box;
pub(crate) mod brillig_block;
pub(crate) mod brillig_block_variables;
pub(crate) mod brillig_directive;
pub(crate) mod brillig_fn;
pub(crate) mod brillig_slice_ops;
mod constant_allocation;
Expand Down
6 changes: 2 additions & 4 deletions compiler/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@

pub mod errors;

// SSA code to create the SSA based IR
// for functions and execute different optimizations.
pub mod ssa;

mod acir;
pub mod brillig;
pub mod ssa;

pub use ssa::create_program;

Expand Down
9 changes: 3 additions & 6 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,16 @@ use acvm::{

use noirc_errors::debug_info::{DebugFunctions, DebugInfo, DebugTypes, DebugVariables};

use noirc_frontend::ast::Visibility;
use noirc_frontend::{
ast::Visibility,
hir_def::{function::FunctionSignature, types::Type as HirType},
monomorphization::ast::Program,
};
use ssa_gen::Ssa;
use tracing::{span, Level};

use self::{
acir_gen::{Artifacts, GeneratedAcir},
ssa_gen::Ssa,
};
use crate::acir::{Artifacts, GeneratedAcir};

mod acir_gen;
mod checks;
pub(super) mod function_builder;
pub mod ir;
Expand Down
3 changes: 0 additions & 3 deletions compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir.rs

This file was deleted.

4 changes: 3 additions & 1 deletion compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,9 @@ fn simplify_black_box_func(
acvm::blackbox_solver::ecdsa_secp256r1_verify,
),

BlackBoxFunc::MultiScalarMul => SimplifyResult::None,
BlackBoxFunc::MultiScalarMul => {
blackbox::simplify_msm(dfg, solver, arguments, block, call_stack)
}
BlackBoxFunc::EmbeddedCurveAdd => {
blackbox::simplify_ec_add(dfg, solver, arguments, block, call_stack)
}
Expand Down
58 changes: 58 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,64 @@ pub(super) fn simplify_ec_add(
}
}

pub(super) fn simplify_msm(
dfg: &mut DataFlowGraph,
solver: impl BlackBoxFunctionSolver<FieldElement>,
arguments: &[ValueId],
block: BasicBlockId,
call_stack: &CallStack,
) -> SimplifyResult {
// TODO: Handle MSMs where a subset of the terms are constant.
match (dfg.get_array_constant(arguments[0]), dfg.get_array_constant(arguments[1])) {
(Some((points, _)), Some((scalars, _))) => {
let Some(points) = points
.into_iter()
.map(|id| dfg.get_numeric_constant(id))
.collect::<Option<Vec<_>>>()
else {
return SimplifyResult::None;
};

let Some(scalars) = scalars
.into_iter()
.map(|id| dfg.get_numeric_constant(id))
.collect::<Option<Vec<_>>>()
else {
return SimplifyResult::None;
};

let mut scalars_lo = Vec::new();
let mut scalars_hi = Vec::new();
for (i, scalar) in scalars.into_iter().enumerate() {
if i % 2 == 0 {
scalars_lo.push(scalar);
} else {
scalars_hi.push(scalar);
}
}

let Ok((result_x, result_y, result_is_infinity)) =
solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi)
else {
return SimplifyResult::None;
};

let result_x = dfg.make_constant(result_x, Type::field());
let result_y = dfg.make_constant(result_y, Type::field());
let result_is_infinity = dfg.make_constant(result_is_infinity, Type::bool());

let elements = im::vector![result_x, result_y, result_is_infinity];
let typ = Type::Array(Arc::new(vec![Type::field()]), 3);
let instruction = Instruction::MakeArray { elements, typ };
let result_array =
dfg.insert_instruction_and_results(instruction, block, None, call_stack.clone());

SimplifyResult::SimplifiedTo(result_array.first())
}
_ => SimplifyResult::None,
}
}

pub(super) fn simplify_poseidon2_permutation(
dfg: &mut DataFlowGraph,
solver: impl BlackBoxFunctionSolver<FieldElement>,
Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[package]
name = "brillig_assert"
name = "embedded_curve_msm_simplification"
type = "bin"
authors = [""]

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
fn main() {
let pub_x = 0x0000000000000000000000000000000000000000000000000000000000000001;
let pub_y = 0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c;

let g1_y = 17631683881184975370165255887551781615748388533673675138860;
let g1 = std::embedded_curve_ops::EmbeddedCurvePoint { x: 1, y: g1_y, is_infinite: false };
let scalar = std::embedded_curve_ops::EmbeddedCurveScalar { lo: 1, hi: 0 };
// Test that multi_scalar_mul correctly derives the public key
let res = std::embedded_curve_ops::multi_scalar_mul([g1], [scalar]);
assert(res.x == pub_x);
assert(res.y == pub_y);
}
7 changes: 7 additions & 0 deletions test_programs/execution_success/assert/src/main.nr
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
fn main(x: Field) {
assert(x == 1);
assert(1 == conditional(x as bool));
}

fn conditional(x: bool) -> Field {
assert(x, f"Expected x to be true but got {x}");
assert_eq(x, true, f"Expected x to be true but got {x}");
1
}

This file was deleted.

This file was deleted.

Loading

0 comments on commit f473d1b

Please sign in to comment.