diff --git a/acvm-repo/acir/src/native_types/expression/mod.rs b/acvm-repo/acir/src/native_types/expression/mod.rs index 1feda5703c8..2bbbc39d0ca 100644 --- a/acvm-repo/acir/src/native_types/expression/mod.rs +++ b/acvm-repo/acir/src/native_types/expression/mod.rs @@ -273,6 +273,60 @@ impl Expression { 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 From for Expression { diff --git a/acvm-repo/acvm/src/compiler/transformers/csat.rs b/acvm-repo/acvm/src/compiler/transformers/csat.rs index 19cc18ca7f3..f258e0a8818 100644 --- a/acvm-repo/acvm/src/compiler/transformers/csat.rs +++ b/acvm-repo/acvm/src/compiler/transformers/csat.rs @@ -415,71 +415,8 @@ fn fits_in_one_identity(expr: &Expression, width: usize) -> boo 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)] diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index dd774a1eeec..f430eb8ad19 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -56,6 +56,12 @@ pub struct CompileOptions { #[arg(long, value_parser = parse_expression_width)] pub expression_width: Option, + /// 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, @@ -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. @@ -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, .. } = diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 81327cec013..41dbf3b7272 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -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); /// Optimize the given program by converting it into SSA @@ -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)) } @@ -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. diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 74149af25ef..9e97fd3bc50 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -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::{ @@ -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)] @@ -124,9 +125,15 @@ pub(crate) struct AcirContext { /// The BigIntContext, used to generate identifiers for BigIntegers big_int_ctx: BigIntContext, + + expression_width: ExpressionWidth, } impl AcirContext { + 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() } @@ -584,6 +591,7 @@ impl AcirContext { pub(crate) fn mul_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { 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, @@ -655,6 +663,7 @@ impl AcirContext { self.mul_var(lhs, rhs)? } }; + Ok(result) } @@ -670,9 +679,62 @@ impl AcirContext { pub(crate) fn add_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { 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)?; + 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 @@ -1990,6 +2052,23 @@ impl From> for AcirVarData { } } +/// Checks if this expression can fit into one arithmetic identity +fn fits_in_one_identity(expr: &Expression, 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); diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 1bdc9aaf4eb..dc0e026b3d1 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -33,7 +33,7 @@ use acvm::acir::circuit::opcodes::BlockType; use noirc_frontend::monomorphization::ast::InlineType; use acvm::acir::circuit::brillig::BrilligBytecode; -use acvm::acir::circuit::{AssertionPayload, ErrorSelector, OpcodeLocation}; +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}; @@ -282,12 +282,16 @@ pub(crate) type Artifacts = ( impl Ssa { #[tracing::instrument(level = "trace", skip_all)] - pub(crate) fn into_acir(self, brillig: &Brillig) -> Result { + pub(crate) fn into_acir( + self, + brillig: &Brillig, + expression_width: ExpressionWidth, + ) -> Result { let mut acirs = Vec::new(); - // TODO: can we parallelise this? + // TODO: can we parallelize this? let mut shared_context = SharedContext::default(); for function in self.functions.values() { - let context = Context::new(&mut shared_context); + let context = Context::new(&mut shared_context, expression_width); if let Some(mut generated_acir) = context.convert_ssa_function(&self, function, brillig)? { @@ -334,8 +338,12 @@ impl Ssa { } impl<'a> Context<'a> { - fn new(shared_context: &'a mut SharedContext) -> Context<'a> { + fn new( + shared_context: &'a mut SharedContext, + expression_width: ExpressionWidth, + ) -> Context<'a> { let mut acir_context = AcirContext::default(); + acir_context.set_expression_width(expression_width); let current_side_effects_enabled_var = acir_context.add_constant(FieldElement::one()); Context { @@ -1288,6 +1296,7 @@ impl<'a> Context<'a> { index_side_effect = false; } } + // Fallback to multiplication if the index side_effects have not already been handled if index_side_effect { // Set the value to 0 if current_side_effects is 0, to ensure it fits in any value type @@ -2820,7 +2829,7 @@ mod test { use acvm::{ acir::{ - circuit::{Opcode, OpcodeLocation}, + circuit::{ExpressionWidth, Opcode, OpcodeLocation}, native_types::Witness, }, FieldElement, @@ -2917,7 +2926,7 @@ mod test { let ssa = builder.finish(); let (acir_functions, _, _) = ssa - .into_acir(&Brillig::default()) + .into_acir(&Brillig::default(), ExpressionWidth::default()) .expect("Should compile manually written SSA into ACIR"); // Expected result: // main f0 @@ -3012,7 +3021,7 @@ mod test { let ssa = builder.finish(); let (acir_functions, _, _) = ssa - .into_acir(&Brillig::default()) + .into_acir(&Brillig::default(), ExpressionWidth::default()) .expect("Should compile manually written SSA into ACIR"); // The expected result should look very similar to the above test expect that the input witnesses of the `Call` // opcodes will be different. The changes can discerned from the checks below. @@ -3102,7 +3111,7 @@ mod test { let ssa = builder.finish(); let (acir_functions, _, _) = ssa - .into_acir(&Brillig::default()) + .into_acir(&Brillig::default(), ExpressionWidth::default()) .expect("Should compile manually written SSA into ACIR"); assert_eq!(acir_functions.len(), 3, "Should have three ACIR functions"); @@ -3215,8 +3224,9 @@ mod test { let ssa = builder.finish(); let brillig = ssa.to_brillig(false); - let (acir_functions, brillig_functions, _) = - ssa.into_acir(&brillig).expect("Should compile manually written SSA into ACIR"); + let (acir_functions, brillig_functions, _) = ssa + .into_acir(&brillig, ExpressionWidth::default()) + .expect("Should compile manually written SSA into ACIR"); assert_eq!(acir_functions.len(), 1, "Should only have a `main` ACIR function"); assert_eq!(brillig_functions.len(), 2, "Should only have generated two Brillig functions"); @@ -3272,7 +3282,7 @@ mod test { // The Brillig bytecode we insert for the stdlib is hardcoded so we do not need to provide any // Brillig artifacts to the ACIR gen pass. let (acir_functions, brillig_functions, _) = ssa - .into_acir(&Brillig::default()) + .into_acir(&Brillig::default(), ExpressionWidth::default()) .expect("Should compile manually written SSA into ACIR"); assert_eq!(acir_functions.len(), 1, "Should only have a `main` ACIR function"); @@ -3343,8 +3353,9 @@ mod test { let brillig = ssa.to_brillig(false); println!("{}", ssa); - let (acir_functions, brillig_functions, _) = - ssa.into_acir(&brillig).expect("Should compile manually written SSA into ACIR"); + let (acir_functions, brillig_functions, _) = ssa + .into_acir(&brillig, ExpressionWidth::default()) + .expect("Should compile manually written SSA into ACIR"); assert_eq!(acir_functions.len(), 1, "Should only have a `main` ACIR function"); // We expect 3 brillig functions: @@ -3431,8 +3442,9 @@ mod test { let brillig = ssa.to_brillig(false); println!("{}", ssa); - let (acir_functions, brillig_functions, _) = - ssa.into_acir(&brillig).expect("Should compile manually written SSA into ACIR"); + let (acir_functions, brillig_functions, _) = ssa + .into_acir(&brillig, ExpressionWidth::default()) + .expect("Should compile manually written SSA into ACIR"); assert_eq!(acir_functions.len(), 2, "Should only have two ACIR functions"); // We expect 3 brillig functions: diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index a2877ebdeac..3e3560c91bf 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -9,8 +9,8 @@ use nargo::package::Package; use nargo::workspace::Workspace; use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; -use noirc_driver::file_manager_with_stdlib; use noirc_driver::NOIR_ARTIFACT_VERSION_STRING; +use noirc_driver::{file_manager_with_stdlib, DEFAULT_EXPRESSION_WIDTH}; use noirc_driver::{CompilationResult, CompileOptions, CompiledContract}; use noirc_frontend::graph::CrateName; @@ -250,12 +250,6 @@ fn save_contract( } } -/// 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. -const DEFAULT_EXPRESSION_WIDTH: ExpressionWidth = ExpressionWidth::Bounded { width: 4 }; - /// If a target width was not specified in the CLI we can safely override the default. pub(crate) fn get_target_width( package_default_width: Option,