Skip to content

Commit

Permalink
fix: Make nargo::ops::transform_program idempotent (#6695)
Browse files Browse the repository at this point in the history
  • Loading branch information
aakoshh authored Dec 5, 2024
1 parent 73e11c6 commit 82b38bd
Show file tree
Hide file tree
Showing 6 changed files with 366 additions and 18 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion acvm-repo/acvm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ workspace = true
thiserror.workspace = true
tracing.workspace = true
serde.workspace = true

fxhash.workspace = true
acir.workspace = true
brillig_vm.workspace = true
acvm_blackbox_solver.workspace = true
Expand Down
41 changes: 35 additions & 6 deletions acvm-repo/acvm/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ pub use simulator::CircuitSimulator;
use transformers::transform_internal;
pub use transformers::{transform, MIN_EXPRESSION_WIDTH};

/// We need multiple passes to stabilize the output.
/// The value was determined by running tests.
const MAX_OPTIMIZER_PASSES: usize = 3;

/// This module moves and decomposes acir opcodes. The transformation map allows consumers of this module to map
/// metadata they had about the opcodes to the new opcode structure generated after the transformation.
#[derive(Debug)]
Expand All @@ -28,9 +32,9 @@ impl AcirTransformationMap {
/// Builds a map from a vector of pointers to the old acir opcodes.
/// The index of the vector is the new opcode index.
/// The value of the vector is the old opcode index pointed.
fn new(acir_opcode_positions: Vec<usize>) -> Self {
fn new(acir_opcode_positions: &[usize]) -> Self {
let mut old_indices_to_new_indices = HashMap::with_capacity(acir_opcode_positions.len());
for (new_index, old_index) in acir_opcode_positions.into_iter().enumerate() {
for (new_index, old_index) in acir_opcode_positions.iter().copied().enumerate() {
old_indices_to_new_indices.entry(old_index).or_insert_with(Vec::new).push(new_index);
}
AcirTransformationMap { old_indices_to_new_indices }
Expand Down Expand Up @@ -72,17 +76,42 @@ fn transform_assert_messages<F: Clone>(
}

/// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] specific optimizations to a [`Circuit`].
///
/// Runs multiple passes until the output stabilizes.
pub fn compile<F: AcirField>(
acir: Circuit<F>,
expression_width: ExpressionWidth,
) -> (Circuit<F>, AcirTransformationMap) {
let (acir, acir_opcode_positions) = optimize_internal(acir);
let mut pass = 0;
let mut prev_opcodes_hash = fxhash::hash64(&acir.opcodes);
let mut prev_acir = acir;

// For most test programs it would be enough to only loop `transform_internal`,
// but some of them don't stabilize unless we also repeat the backend agnostic optimizations.
let (mut acir, acir_opcode_positions) = loop {
let (acir, acir_opcode_positions) = optimize_internal(prev_acir);

// Stop if we have already done at least one transform and an extra optimization changed nothing.
if pass > 0 && prev_opcodes_hash == fxhash::hash64(&acir.opcodes) {
break (acir, acir_opcode_positions);
}

let (mut acir, acir_opcode_positions) =
transform_internal(acir, expression_width, acir_opcode_positions);
let (acir, acir_opcode_positions) =
transform_internal(acir, expression_width, acir_opcode_positions);

let opcodes_hash = fxhash::hash64(&acir.opcodes);

// Stop if the output hasn't change in this loop or we went too long.
if pass == MAX_OPTIMIZER_PASSES - 1 || prev_opcodes_hash == opcodes_hash {
break (acir, acir_opcode_positions);
}

let transformation_map = AcirTransformationMap::new(acir_opcode_positions);
pass += 1;
prev_acir = acir;
prev_opcodes_hash = opcodes_hash;
};

let transformation_map = AcirTransformationMap::new(&acir_opcode_positions);
acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map);

(acir, transformation_map)
Expand Down
2 changes: 1 addition & 1 deletion acvm-repo/acvm/src/compiler/optimizers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use super::{transform_assert_messages, AcirTransformationMap};
pub fn optimize<F: AcirField>(acir: Circuit<F>) -> (Circuit<F>, AcirTransformationMap) {
let (mut acir, new_opcode_positions) = optimize_internal(acir);

let transformation_map = AcirTransformationMap::new(new_opcode_positions);
let transformation_map = AcirTransformationMap::new(&new_opcode_positions);

acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map);

Expand Down
Loading

0 comments on commit 82b38bd

Please sign in to comment.