Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(acir_gen): Width aware ACIR gen addition #5493

Merged
merged 34 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
5ba8d43
initial testing program for expr width
vezenovm Jun 28, 2024
8726efe
improved width-aware addition, lots of debug stuff still that has to …
vezenovm Jul 8, 2024
b92780f
initial work with add var to remove exponential blow-up in addition l…
vezenovm Jul 11, 2024
fb58e10
test change'
vezenovm Jul 11, 2024
829d1f8
merge conflicts w/ master
vezenovm Jul 11, 2024
4f84446
expr width prove toml
vezenovm Jul 11, 2024
3f41198
remove dbg printlns
vezenovm Jul 11, 2024
96c16a9
make test expr_width pass execution
vezenovm Jul 11, 2024
84dc258
remove dbgs
vezenovm Jul 11, 2024
a15245e
switch back fits_in_one_idenitity inside csat transformer
vezenovm Jul 11, 2024
f636b42
bring back better csat fits_in_one_identity and enable compile option…
vezenovm Jul 11, 2024
4031fba
remove dbgs
vezenovm Jul 11, 2024
3efa35a
check mul terms
vezenovm Jul 11, 2024
89e621b
fmt
vezenovm Jul 11, 2024
00fab61
clkeanup
vezenovm Jul 12, 2024
1e9692a
remove test that does not test anything
vezenovm Jul 12, 2024
5483dc8
remove unnecessary bounds on add_var if
vezenovm Jul 12, 2024
e2bf2b3
cleanup add_var
vezenovm Jul 12, 2024
83f0485
Merge branch 'master' into mv/acir-gen-width
vezenovm Jul 12, 2024
73952f9
Merge branch 'master' into mv/acir-gen-width
vezenovm Jul 12, 2024
8d95543
Merge branch 'master' into mv/acir-gen-width
TomAFrench Jul 15, 2024
9653424
more granular add_var check
vezenovm Jul 18, 2024
d04be53
Merge remote-tracking branch 'origin/mv/acir-gen-width' into mv/acir-…
vezenovm Jul 18, 2024
aaf612c
remove unnecessary mul-terms len check in width()
vezenovm Jul 18, 2024
ef2c178
make unbounded codegen the default
vezenovm Jul 18, 2024
21993ca
Merge branch 'master' into mv/acir-gen-width
vezenovm Jul 18, 2024
719bd90
fix messed up remote diff
vezenovm Jul 18, 2024
88d9dd8
fixup
vezenovm Jul 18, 2024
170bc33
switch flag to bounded_codegen
vezenovm Jul 18, 2024
48ee000
add comment
vezenovm Jul 18, 2024
5479f6b
Merge branch 'master' into mv/acir-gen-width
vezenovm Jul 19, 2024
3a39117
Merge branch 'master' into mv/acir-gen-width
vezenovm Jul 22, 2024
ddb93b4
check that we do not need to create witness for rhs when widths are e…
vezenovm Jul 24, 2024
1a39274
Merge branch 'master' into mv/acir-gen-width
vezenovm Jul 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions acvm-repo/acir/src/native_types/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,60 @@

Expression { mul_terms, linear_combinations, q_c }
}

/// Determine the width of this expression.
/// The width meaning the number of unique witnesses needed for this expression.
pub fn width(&self) -> usize {
let mut width = 0;

for mul_term in &self.mul_terms {
// The coefficient should be non-zero, as this method is ran after the compiler removes all zero coefficient terms
assert_ne!(mul_term.0, F::zero());

let mut found_x = false;
let mut found_y = false;

for term in self.linear_combinations.iter() {
let witness = &term.1;
let x = &mul_term.1;
let y = &mul_term.2;
if witness == x {
found_x = true;
};
if witness == y {
found_y = true;
};
if found_x & found_y {
break;
}
}

// If the multiplication is a squaring then we must assign the two witnesses to separate wires and so we
// can never get a zero contribution to the width.
let multiplication_is_squaring = mul_term.1 == mul_term.2;

let mul_term_width_contribution = if !multiplication_is_squaring && (found_x & found_y)
{
// Both witnesses involved in the multiplication exist elsewhere in the expression.
// They both do not contribute to the width of the expression as this would be double-counting
// due to their appearance in the linear terms.
0
} else if found_x || found_y {
// One of the witnesses involved in the multiplication exists elsewhere in the expression.
// The multiplication then only contributes 1 new witness to the width.
1
} else {
// Worst case scenario, the multiplication is using completely unique witnesses so has a contribution of 2.
2
};

width += mul_term_width_contribution;
}

width += self.linear_combinations.len();

width
}
}

