Skip to content

Commit 85fa592

Browse files
vezenovmTomAFrench
andauthored
feat(acir_gen): Width aware ACIR gen addition (#5493)
# Description ## Problem\* Resolves #4629 ## Summary\* This PR updates how we add vars in ACIR gen to account for an expression width. This is under a compile time flag `--bounded-codegen`. If the sum of two expressions is going to go over the specified expression width we automatically create witnesses for either the lhs, the rhs, or both, before then generating a sum expression. This new bounded codegen could provide an easy way for developers to tell whether their program can be optimized using `as_witness`. Reference the additional context section below for some numbers. ## Additional Context There are some limitations in this approach as it is pretty naive in how it decides to when to generate a new witness. It doesn't look ahead at all other than for whether the two AcirVar's being added are going to create an expression over the specified ACIR width. For example we can see the following gate counts for `poseidonsponge_x5_254`: ``` No width awareness: +-----------------------+----------+----------------------+--------------+ | Package | Function | Expression Width | ACIR Opcodes | +-----------------------+----------+----------------------+--------------+ | poseidonsponge_x5_254 | main | Bounded { width: 4 } | 3096 | +-----------------------+----------+----------------------+--------------+ No width awareness w/ as_witness (this is the currently optimized poseidon we have in the stdlib): +-----------------------+----------+----------------------+--------------+ | Package | Function | Expression Width | ACIR Opcodes | +-----------------------+----------+----------------------+--------------+ | poseidonsponge_x5_254 | main | Bounded { width: 4 } | 1302 | +-----------------------+----------+----------------------+--------------+ Width awareness: +-----------------------+----------+----------------------+--------------+ | Package | Function | Expression Width | ACIR Opcodes | +-----------------------+----------+----------------------+--------------+ | poseidonsponge_x5_254 | main | Bounded { width: 4 } | 2114 | +-----------------------+----------+----------------------+--------------+ Width awareness w/ as_witness: +-----------------------+----------+----------------------+--------------+ | Package | Function | Expression Width | ACIR Opcodes | +-----------------------+----------+----------------------+--------------+ | poseidonsponge_x5_254 | main | Bounded { width: 4 } | 1792 | +-----------------------+----------+----------------------+--------------+ ``` From the above we can see that we actually have a degradation when using the addition strategy used in this PR with a hand optimized program using `as_witness`. Although this PR still gives an improvement in the default. Another example is the following program: ```rust fn main(x: Field, y: pub Field) { let state = [x, y]; let state = oh_no_not_again(state); // This assert will fail if we execute assert(state[0] + state[1] == 0); } fn oh_no_not_again(mut state: [Field; 2]) -> [Field; 2] { for _ in 0..200 { state[0] = state[0] * state[0] + state[1]; state[1] += state[0]; } state } ``` Without any width awareness we get 1150 ACIR gates. With this PR we will get 399 gates. If we substitute `oh_no_not_again` for the following: ```rust fn oh_no_not_again_as_witness(mut state: [Field; 2]) -> [Field; 2] { for i in 0..200 { state[0] = state[0] * state[0] + state[1]; std::as_witness(state[0]); state[1] += state[0]; if (i & 1 == 1) { std::as_witness(state[1]); } } state } ``` We will get 301 gates if the method above is called instead of `oh_no_not_again`. ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** 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. --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
1 parent cec6390 commit 85fa592

File tree

7 files changed

+201
-103
lines changed

7 files changed

+201
-103
lines changed

acvm-repo/acir/src/native_types/expression/mod.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,60 @@ impl<F: AcirField> Expression<F> {
273273

274274
Expression { mul_terms, linear_combinations, q_c }
275275
}
276+
277+
/// Determine the width of this expression.
278+
/// The width meaning the number of unique witnesses needed for this expression.
279+
pub fn width(&self) -> usize {
280+
let mut width = 0;
281+
282+
for mul_term in &self.mul_terms {
283+
// The coefficient should be non-zero, as this method is ran after the compiler removes all zero coefficient terms
284+
assert_ne!(mul_term.0, F::zero());
285+
286+
let mut found_x = false;
287+
let mut found_y = false;
288+
289+
for term in self.linear_combinations.iter() {
290+
let witness = &term.1;
291+
let x = &mul_term.1;
292+
let y = &mul_term.2;
293+
if witness == x {
294+
found_x = true;
295+
};
296+
if witness == y {
297+
found_y = true;
298+
};
299+
if found_x & found_y {
300+
break;
301+
}
302+
}
303+
304+
// If the multiplication is a squaring then we must assign the two witnesses to separate wires and so we
305+
// can never get a zero contribution to the width.
306+
let multiplication_is_squaring = mul_term.1 == mul_term.2;
307+
308+
let mul_term_width_contribution = if !multiplication_is_squaring && (found_x & found_y)
309+
{
310+
// Both witnesses involved in the multiplication exist elsewhere in the expression.
311+
// They both do not contribute to the width of the expression as this would be double-counting
312+
// due to their appearance in the linear terms.
313+
0
314+
} else if found_x || found_y {
315+
// One of the witnesses involved in the multiplication exists elsewhere in the expression.
316+
// The multiplication then only contributes 1 new witness to the width.
317+
1
318+
} else {
319+
// Worst case scenario, the multiplication is using completely unique witnesses so has a contribution of 2.
320+
2
321+
};
322+
323+
width += mul_term_width_contribution;
324+
}
325+
326+
width += self.linear_combinations.len();
327+
328+
width
329+
}
276330
}
277331

