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(test): Check that nargo::ops::transform_program is idempotent #6694

Merged
merged 15 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading