Skip to content

Commit

Permalink
Merge branch 'master' into jf/stop-overlapping-impls
Browse files Browse the repository at this point in the history
  • Loading branch information
TomAFrench authored Jan 20, 2025
2 parents 8c28477 + 9be84f7 commit 4d0a4f9
Show file tree
Hide file tree
Showing 172 changed files with 8,025 additions and 1,140 deletions.
3 changes: 3 additions & 0 deletions .github/critical_libraries_status/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.actual.jsonl
.actual.jsonl.jq
.failures.jsonl.jq
Empty file.
26 changes: 26 additions & 0 deletions .github/workflows/formatting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,29 @@ jobs:
- name: Format test suite
working-directory: ./test_programs
run: ./format.sh check

# This is a job which depends on all test jobs and reports the overall status.
# This allows us to add/remove test jobs without having to update the required workflows.
formatting-end:
name: Formatting End
runs-on: ubuntu-22.04
# We want this job to always run (even if the dependant jobs fail) as we want this job to fail rather than skipping.
if: ${{ always() }}
needs:
- clippy
- rustfmt
- eslint
- nargo_fmt

steps:
- name: Report overall success
run: |
if [[ $FAIL == true ]]; then
exit 1
else
exit 0
fi
env:
# We treat any skipped or failing jobs as a failure for the workflow as a whole.
FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }}

311 changes: 174 additions & 137 deletions .github/workflows/reports.yml

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions CRITICAL_NOIR_LIBRARIES
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
https://github.com/noir-lang/noir_check_shuffle
https://github.com/noir-lang/ec
https://github.com/noir-lang/eddsa
https://github.com/noir-lang/mimc
Expand Down
37 changes: 37 additions & 0 deletions Cargo.lock

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

41 changes: 3 additions & 38 deletions acvm-repo/acvm/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ 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 Down Expand Up @@ -84,41 +80,10 @@ pub fn compile<F: AcirField>(
) -> (Circuit<F>, AcirTransformationMap) {
let acir_opcode_positions = (0..acir.opcodes.len()).collect::<Vec<_>>();

if MAX_OPTIMIZER_PASSES == 0 {
return (acir, AcirTransformationMap::new(&acir_opcode_positions));
}

let mut pass = 0;
let mut prev_opcodes_hash = fxhash::hash64(&acir.opcodes);
let mut prev_acir = acir;
let mut prev_acir_opcode_positions = acir_opcode_positions;

// 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, prev_acir_opcode_positions);

// 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 (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 (acir, acir_opcode_positions) = optimize_internal(acir, acir_opcode_positions);

pass += 1;
prev_acir = acir;
prev_opcodes_hash = opcodes_hash;
prev_acir_opcode_positions = acir_opcode_positions;
};
let (mut acir, acir_opcode_positions) =
transform_internal(acir, expression_width, acir_opcode_positions);

let transformation_map = AcirTransformationMap::new(&acir_opcode_positions);
acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map);
Expand Down
9 changes: 8 additions & 1 deletion acvm-repo/acvm/src/compiler/optimizers/unused_memory.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use acir::circuit::{opcodes::BlockId, Circuit, Opcode};
use acir::circuit::{brillig::BrilligInputs, opcodes::BlockId, Circuit, Opcode};
use std::collections::HashSet;

/// `UnusedMemoryOptimizer` will remove initializations of memory blocks which are unused.
Expand Down Expand Up @@ -29,6 +29,13 @@ impl<F> UnusedMemoryOptimizer<F> {
Opcode::MemoryOp { block_id, .. } => {
unused_memory_initialization.remove(block_id);
}
Opcode::BrilligCall { inputs, .. } => {
for input in inputs {
if let BrilligInputs::MemoryArray(block) = input {
unused_memory_initialization.remove(block);
}
}
}
_ => (),
}
}
Expand Down
23 changes: 20 additions & 3 deletions acvm-repo/acvm/src/compiler/transformers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,17 @@ mod csat;

pub(crate) use csat::CSatTransformer;
pub use csat::MIN_EXPRESSION_WIDTH;
use tracing::info;

use super::{
optimizers::MergeExpressionsOptimizer, transform_assert_messages, AcirTransformationMap,
MAX_OPTIMIZER_PASSES,
optimizers::{MergeExpressionsOptimizer, RangeOptimizer},
transform_assert_messages, AcirTransformationMap,
};

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

/// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] specific optimizations to a [`Circuit`].
pub fn transform<F: AcirField>(
acir: Circuit<F>,
Expand Down Expand Up @@ -50,12 +55,18 @@ pub(super) fn transform_internal<F: AcirField>(
expression_width: ExpressionWidth,
mut acir_opcode_positions: Vec<usize>,
) -> (Circuit<F>, Vec<usize>) {
if acir.opcodes.len() == 1 && matches!(acir.opcodes[0], Opcode::BrilligCall { .. }) {
info!("Program is fully unconstrained, skipping transformation pass");
return (acir, acir_opcode_positions);
}

// Allow multiple passes until we have stable output.
let mut prev_opcodes_hash = fxhash::hash64(&acir.opcodes);

// For most test programs it would be enough to loop here, but some of them
// don't stabilize unless we also repeat the backend agnostic optimizations.
for _ in 0..MAX_OPTIMIZER_PASSES {
for _ in 0..MAX_TRANSFORMER_PASSES {
info!("Number of opcodes {}", acir.opcodes.len());
let (new_acir, new_acir_opcode_positions) =
transform_internal_once(acir, expression_width, acir_opcode_positions);

Expand Down Expand Up @@ -217,6 +228,12 @@ fn transform_internal_once<F: AcirField>(
..acir
};

// The `MergeOptimizer` can merge two witnesses which have range opcodes applied to them
// so we run the `RangeOptimizer` afterwards to clear these up.
let range_optimizer = RangeOptimizer::new(acir);
let (acir, new_acir_opcode_positions) =
range_optimizer.replace_redundant_ranges(new_acir_opcode_positions);

(acir, new_acir_opcode_positions)
}

Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_evaluator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ chrono = "0.4.37"
rayon.workspace = true
cfg-if.workspace = true
smallvec = { version = "1.13.2", features = ["serde"] }
vec-collections = "0.4.3"

[dev-dependencies]
proptest.workspace = true
Expand Down
23 changes: 23 additions & 0 deletions compiler/noirc_evaluator/src/acir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,29 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
Ok(())
}

/// Constrains the `lhs` and `rhs` to be non-equal.
///
/// This is done by asserting the existence of an inverse for the value `lhs - rhs`.
/// The constraint `(lhs - rhs) * inverse == 1` will only be satisfiable if `lhs` and `rhs` are non-equal.
pub(crate) fn assert_neq_var(
&mut self,
lhs: AcirVar,
rhs: AcirVar,
assert_message: Option<AssertionPayload<F>>,
) -> Result<(), RuntimeError> {
let diff_var = self.sub_var(lhs, rhs)?;

let one = self.add_constant(F::one());
let _ = self.inv_var(diff_var, one)?;
if let Some(payload) = assert_message {
self.acir_ir
.assertion_payloads
.insert(self.acir_ir.last_acir_opcode_location(), payload);
}

Ok(())
}

pub(crate) fn vars_to_expressions_or_memory(
&self,
values: &[AcirValue],
Expand Down
52 changes: 48 additions & 4 deletions compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,47 @@ impl<'a> Context<'a> {

self.acir_context.assert_eq_var(lhs, rhs, assert_payload)?;
}
Instruction::ConstrainNotEqual(lhs, rhs, assert_message) => {
let lhs = self.convert_numeric_value(*lhs, dfg)?;
let rhs = self.convert_numeric_value(*rhs, dfg)?;

let assert_payload = if let Some(error) = assert_message {
match error {
ConstrainError::StaticString(string) => Some(
self.acir_context.generate_assertion_message_payload(string.clone()),
),
ConstrainError::Dynamic(error_selector, is_string_type, values) => {
if let Some(constant_string) = try_to_extract_string_from_error_payload(
*is_string_type,
values,
dfg,
) {
Some(
self.acir_context
.generate_assertion_message_payload(constant_string),
)
} else {
let acir_vars: Vec<_> = values
.iter()
.map(|value| self.convert_value(*value, dfg))
.collect();

let expressions_or_memory =
self.acir_context.vars_to_expressions_or_memory(&acir_vars)?;

Some(AssertionPayload {
error_selector: error_selector.as_u64(),
payload: expressions_or_memory,
})
}
}
}
} else {
None
};

self.acir_context.assert_neq_var(lhs, rhs, assert_payload)?;
}
Instruction::Cast(value_id, _) => {
let acir_var = self.convert_numeric_value(*value_id, dfg)?;
self.define_result_var(dfg, instruction_id, acir_var);
Expand Down Expand Up @@ -2868,7 +2909,7 @@ mod test {
use std::collections::BTreeMap;

use crate::{
acir::BrilligStdlibFunc,
acir::{BrilligStdlibFunc, Function},
brillig::Brillig,
ssa::{
function_builder::FunctionBuilder,
Expand Down Expand Up @@ -3300,7 +3341,8 @@ mod test {
build_basic_foo_with_return(&mut builder, foo_id, true, InlineType::default());
build_basic_foo_with_return(&mut builder, bar_id, true, InlineType::default());

let ssa = builder.finish();
let mut ssa = builder.finish();
ssa.globals = Function::new("globals".to_owned(), ssa.main_id);
let brillig = ssa.to_brillig(false);

let (acir_functions, brillig_functions, _, _) = ssa
Expand Down Expand Up @@ -3438,7 +3480,8 @@ mod test {

build_basic_foo_with_return(&mut builder, foo_id, true, InlineType::default());

let ssa = builder.finish();
let mut ssa = builder.finish();
ssa.globals = Function::new("globals".to_owned(), ssa.main_id);
// We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass.
let brillig = ssa.to_brillig(false);
println!("{}", ssa);
Expand Down Expand Up @@ -3527,7 +3570,8 @@ mod test {
// Build an ACIR function which has the same logic as the Brillig function above
build_basic_foo_with_return(&mut builder, bar_id, false, InlineType::Fold);

let ssa = builder.finish();
let mut ssa = builder.finish();
ssa.globals = Function::new("globals".to_owned(), ssa.main_id);
// We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass.
let brillig = ssa.to_brillig(false);
println!("{}", ssa);
Expand Down
Loading

0 comments on commit 4d0a4f9

Please sign in to comment.