278332
impl<F: AcirField> From<F> for Expression<F> {

acvm-repo/acvm/src/compiler/transformers/csat.rs

Lines changed: 1 addition & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -415,71 +415,8 @@ fn fits_in_one_identity<F: AcirField>(expr: &Expression<F>, width: usize) -> boo
415415
if expr.mul_terms.len() > 1 {
416416
return false;
417417
};
418-
// A Polynomial with more terms than fan-in cannot fit within a single opcode
419-
if expr.linear_combinations.len() > width {
420-
return false;
421-
}
422-
423-
// A polynomial with no mul term and a fan-in that fits inside of the width can fit into a single opcode
424-
if expr.mul_terms.is_empty() {
425-
return true;
426-
}
427-
428-
// A polynomial with width-2 fan-in terms and a single non-zero mul term can fit into one opcode
429-
// Example: Axy + Dz . Notice, that the mul term places a constraint on the first two terms, but not the last term
430-
// XXX: This would change if our arithmetic polynomial equation was changed to Axyz for example, but for now it is not.
431-
if expr.linear_combinations.len() <= (width - 2) {
432-
return true;
433-
}
434-
435-
// 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
436-
// 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
437-
// An example of this is: Axy + Bx + Cy + ...
438-
// Notice how the bivariate monomial xy has two univariate monomials with their respective coefficients
439-
// XXX: note that if x or y is zero, then we could apply a further optimization, but this would be done in another algorithm.
440-
// It would be the same as when we have zero coefficients - Can only work if wire is constrained to be zero publicly
441-
let mul_term = &expr.mul_terms[0];
442-
443-
// The coefficient should be non-zero, as this method is ran after the compiler removes all zero coefficient terms
444-
assert_ne!(mul_term.0, F::zero());
445-
446-
let mut found_x = false;
447-
let mut found_y = false;
448-
449-
for term in expr.linear_combinations.iter() {
450-
let witness = &term.1;
451-
let x = &mul_term.1;
452-
let y = &mul_term.2;
453-
if witness == x {
454-
found_x = true;
455-
};
456-
if witness == y {
457-
found_y = true;
458-
};
459-
if found_x & found_y {
460-
break;
461-
}
462-
}
463-
464-
// If the multiplication is a squaring then we must assign the two witnesses to separate wires and so we
465-
// can never get a zero contribution to the width.
466-
let multiplication_is_squaring = mul_term.1 == mul_term.2;
467-
468-
let mul_term_width_contribution = if !multiplication_is_squaring && (found_x & found_y) {
469-
// Both witnesses involved in the multiplication exist elsewhere in the expression.
470-
// They both do not contribute to the width of the expression as this would be double-counting
471-
// due to their appearance in the linear terms.
472-
0
473-
} else if found_x || found_y {
474-
// One of the witnesses involved in the multiplication exists elsewhere in the expression.
475-
// The multiplication then only contributes 1 new witness to the width.
476-
1
477-
} else {
478-
// Worst case scenario, the multiplication is using completely unique witnesses so has a contribution of 2.
479-
2
480-
};
481418

482-
mul_term_width_contribution + expr.linear_combinations.len() <= width
419+
expr.width() <= width
483420
}
484421

485422
#[cfg(test)]

compiler/noirc_driver/src/lib.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ pub struct CompileOptions {
5656
#[arg(long, value_parser = parse_expression_width)]
5757
pub expression_width: Option<ExpressionWidth>,
5858

59+
/// Generate ACIR with the target backend expression width.
60+
/// The default is to generate ACIR without a bound and split expressions after code generation.
61+
/// Activating this flag can sometimes provide optimizations for certain programs.
62+
#[arg(long, default_value = "false")]
63+
pub bounded_codegen: bool,
64+
5965
/// Force a full recompilation.
6066
#[arg(long = "force")]
6167
pub force_compile: bool,
@@ -512,6 +518,12 @@ fn compile_contract_inner(
512518
}
513519
}
514520

521+
/// Default expression width used for Noir compilation.
522+
/// The ACVM native type `ExpressionWidth` has its own default which should always be unbounded,
523+
/// while we can sometimes expect the compilation target width to change.
524+
/// Thus, we set it separately here rather than trying to alter the default derivation of the type.
525+
pub const DEFAULT_EXPRESSION_WIDTH: ExpressionWidth = ExpressionWidth::Bounded { width: 4 };
526+
515527
/// Compile the current crate using `main_function` as the entrypoint.
516528
///
517529
/// This function assumes [`check_crate`] is called beforehand.
@@ -550,6 +562,11 @@ pub fn compile_no_check(
550562
enable_brillig_logging: options.show_brillig,
551563
force_brillig_output: options.force_brillig,
552564
print_codegen_timings: options.benchmark_codegen,
565+
expression_width: if options.bounded_codegen {
566+
options.expression_width.unwrap_or(DEFAULT_EXPRESSION_WIDTH)
567+
} else {
568+
ExpressionWidth::default()
569+
},
553570
};
554571

555572
let SsaProgramArtifact { program, debug, warnings, names, error_types, .. } =

compiler/noirc_evaluator/src/ssa.rs

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,22 @@ pub mod ir;
4242
mod opt;
4343
pub mod ssa_gen;
4444

45+
pub struct SsaEvaluatorOptions {
46+
/// Emit debug information for the intermediate SSA IR
47+
pub enable_ssa_logging: bool,
48+
49+
pub enable_brillig_logging: bool,
50+
51+
/// Force Brillig output (for step debugging)
52+
pub force_brillig_output: bool,
53+
54+
/// Pretty print benchmark times of each code generation pass
55+
pub print_codegen_timings: bool,
56+
57+
/// Width of expressions to be used for ACIR
58+
pub expression_width: ExpressionWidth,
59+
}
60+
4561
pub(crate) struct ArtifactsAndWarnings(Artifacts, Vec<SsaReport>);
4662

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

100116
drop(ssa_gen_span_guard);
101117

102-
let artifacts = time("SSA to ACIR", options.print_codegen_timings, || ssa.into_acir(&brillig))?;
118+
let artifacts = time("SSA to ACIR", options.print_codegen_timings, || {
119+
ssa.into_acir(&brillig, options.expression_width)
120+
})?;
103121
Ok(ArtifactsAndWarnings(artifacts, ssa_level_warnings))
104122
}
105123

@@ -160,19 +178,6 @@ impl SsaProgramArtifact {
160178
}
161179
}
162180

163-
pub struct SsaEvaluatorOptions {
164-
/// Emit debug information for the intermediate SSA IR
165-
pub enable_ssa_logging: bool,
166-
167-
pub enable_brillig_logging: bool,
168-
169-
/// Force Brillig output (for step debugging)
170-
pub force_brillig_output: bool,
171-
172-
/// Pretty print benchmark times of each code generation pass
173-
pub print_codegen_timings: bool,
174-
}
175-
176181
/// Compiles the [`Program`] into [`ACIR``][acvm::acir::circuit::Program].
177182
///
178183
/// The output ACIR is backend-agnostic and so must go through a transformation pass before usage in proof generation.

compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

Lines changed: 81 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::ssa::ir::types::Type as SsaType;
99
use crate::ssa::ir::{instruction::Endian, types::NumericType};
1010
use acvm::acir::circuit::brillig::{BrilligInputs, BrilligOutputs};
1111
use acvm::acir::circuit::opcodes::{BlockId, BlockType, MemOp};
12-
use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, Opcode};
12+
use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, ExpressionWidth, Opcode};
1313
use acvm::blackbox_solver;
1414
use acvm::brillig_vm::{MemoryValue, VMStatus, VM};
1515
use acvm::{
@@ -24,6 +24,7 @@ use acvm::{
2424
use fxhash::FxHashMap as HashMap;
2525
use iter_extended::{try_vecmap, vecmap};
2626
use num_bigint::BigUint;
27+
use std::cmp::Ordering;
2728
use std::{borrow::Cow, hash::Hash};
2829

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

125126
/// The BigIntContext, used to generate identifiers for BigIntegers
126127
big_int_ctx: BigIntContext,
128+
129+
expression_width: ExpressionWidth,
127130
}
128131

129132
impl<F: AcirField> AcirContext<F> {
133+
pub(crate) fn set_expression_width(&mut self, expression_width: ExpressionWidth) {
134+
self.expression_width = expression_width;
135+
}
136+
130137
pub(crate) fn current_witness_index(&self) -> Witness {
131138
self.acir_ir.current_witness_index()
132139
}
@@ -584,6 +591,7 @@ impl<F: AcirField> AcirContext<F> {
584591
pub(crate) fn mul_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result<AcirVar, RuntimeError> {
585592
let lhs_data = self.vars[&lhs].clone();
586593
let rhs_data = self.vars[&rhs].clone();
594+
587595
let result = match (lhs_data, rhs_data) {
588596
// (x * 1) == (1 * x) == x
589597
(AcirVarData::Const(constant), _) if constant.is_one() => rhs,
@@ -655,6 +663,7 @@ impl<F: AcirField> AcirContext<F> {
655663
self.mul_var(lhs, rhs)?
656664
}
657665
};
666+
658667
Ok(result)
659668
}
660669

@@ -670,9 +679,62 @@ impl<F: AcirField> AcirContext<F> {
670679
pub(crate) fn add_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result<AcirVar, RuntimeError> {
671680
let lhs_expr = self.var_to_expression(lhs)?;
672681
let rhs_expr = self.var_to_expression(rhs)?;
682+
673683
let sum_expr = &lhs_expr + &rhs_expr;
684+
if fits_in_one_identity(&sum_expr, self.expression_width) {
685+
let sum_var = self.add_data(AcirVarData::from(sum_expr));
686+
687+
return Ok(sum_var);
688+
}
689+
690+
let sum_expr = match lhs_expr.width().cmp(&rhs_expr.width()) {
691+
Ordering::Greater => {
692+
let lhs_witness_var = self.get_or_create_witness_var(lhs)?;
693+
let lhs_witness_expr = self.var_to_expression(lhs_witness_var)?;
694+
695+
let new_sum_expr = &lhs_witness_expr + &rhs_expr;
696+
if fits_in_one_identity(&new_sum_expr, self.expression_width) {
697+
new_sum_expr
698+
} else {
699+
let rhs_witness_var = self.get_or_create_witness_var(rhs)?;
700+
let rhs_witness_expr = self.var_to_expression(rhs_witness_var)?;
701+
702+
&lhs_expr + &rhs_witness_expr
703+
}
704+
}
705+
Ordering::Less => {
706+
let rhs_witness_var = self.get_or_create_witness_var(rhs)?;
707+
let rhs_witness_expr = self.var_to_expression(rhs_witness_var)?;
708+
709+
let new_sum_expr = &lhs_expr + &rhs_witness_expr;
710+
if fits_in_one_identity(&new_sum_expr, self.expression_width) {
711+
new_sum_expr
712+
} else {
713+
let lhs_witness_var = self.get_or_create_witness_var(lhs)?;
714+
let lhs_witness_expr = self.var_to_expression(lhs_witness_var)?;
674715

675-
Ok(self.add_data(AcirVarData::from(sum_expr)))
716+
&lhs_witness_expr + &rhs_expr
717+
}
718+
}
719+
Ordering::Equal => {
720+
let lhs_witness_var = self.get_or_create_witness_var(lhs)?;
721+
let lhs_witness_expr = self.var_to_expression(lhs_witness_var)?;
722+
723+
let new_sum_expr = &lhs_witness_expr + &rhs_expr;
724+
if fits_in_one_identity(&new_sum_expr, self.expression_width) {
725+
new_sum_expr
726+
} else {
727+
let rhs_witness_var = self.get_or_create_witness_var(rhs)?;
728+
let rhs_witness_expr = self.var_to_expression(rhs_witness_var)?;
729+
730+
&lhs_witness_expr + &rhs_witness_expr
731+
}
732+
}
733+
};
734+
735+
let sum_var = self.add_data(AcirVarData::from(sum_expr));
736+
737+
Ok(sum_var)
676738
}
677739

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

2055+
/// Checks if this expression can fit into one arithmetic identity
2056+
fn fits_in_one_identity<F: AcirField>(expr: &Expression<F>, width: ExpressionWidth) -> bool {
2057+
let width = match &width {
2058+
ExpressionWidth::Unbounded => {
2059+
return true;
2060+
}
2061+
ExpressionWidth::Bounded { width } => *width,
2062+
};
2063+
2064+
// A Polynomial with more than one mul term cannot fit into one opcode
2065+
if expr.mul_terms.len() > 1 {
2066+
return false;
2067+
};
2068+
2069+
expr.width() <= width
2070+
}
2071+
19932072
/// A Reference to an `AcirVarData`
19942073
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
19952074
pub(crate) struct AcirVar(usize);

0 commit comments

Comments
 (0)