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

fix: Do not merge expressions that contain output witnesses #6757

Merged
merged 3 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 5 additions & 0 deletions acvm-repo/acvm/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ pub fn compile<F: AcirField>(
acir: Circuit<F>,
expression_width: ExpressionWidth,
) -> (Circuit<F>, AcirTransformationMap) {
if MAX_OPTIMIZER_PASSES == 0 {
let acir_opcode_positions = (0..acir.opcodes.len()).collect::<Vec<_>>();
let transformation_map = AcirTransformationMap::new(&acir_opcode_positions);
return (acir, transformation_map);
}
let mut pass = 0;
let mut prev_opcodes_hash = fxhash::hash64(&acir.opcodes);
let mut prev_acir = acir;
Expand Down
10 changes: 6 additions & 4 deletions acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,18 @@ impl<F: AcirField> MergeExpressionsOptimizer<F> {
self.resolved_blocks.clear();

// Keep track, for each witness, of the gates that use it
let circuit_inputs = circuit.circuit_arguments();
let circuit_io: BTreeSet<Witness> =
circuit.circuit_arguments().union(&circuit.public_inputs().0).cloned().collect();

let mut used_witness: BTreeMap<Witness, BTreeSet<usize>> = BTreeMap::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) {
// We do not simplify circuit inputs and outputs
if !circuit_io.contains(&w) {
used_witness.entry(w).or_default().insert(i);
}
}
Expand Down Expand Up @@ -102,7 +104,7 @@ impl<F: AcirField> MergeExpressionsOptimizer<F> {
let mut witness_list = CircuitSimulator::expr_wit(&expr_use);
witness_list.extend(CircuitSimulator::expr_wit(&expr_define));
for w2 in witness_list {
if !circuit_inputs.contains(&w2) {
if !circuit_io.contains(&w2) {
used_witness.entry(w2).and_modify(|v| {
v.insert(target);
v.remove(&source);
Expand Down
1 change: 0 additions & 1 deletion acvm-repo/acvm/src/pwg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
RequiresForeignCall(ForeignCallWaitInfo<F>),

/// The ACVM has encountered a request for an ACIR [call][acir::circuit::Opcode]
/// to execute a separate ACVM instance. The result of the ACIR call must be passd back to the ACVM.

Check warning on line 58 in acvm-repo/acvm/src/pwg/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (passd)
///
/// Once this is done, the ACVM can be restarted to solve the remaining opcodes.
RequiresAcirCall(AcirCallWaitInfo<F>),
Expand Down Expand Up @@ -359,7 +359,6 @@

pub fn solve_opcode(&mut self) -> ACVMStatus<F> {
let opcode = &self.opcodes[self.instruction_pointer];

let resolution = match opcode {
Opcode::AssertZero(expr) => ExpressionSolver::solve(&mut self.witness_map, expr),
Opcode::BlackBoxFuncCall(bb_func) => blackbox::solve(
Expand Down
Loading