forked from noir-lang/noir
-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: remove 'single use' intermediate variables (noir-lang#6268)
# Description ## Problem\* Resolves noir-lang#6085 ## Summary\* This PR tries to benefit from Barretenberg's 'big-add gates' support, which was enabled by PR AztecProtocol/aztec-packages#8960 It's a simple optimisation which removes intermediate variables usually created by the CSatTransformer if they are not re-used elsewhere. The PR assumes that the backend is able to handle infinite width, but still requires the CSatTransformer, which is not really consistent. I plan to make follow-up PRs to get rid of this (but I can't guarantee it will work). ## Additional Context I tested the optimisation on all 'execution_sucess' test cases. In most cases, there were no change at all, while in some cases we would win one or two (10 max) on the circuit size. However, in a few cases, listed below in the form "test case: circuit size with 'intermediate var' optimisation vs no optimisation", it can be more significant: 7_function: 2955 vs 2992 bit_shifts_runtime: 5451 vs 5761 eddsa: 65805 vs 70406 hashmap: 135023 vs 150661 nested_array_dynamic: 12594 vs 12922 nested_array_in_slice: 5371 vs 5449 poseidon_bn254_hash: 1028 vs 1060 poseidon_bn254_hash_width_3: 1028 vs 1495 poseidonsponge_x5_254: 1244 vs 1307 regression_5252: 76491 vs 83862 sha256_var_size_regression: 74093 vs 74529 sha2_byte: 93998 vs 94006 slice_dynamic_index: 6308 vs 6419 slices: 3835 vs 3874 to_be_bytes: 135 vs 143 to_bytes_consistent: 6 vs 51 to_bytes_integration: 434 vs 484 u128: 4662 vs 4707 u16_support: 3023 vs 3057 ## 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 <[email protected]>
- Loading branch information
1 parent
3925228
commit ec75e8e
Showing
4 changed files
with
230 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
202 changes: 202 additions & 0 deletions
202
acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,202 @@ | ||
use std::collections::{HashMap, HashSet}; | ||
|
||
use acir::{ | ||
circuit::{brillig::BrilligInputs, directives::Directive, opcodes::BlockId, Circuit, Opcode}, | ||
native_types::{Expression, Witness}, | ||
AcirField, | ||
}; | ||
|
||
pub(crate) struct MergeExpressionsOptimizer { | ||
resolved_blocks: HashMap<BlockId, HashSet<Witness>>, | ||
} | ||
|
||
impl MergeExpressionsOptimizer { | ||
pub(crate) fn new() -> Self { | ||
MergeExpressionsOptimizer { resolved_blocks: HashMap::new() } | ||
} | ||
/// This pass analyzes the circuit and identifies intermediate variables that are | ||
/// only used in two gates. It then merges the gate that produces the | ||
/// intermediate variable into the second one that uses it | ||
/// Note: This pass is only relevant for backends that can handle unlimited width | ||
pub(crate) fn eliminate_intermediate_variable<F: AcirField>( | ||
&mut self, | ||
circuit: &Circuit<F>, | ||
acir_opcode_positions: Vec<usize>, | ||
) -> (Vec<Opcode<F>>, Vec<usize>) { | ||
// Keep track, for each witness, of the gates that use it | ||
let circuit_inputs = circuit.circuit_arguments(); | ||
self.resolved_blocks = HashMap::new(); | ||
let mut used_witness: HashMap<Witness, HashSet<usize>> = HashMap::new(); | ||
for (i, opcode) in circuit.opcodes.iter().enumerate() { | ||
let witnesses = self.witness_inputs(opcode); | ||
if let Opcode::MemoryInit { block_id, .. } = opcode { | ||
self.resolved_blocks.insert(*block_id, witnesses.clone()); | ||
} | ||
for w in witnesses { | ||
// We do not simplify circuit inputs | ||
if !circuit_inputs.contains(&w) { | ||
used_witness.entry(w).or_default().insert(i); | ||
} | ||
} | ||
} | ||
|
||
let mut modified_gates: HashMap<usize, Opcode<F>> = HashMap::new(); | ||
let mut new_circuit = Vec::new(); | ||
let mut new_acir_opcode_positions = Vec::new(); | ||
// For each opcode, try to get a target opcode to merge with | ||
for (i, opcode) in circuit.opcodes.iter().enumerate() { | ||
if !matches!(opcode, Opcode::AssertZero(_)) { | ||
new_circuit.push(opcode.clone()); | ||
new_acir_opcode_positions.push(acir_opcode_positions[i]); | ||
continue; | ||
} | ||
let opcode = modified_gates.get(&i).unwrap_or(opcode).clone(); | ||
let mut to_keep = true; | ||
let input_witnesses = self.witness_inputs(&opcode); | ||
for w in input_witnesses.clone() { | ||
let empty_gates = HashSet::new(); | ||
let gates_using_w = used_witness.get(&w).unwrap_or(&empty_gates); | ||
// We only consider witness which are used in exactly two arithmetic gates | ||
if gates_using_w.len() == 2 { | ||
let gates_using_w: Vec<_> = gates_using_w.iter().collect(); | ||
let mut b = *gates_using_w[1]; | ||
if b == i { | ||
b = *gates_using_w[0]; | ||
} else { | ||
// sanity check | ||
assert!(i == *gates_using_w[0]); | ||
} | ||
let second_gate = modified_gates.get(&b).unwrap_or(&circuit.opcodes[b]).clone(); | ||
if let (Opcode::AssertZero(expr_define), Opcode::AssertZero(expr_use)) = | ||
(opcode.clone(), second_gate) | ||
{ | ||
if let Some(expr) = Self::merge(&expr_use, &expr_define, w) { | ||
// sanity check | ||
assert!(i < b); | ||
modified_gates.insert(b, Opcode::AssertZero(expr)); | ||
to_keep = false; | ||
// Update the 'used_witness' map to account for the merge. | ||
for w2 in Self::expr_wit(&expr_define) { | ||
if !circuit_inputs.contains(&w2) { | ||
let mut v = used_witness[&w2].clone(); | ||
v.insert(b); | ||
v.remove(&i); | ||
used_witness.insert(w2, v); | ||
} | ||
} | ||
// We need to stop here and continue with the next opcode | ||
// because the merge invalidate the current opcode | ||
break; | ||
} | ||
} | ||
} | ||
} | ||
|
||
if to_keep { | ||
if modified_gates.contains_key(&i) { | ||
new_circuit.push(modified_gates[&i].clone()); | ||
} else { | ||
new_circuit.push(opcode.clone()); | ||
} | ||
new_acir_opcode_positions.push(acir_opcode_positions[i]); | ||
} | ||
} | ||
(new_circuit, new_acir_opcode_positions) | ||
} | ||
|
||
fn expr_wit<F>(expr: &Expression<F>) -> HashSet<Witness> { | ||
let mut result = HashSet::new(); | ||
result.extend(expr.mul_terms.iter().flat_map(|i| vec![i.1, i.2])); | ||
result.extend(expr.linear_combinations.iter().map(|i| i.1)); | ||
result | ||
} | ||
|
||
fn brillig_input_wit<F>(&self, input: &BrilligInputs<F>) -> HashSet<Witness> { | ||
let mut result = HashSet::new(); | ||
match input { | ||
BrilligInputs::Single(expr) => { | ||
result.extend(Self::expr_wit(expr)); | ||
} | ||
BrilligInputs::Array(exprs) => { | ||
for expr in exprs { | ||
result.extend(Self::expr_wit(expr)); | ||
} | ||
} | ||
BrilligInputs::MemoryArray(block_id) => { | ||
let witnesses = self.resolved_blocks.get(block_id).expect("Unknown block id"); | ||
result.extend(witnesses); | ||
} | ||
} | ||
result | ||
} | ||
|
||
// Returns the input witnesses used by the opcode | ||
fn witness_inputs<F: AcirField>(&self, opcode: &Opcode<F>) -> HashSet<Witness> { | ||
let mut witnesses = HashSet::new(); | ||
match opcode { | ||
Opcode::AssertZero(expr) => Self::expr_wit(expr), | ||
Opcode::BlackBoxFuncCall(bb_func) => bb_func.get_input_witnesses(), | ||
Opcode::Directive(Directive::ToLeRadix { a, .. }) => Self::expr_wit(a), | ||
Opcode::MemoryOp { block_id: _, op, predicate } => { | ||
//index et value, et predicate | ||
let mut witnesses = HashSet::new(); | ||
witnesses.extend(Self::expr_wit(&op.index)); | ||
witnesses.extend(Self::expr_wit(&op.value)); | ||
if let Some(p) = predicate { | ||
witnesses.extend(Self::expr_wit(p)); | ||
} | ||
witnesses | ||
} | ||
|
||
Opcode::MemoryInit { block_id: _, init, block_type: _ } => { | ||
init.iter().cloned().collect() | ||
} | ||
Opcode::BrilligCall { inputs, .. } => { | ||
for i in inputs { | ||
witnesses.extend(self.brillig_input_wit(i)); | ||
} | ||
witnesses | ||
} | ||
Opcode::Call { id: _, inputs, outputs: _, predicate } => { | ||
for i in inputs { | ||
witnesses.insert(*i); | ||
} | ||
if let Some(p) = predicate { | ||
witnesses.extend(Self::expr_wit(p)); | ||
} | ||
witnesses | ||
} | ||
} | ||
} | ||
|
||
// Merge 'expr' into 'target' via Gaussian elimination on 'w' | ||
// Returns None if the expressions cannot be merged | ||
fn merge<F: AcirField>( | ||
target: &Expression<F>, | ||
expr: &Expression<F>, | ||
w: Witness, | ||
) -> Option<Expression<F>> { | ||
// Check that the witness is not part of multiplication terms | ||
for m in &target.mul_terms { | ||
if m.1 == w || m.2 == w { | ||
return None; | ||
} | ||
} | ||
for m in &expr.mul_terms { | ||
if m.1 == w || m.2 == w { | ||
return None; | ||
} | ||
} | ||
|
||
for k in &target.linear_combinations { | ||
if k.1 == w { | ||
for i in &expr.linear_combinations { | ||
if i.1 == w { | ||
return Some(target.add_mul(-(k.0 / i.0), expr)); | ||
} | ||
} | ||
} | ||
} | ||
None | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters