Skip to content

Commit

Permalink
Merge branch 'master' into aztec-packages
Browse files Browse the repository at this point in the history
* master: (65 commits)
  chore: remove some `_else_condition` tech debt (#6522)
  chore: revert #6375 (#6552)
  feat: simplify constant MSM calls in SSA (#6547)
  chore(test): Remove duplicate brillig tests (#6523)
  chore: restructure `noirc_evaluator` crate (#6534)
  fix: take blackbox function outputs into account when merging expressions (#6532)
  chore: Add `Instruction::MakeArray` to SSA (#6071)
  feat(profiler): Reduce memory in Brillig execution flamegraph (#6538)
  chore: convert some tests to use SSA parser (#6543)
  chore(ci): bump mac github runner image to `macos-14` (#6545)
  chore(test): More descriptive labels in test matrix (#6542)
  chore: Remove unused methods from implicit numeric generics (#6541)
  fix: Fix poor handling of aliased references in flattening pass causing some values to be zeroed (#6434)
  fix: allow range checks to be performed within the comptime intepreter (#6514)
  fix: disallow `#[test]` on associated functions (#6449)
  chore(ssa): Skip array_set pass for Brillig functions (#6513)
  chore: Reverse ssa parser diff order (#6511)
  chore: Parse negatives in SSA parser (#6510)
  feat: avoid unnecessary ssa passes while loop unrolling (#6509)
  fix(tests): Use a file lock as well as a mutex to isolate tests cases (#6508)
  ...
  • Loading branch information
TomAFrench committed Nov 19, 2024
2 parents 0317d46 + 5f730e8 commit 92617d8
Show file tree
Hide file tree
Showing 104 changed files with 2,345 additions and 1,974 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/publish-nargo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ permissions:

jobs:
build-apple-darwin:
runs-on: macos-12
runs-on: macos-14
env:
CROSS_CONFIG: ${{ github.workspace }}/.github/Cross.toml
NIGHTLY_RELEASE: ${{ inputs.tag == '' }}
Expand All @@ -41,7 +41,7 @@ jobs:
- name: Setup for Apple Silicon
if: matrix.target == 'aarch64-apple-darwin'
run: |
sudo xcode-select -s /Applications/Xcode_13.2.1.app/Contents/Developer/
sudo xcode-select -s /Applications/Xcode_15.4.0.app/Contents/Developer/
echo "SDKROOT=$(xcrun -sdk macosx$(sw_vers -productVersion) --show-sdk-path)" >> $GITHUB_ENV
echo "MACOSX_DEPLOYMENT_TARGET=$(xcrun -sdk macosx$(sw_vers -productVersion) --show-sdk-platform-version)" >> $GITHUB_ENV
Expand Down
67 changes: 56 additions & 11 deletions acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,12 @@ impl MergeExpressionsOptimizer {

// Returns the input witnesses used by the opcode
fn witness_inputs<F: AcirField>(&self, opcode: &Opcode<F>) -> BTreeSet<Witness> {
let mut witnesses = BTreeSet::new();
match opcode {
Opcode::AssertZero(expr) => CircuitSimulator::expr_wit(expr),
Opcode::BlackBoxFuncCall(bb_func) => bb_func.get_input_witnesses(),
Opcode::MemoryOp { block_id: _, op, predicate } => {
//index et value, et predicate
let mut witnesses = BTreeSet::new();
witnesses.extend(CircuitSimulator::expr_wit(&op.index));
let mut witnesses = CircuitSimulator::expr_wit(&op.index);
witnesses.extend(CircuitSimulator::expr_wit(&op.value));
if let Some(p) = predicate {
witnesses.extend(CircuitSimulator::expr_wit(p));
Expand All @@ -171,6 +169,7 @@ impl MergeExpressionsOptimizer {
init.iter().cloned().collect()
}
Opcode::BrilligCall { inputs, outputs, .. } => {
let mut witnesses = BTreeSet::new();
for i in inputs {
witnesses.extend(self.brillig_input_wit(i));
}
Expand All @@ -180,12 +179,9 @@ impl MergeExpressionsOptimizer {
witnesses
}
Opcode::Call { id: _, inputs, outputs, predicate } => {
for i in inputs {
witnesses.insert(*i);
}
for i in outputs {
witnesses.insert(*i);
}
let mut witnesses: BTreeSet<Witness> = BTreeSet::from_iter(inputs.iter().copied());
witnesses.extend(outputs);

if let Some(p) = predicate {
witnesses.extend(CircuitSimulator::expr_wit(p));
}
Expand Down Expand Up @@ -233,15 +229,15 @@ mod tests {
acir_field::AcirField,
circuit::{
brillig::{BrilligFunctionId, BrilligOutputs},
opcodes::FunctionInput,
opcodes::{BlackBoxFuncCall, FunctionInput},
Circuit, ExpressionWidth, Opcode, PublicInputs,
},
native_types::{Expression, Witness},
FieldElement,
};
use std::collections::BTreeSet;

fn check_circuit(circuit: Circuit<FieldElement>) {
fn check_circuit(circuit: Circuit<FieldElement>) -> Circuit<FieldElement> {
assert!(CircuitSimulator::default().check_circuit(&circuit));
let mut merge_optimizer = MergeExpressionsOptimizer::new();
let acir_opcode_positions = vec![0; 20];
Expand All @@ -251,6 +247,7 @@ mod tests {
optimized_circuit.opcodes = opcodes;
// check that the circuit is still valid after optimization
assert!(CircuitSimulator::default().check_circuit(&optimized_circuit));
optimized_circuit
}

#[test]
Expand Down Expand Up @@ -293,6 +290,7 @@ mod tests {
public_parameters: PublicInputs::default(),
return_values: PublicInputs::default(),
assert_messages: Default::default(),
recursive: false,
};
check_circuit(circuit);
}
Expand Down Expand Up @@ -345,7 +343,54 @@ mod tests {
public_parameters: PublicInputs::default(),
return_values: PublicInputs::default(),
assert_messages: Default::default(),
recursive: false,
};
check_circuit(circuit);
}

#[test]
fn takes_blackbox_opcode_outputs_into_account() {
// Regression test for https://github.com/noir-lang/noir/issues/6527
// Previously we would not track the usage of witness 4 in the output of the blackbox function.
// We would then merge the final two opcodes losing the check that the brillig call must match
// with `_0 ^ _1`.

let circuit: Circuit<FieldElement> = Circuit {
current_witness_index: 7,
opcodes: vec![
Opcode::BrilligCall {
id: BrilligFunctionId(0),
inputs: Vec::new(),
outputs: vec![BrilligOutputs::Simple(Witness(3))],
predicate: None,
},
Opcode::BlackBoxFuncCall(BlackBoxFuncCall::AND {
lhs: FunctionInput::witness(Witness(0), 8),
rhs: FunctionInput::witness(Witness(1), 8),
output: Witness(4),
}),
Opcode::AssertZero(Expression {
linear_combinations: vec![
(FieldElement::one(), Witness(3)),
(-FieldElement::one(), Witness(4)),
],
..Default::default()
}),
Opcode::AssertZero(Expression {
linear_combinations: vec![
(-FieldElement::one(), Witness(2)),
(FieldElement::one(), Witness(4)),
],
..Default::default()
}),
],
expression_width: ExpressionWidth::Bounded { width: 4 },
private_parameters: BTreeSet::from([Witness(0), Witness(1)]),
return_values: PublicInputs(BTreeSet::from([Witness(2)])),
..Default::default()
};

let new_circuit = check_circuit(circuit.clone());
assert_eq!(circuit, new_circuit);
}
}
13 changes: 13 additions & 0 deletions acvm-repo/acvm/src/compiler/simulator.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use acir::{
circuit::{
brillig::{BrilligInputs, BrilligOutputs},
directives::Directive,
opcodes::{BlockId, FunctionInput},
Circuit, Opcode,
},
Expand Down Expand Up @@ -83,6 +84,17 @@ impl CircuitSimulator {
}
true
}
Opcode::Directive(directive) => match directive {
Directive::ToLeRadix { a, b, .. } => {
if !self.can_solve_expression(a) {
return false;
}
for w in b {
self.mark_solvable(*w);
}
true
}
},
Opcode::MemoryOp { block_id, op, predicate } => {
if !self.can_solve_expression(&op.index) {
return false;
Expand Down Expand Up @@ -234,6 +246,7 @@ mod tests {
public_parameters,
return_values: PublicInputs::default(),
assert_messages: Default::default(),
recursive: false,
}
}

Expand Down
1 change: 0 additions & 1 deletion acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ pub fn multi_scalar_mul(
scalars_hi: &[FieldElement],
) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> {
if points.len() != 3 * scalars_lo.len() || scalars_lo.len() != scalars_hi.len() {
dbg!(&points.len(), &scalars_lo.len(), &scalars_hi.len());
return Err(BlackBoxResolutionError::Failed(
BlackBoxFunc::MultiScalarMul,
"Points and scalars must have the same length".to_string(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,34 +1,35 @@
use super::big_int::BigIntContext;
use super::generated_acir::{BrilligStdlibFunc, GeneratedAcir, PLACEHOLDER_BRILLIG_INDEX};
use crate::brillig::brillig_gen::brillig_directive;
use crate::brillig::brillig_ir::artifact::GeneratedBrillig;
use crate::errors::{InternalBug, InternalError, RuntimeError, SsaReport};
use crate::ssa::acir_gen::{AcirDynamicArray, AcirValue};
use crate::ssa::ir::dfg::CallStack;
use crate::ssa::ir::types::Type as SsaType;
use crate::ssa::ir::{instruction::Endian, types::NumericType};
use acvm::acir::circuit::brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs};
use acvm::acir::circuit::opcodes::{
AcirFunctionId, BlockId, BlockType, ConstantOrWitnessEnum, MemOp,
};
use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, ExpressionWidth, Opcode};
use acvm::brillig_vm::{MemoryValue, VMStatus, VM};
use acvm::BlackBoxFunctionSolver;
use acvm::{
acir::AcirField,
acir::{
brillig::Opcode as BrilligOpcode,
circuit::opcodes::FunctionInput,
circuit::{
brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs},
opcodes::{
AcirFunctionId, BlockId, BlockType, ConstantOrWitnessEnum, FunctionInput, MemOp,
},
AssertionPayload, ExpressionOrMemory, ExpressionWidth, Opcode,
},
native_types::{Expression, Witness},
BlackBoxFunc,
AcirField, BlackBoxFunc,
},
brillig_vm::{MemoryValue, VMStatus, VM},
BlackBoxFunctionSolver,
};
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};

use crate::brillig::brillig_ir::artifact::GeneratedBrillig;
use crate::errors::{InternalBug, InternalError, RuntimeError, SsaReport};
use crate::ssa::ir::{
dfg::CallStack, instruction::Endian, types::NumericType, types::Type as SsaType,
};

use super::big_int::BigIntContext;
use super::generated_acir::{BrilligStdlibFunc, GeneratedAcir, PLACEHOLDER_BRILLIG_INDEX};
use super::{brillig_directive, AcirDynamicArray, AcirValue};

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
/// High level Type descriptor for Variables.
///
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
//! `GeneratedAcir` is constructed as part of the `acir_gen` pass to accumulate all of the ACIR
//! program as it is being converted from SSA form.
use std::{collections::BTreeMap, u32};
use std::collections::BTreeMap;

use crate::{
brillig::{brillig_gen::brillig_directive, brillig_ir::artifact::GeneratedBrillig},
errors::{InternalError, RuntimeError, SsaReport},
ssa::ir::{dfg::CallStack, instruction::ErrorType},
};
use acvm::acir::{
circuit::{
brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs},
Expand All @@ -16,7 +11,17 @@ use acvm::acir::{
native_types::Witness,
BlackBoxFunc,
};
use acvm::{acir::native_types::Expression, acir::AcirField};
use acvm::{
acir::AcirField,
acir::{circuit::directives::Directive, native_types::Expression},
};

use super::brillig_directive;
use crate::{
brillig::brillig_ir::artifact::GeneratedBrillig,
errors::{InternalError, RuntimeError, SsaReport},
ssa::ir::dfg::CallStack,
};

use iter_extended::vecmap;
use noirc_errors::debug_info::ProcedureDebugId;
Expand Down Expand Up @@ -155,7 +160,7 @@ impl<F: AcirField> GeneratedAcir<F> {
/// This means you cannot multiply an infinite amount of `Expression`s together.
/// Once the `Expression` goes over degree-2, then it needs to be reduced to a `Witness`
/// which has degree-1 in order to be able to continue the multiplication chain.
pub(crate) fn create_witness_for_expression(&mut self, expression: &Expression<F>) -> Witness {
fn create_witness_for_expression(&mut self, expression: &Expression<F>) -> Witness {
let fresh_witness = self.next_witness_index();

// Create a constraint that sets them to be equal to each other
Expand Down
Loading

0 comments on commit 92617d8

Please sign in to comment.