impl<F: AcirField> From<F> for Expression<F> {
Expand Down Expand Up @@ -302,7 +356,7 @@
use acir_field::{AcirField, FieldElement};

#[test]
fn add_mul_smoketest() {

Check warning on line 359 in acvm-repo/acir/src/native_types/expression/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (smoketest)
let a = Expression {
mul_terms: vec![(FieldElement::from(2u128), Witness(1), Witness(2))],
..Default::default()
Expand Down
65 changes: 1 addition & 64 deletions acvm-repo/acvm/src/compiler/transformers/csat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
// This optimization will search for combinations of terms which can be represented in a single assert-zero opcode
// Case 1 : qM * wL * wR + qL * wL + qR * wR + qO * wO + qC
// This polynomial does not require any further optimizations, it can be safely represented in one opcode
// ie a polynomial with 1 mul(bi-variate) term and 3 (univariate) terms where 2 of those terms match the bivariate term

Check warning on line 90 in acvm-repo/acvm/src/compiler/transformers/csat.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bivariate)
// wL and wR, we can represent it in one opcode
// GENERALIZED for WIDTH: instead of the number 3, we use `WIDTH`
//
Expand Down Expand Up @@ -415,71 +415,8 @@
if expr.mul_terms.len() > 1 {
return false;
};
// A Polynomial with more terms than fan-in cannot fit within a single opcode
if expr.linear_combinations.len() > width {
return false;
}

// A polynomial with no mul term and a fan-in that fits inside of the width can fit into a single opcode
if expr.mul_terms.is_empty() {
return true;
}

// A polynomial with width-2 fan-in terms and a single non-zero mul term can fit into one opcode
// Example: Axy + Dz . Notice, that the mul term places a constraint on the first two terms, but not the last term
// XXX: This would change if our arithmetic polynomial equation was changed to Axyz for example, but for now it is not.
if expr.linear_combinations.len() <= (width - 2) {
return true;
}

// We now know that we have a single mul term. We also know that the mul term must match up with at least one of the other terms
// A polynomial whose mul terms are non zero which do not match up with two terms in the fan-in cannot fit into one opcode
// An example of this is: Axy + Bx + Cy + ...
// Notice how the bivariate monomial xy has two univariate monomials with their respective coefficients
// XXX: note that if x or y is zero, then we could apply a further optimization, but this would be done in another algorithm.
// It would be the same as when we have zero coefficients - Can only work if wire is constrained to be zero publicly
let mul_term = &expr.mul_terms[0];

// The coefficient should be non-zero, as this method is ran after the compiler removes all zero coefficient terms
assert_ne!(mul_term.0, F::zero());

let mut found_x = false;
let mut found_y = false;

for term in expr.linear_combinations.iter() {
let witness = &term.1;
let x = &mul_term.1;
let y = &mul_term.2;
if witness == x {
found_x = true;
};
if witness == y {
found_y = true;
};
if found_x & found_y {
break;
}
}

// If the multiplication is a squaring then we must assign the two witnesses to separate wires and so we
// can never get a zero contribution to the width.
let multiplication_is_squaring = mul_term.1 == mul_term.2;

let mul_term_width_contribution = if !multiplication_is_squaring && (found_x & found_y) {
// Both witnesses involved in the multiplication exist elsewhere in the expression.
// They both do not contribute to the width of the expression as this would be double-counting
// due to their appearance in the linear terms.
0
} else if found_x || found_y {
// One of the witnesses involved in the multiplication exists elsewhere in the expression.
// The multiplication then only contributes 1 new witness to the width.
1
} else {
// Worst case scenario, the multiplication is using completely unique witnesses so has a contribution of 2.
2
};

mul_term_width_contribution + expr.linear_combinations.len() <= width
expr.width() <= width
}

#[cfg(test)]
Expand Down
17 changes: 17 additions & 0 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ pub struct CompileOptions {
#[arg(long, value_parser = parse_expression_width)]
pub expression_width: Option<ExpressionWidth>,

/// Generate ACIR with the target backend expression width.
/// The default is to generate ACIR without a bound and split expressions after code generation.
/// Activating this flag can sometimes provide optimizations for certain programs.
#[arg(long, default_value = "false")]
pub bounded_codegen: bool,

/// Force a full recompilation.
#[arg(long = "force")]
pub force_compile: bool,
Expand Down Expand Up @@ -512,6 +518,12 @@ fn compile_contract_inner(
}
}

/// Default expression width used for Noir compilation.
/// The ACVM native type `ExpressionWidth` has its own default which should always be unbounded,
/// while we can sometimes expect the compilation target width to change.
/// Thus, we set it separately here rather than trying to alter the default derivation of the type.
pub const DEFAULT_EXPRESSION_WIDTH: ExpressionWidth = ExpressionWidth::Bounded { width: 4 };

/// Compile the current crate using `main_function` as the entrypoint.
///
/// This function assumes [`check_crate`] is called beforehand.
Expand Down Expand Up @@ -550,6 +562,11 @@ pub fn compile_no_check(
enable_brillig_logging: options.show_brillig,
force_brillig_output: options.force_brillig,
print_codegen_timings: options.benchmark_codegen,
expression_width: if options.bounded_codegen {
options.expression_width.unwrap_or(DEFAULT_EXPRESSION_WIDTH)
} else {
ExpressionWidth::default()
},
};

let SsaProgramArtifact { program, debug, warnings, names, error_types, .. } =
Expand Down
33 changes: 19 additions & 14 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,22 @@ pub mod ir;
mod opt;
pub mod ssa_gen;

pub struct SsaEvaluatorOptions {
/// Emit debug information for the intermediate SSA IR
pub enable_ssa_logging: bool,

pub enable_brillig_logging: bool,

/// Force Brillig output (for step debugging)
pub force_brillig_output: bool,

/// Pretty print benchmark times of each code generation pass
pub print_codegen_timings: bool,

/// Width of expressions to be used for ACIR
pub expression_width: ExpressionWidth,
}

pub(crate) struct ArtifactsAndWarnings(Artifacts, Vec<SsaReport>);

/// Optimize the given program by converting it into SSA
Expand Down Expand Up @@ -99,7 +115,9 @@ pub(crate) fn optimize_into_acir(

drop(ssa_gen_span_guard);

let artifacts = time("SSA to ACIR", options.print_codegen_timings, || ssa.into_acir(&brillig))?;
let artifacts = time("SSA to ACIR", options.print_codegen_timings, || {
ssa.into_acir(&brillig, options.expression_width)
})?;
Ok(ArtifactsAndWarnings(artifacts, ssa_level_warnings))
}

Expand Down Expand Up @@ -160,19 +178,6 @@ impl SsaProgramArtifact {
}
}

pub struct SsaEvaluatorOptions {
/// Emit debug information for the intermediate SSA IR
pub enable_ssa_logging: bool,

pub enable_brillig_logging: bool,

/// Force Brillig output (for step debugging)
pub force_brillig_output: bool,

/// Pretty print benchmark times of each code generation pass
pub print_codegen_timings: bool,
}

/// Compiles the [`Program`] into [`ACIR``][acvm::acir::circuit::Program].
///
/// The output ACIR is backend-agnostic and so must go through a transformation pass before usage in proof generation.
Expand Down
83 changes: 81 additions & 2 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 @@ -9,7 +9,7 @@ use crate::ssa::ir::types::Type as SsaType;
use crate::ssa::ir::{instruction::Endian, types::NumericType};
use acvm::acir::circuit::brillig::{BrilligInputs, BrilligOutputs};
use acvm::acir::circuit::opcodes::{BlockId, BlockType, MemOp};
use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, Opcode};
use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, ExpressionWidth, Opcode};
use acvm::blackbox_solver;
use acvm::brillig_vm::{MemoryValue, VMStatus, VM};
use acvm::{
Expand All @@ -24,6 +24,7 @@ use acvm::{
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};

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -124,9 +125,15 @@ pub(crate) struct AcirContext<F: AcirField> {

/// The BigIntContext, used to generate identifiers for BigIntegers
big_int_ctx: BigIntContext,

expression_width: ExpressionWidth,
}

impl<F: AcirField> AcirContext<F> {
pub(crate) fn set_expression_width(&mut self, expression_width: ExpressionWidth) {
self.expression_width = expression_width;
}

pub(crate) fn current_witness_index(&self) -> Witness {
self.acir_ir.current_witness_index()
}
Expand Down Expand Up @@ -584,6 +591,7 @@ impl<F: AcirField> AcirContext<F> {
pub(crate) fn mul_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result<AcirVar, RuntimeError> {
let lhs_data = self.vars[&lhs].clone();
let rhs_data = self.vars[&rhs].clone();

let result = match (lhs_data, rhs_data) {
// (x * 1) == (1 * x) == x
(AcirVarData::Const(constant), _) if constant.is_one() => rhs,
Expand Down Expand Up @@ -655,6 +663,7 @@ impl<F: AcirField> AcirContext<F> {
self.mul_var(lhs, rhs)?
}
};

Ok(result)
}

Expand All @@ -670,9 +679,62 @@ impl<F: AcirField> AcirContext<F> {
pub(crate) fn add_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result<AcirVar, RuntimeError> {
let lhs_expr = self.var_to_expression(lhs)?;
let rhs_expr = self.var_to_expression(rhs)?;

let sum_expr = &lhs_expr + &rhs_expr;
if fits_in_one_identity(&sum_expr, self.expression_width) {
let sum_var = self.add_data(AcirVarData::from(sum_expr));

return Ok(sum_var);
}

let sum_expr = match lhs_expr.width().cmp(&rhs_expr.width()) {
Ordering::Greater => {
let lhs_witness_var = self.get_or_create_witness_var(lhs)?;
let lhs_witness_expr = self.var_to_expression(lhs_witness_var)?;

let new_sum_expr = &lhs_witness_expr + &rhs_expr;
if fits_in_one_identity(&new_sum_expr, self.expression_width) {
new_sum_expr
} else {
let rhs_witness_var = self.get_or_create_witness_var(rhs)?;
let rhs_witness_expr = self.var_to_expression(rhs_witness_var)?;

&lhs_expr + &rhs_witness_expr
}
}
Ordering::Less => {
let rhs_witness_var = self.get_or_create_witness_var(rhs)?;
let rhs_witness_expr = self.var_to_expression(rhs_witness_var)?;

let new_sum_expr = &lhs_expr + &rhs_witness_expr;
if fits_in_one_identity(&new_sum_expr, self.expression_width) {
new_sum_expr
} else {
let lhs_witness_var = self.get_or_create_witness_var(lhs)?;
let lhs_witness_expr = self.var_to_expression(lhs_witness_var)?;

Ok(self.add_data(AcirVarData::from(sum_expr)))
&lhs_witness_expr + &rhs_expr
}
}
Ordering::Equal => {
let lhs_witness_var = self.get_or_create_witness_var(lhs)?;
guipublic marked this conversation as resolved.
Show resolved Hide resolved
let lhs_witness_expr = self.var_to_expression(lhs_witness_var)?;

let new_sum_expr = &lhs_witness_expr + &rhs_expr;
if fits_in_one_identity(&new_sum_expr, self.expression_width) {
new_sum_expr
} else {
let rhs_witness_var = self.get_or_create_witness_var(rhs)?;
let rhs_witness_expr = self.var_to_expression(rhs_witness_var)?;

&lhs_witness_expr + &rhs_witness_expr
}
}
};

let sum_var = self.add_data(AcirVarData::from(sum_expr));

Ok(sum_var)
}

/// Adds a new Variable to context whose value will
Expand Down Expand Up @@ -1990,6 +2052,23 @@ impl<F: AcirField> From<Expression<F>> for AcirVarData<F> {
}
}

/// Checks if this expression can fit into one arithmetic identity
fn fits_in_one_identity<F: AcirField>(expr: &Expression<F>, width: ExpressionWidth) -> bool {
let width = match &width {
ExpressionWidth::Unbounded => {
return true;
}
ExpressionWidth::Bounded { width } => *width,
};

// A Polynomial with more than one mul term cannot fit into one opcode
if expr.mul_terms.len() > 1 {
return false;
};

expr.width() <= width
}

/// A Reference to an `AcirVarData`
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub(crate) struct AcirVar(usize);
Expand Down
Loading
Loading