Skip to content

Commit

Permalink
feat(LSP): code action to import trait in a method call (noir-lang/no…
Browse files Browse the repository at this point in the history
…ir#7066)

feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067)
chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049)
chore: reenable reports on rollup root circuits (noir-lang/noir#7061)
fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041)
chore: mark some critical libraries as good again (noir-lang/noir#7065)
fix: Reduce memory usage in mem2reg (noir-lang/noir#7053)
feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026)
chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058)
chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050)
chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056)
chore: clippy warning fix (noir-lang/noir#7051)
chore(ci): Unify compilation/execution report jobs that take averages with single runs  (noir-lang/noir#7048)
fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046)
fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043)
feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037)
feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036)
fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027)
chore: add more protocol circuits to reports (noir-lang/noir#6977)
feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031)
feat: skip codegen of zero iteration loops (noir-lang/noir#7030)
  • Loading branch information
AztecBot committed Jan 14, 2025
2 parents d5a650d + 2f92e4a commit a3586e0
Show file tree
Hide file tree
Showing 46 changed files with 1,015 additions and 179 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1e64f8a7e0481f43f8afeb3dc3c33300f6fa5857
3b8d1da2ac9f751b8fff3eaad0c64cbba8d01575
3 changes: 3 additions & 0 deletions noir/noir-repo/.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
86 changes: 47 additions & 39 deletions noir/noir-repo/.github/workflows/reports.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,19 @@ jobs:
- name: Generate Brillig bytecode size report
working-directory: ./test_programs
run: |
./gates_report_brillig.sh
mv gates_report_brillig.json ../gates_report_brillig.json
mkdir ./reports
./gates_report_brillig.sh 9223372036854775807
jq '.programs |= map(.package_name |= (. + "_inliner_max"))' gates_report_brillig.json > ./reports/gates_report_brillig_inliner_max.json
./gates_report_brillig.sh 0
jq '.programs |= map(.package_name |= (. + "_inliner_zero"))' gates_report_brillig.json > ./reports/gates_report_brillig_inliner_zero.json
./gates_report_brillig.sh -9223372036854775808
jq '.programs |= map(.package_name |= (. + "_inliner_min"))' gates_report_brillig.json > ./reports/gates_report_brillig_inliner_min.json
# Merge all reports
jq -s '{ programs: map(.programs) | add }' ./reports/* > ../gates_report_brillig.json
- name: Compare Brillig bytecode size reports
id: brillig_bytecode_diff
Expand Down Expand Up @@ -165,8 +176,19 @@ jobs:
- name: Generate Brillig execution report
working-directory: ./test_programs
run: |
./gates_report_brillig_execution.sh
mv gates_report_brillig_execution.json ../gates_report_brillig_execution.json
mkdir ./reports
./gates_report_brillig_execution.sh 9223372036854775807
jq '.programs |= map(.package_name |= (. + "_inliner_max"))' gates_report_brillig_execution.json > ./reports/gates_report_brillig_execution_inliner_max.json
./gates_report_brillig_execution.sh 0
jq '.programs |= map(.package_name |= (. + "_inliner_zero"))' gates_report_brillig_execution.json > ./reports/gates_report_brillig_execution_inliner_zero.json
./gates_report_brillig_execution.sh -9223372036854775808
jq '.programs |= map(.package_name |= (. + "_inliner_min"))' gates_report_brillig_execution.json > ./reports/gates_report_brillig_execution_inliner_min.json
# Merge all reports
jq -s '{ programs: map(.programs) | add }' ./reports/* > ../gates_report_brillig_execution.json
- name: Compare Brillig execution reports
id: brillig_execution_diff
Expand Down Expand Up @@ -299,16 +321,16 @@ jobs:
fail-fast: false
matrix:
include:
# - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-contracts, is_library: true }
# - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-contracts, cannot_execute: true }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-inner, num_runs: 5 }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-tail, num_runs: 5 }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset, num_runs: 5 }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-private, num_runs: 5 }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-public, num_runs: 5 }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-merge, num_runs: 5 }
#- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root-empty }
#- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-single-tx }
#- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root-empty, num_runs: 5, cannot_execute: true }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root-single-tx, num_runs: 1, flags: "--skip-brillig-constraints-check --skip-underconstrained-check", cannot_execute: true }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root, num_runs: 1, flags: "--skip-brillig-constraints-check --skip-underconstrained-check" }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-merge, num_runs: 5 }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-root, num_runs: 5 }

Expand Down Expand Up @@ -353,41 +375,17 @@ jobs:
chmod +x parse_time.sh
cp /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh
./compilation_report.sh 1 ${{ matrix.project.num_runs }}
env:
FLAGS: ${{ matrix.project.flags }}

- name: Generate execution report
working-directory: ./test-repo/${{ matrix.project.path }}
if: ${{ !matrix.project.is_library }}
if: ${{ !matrix.project.cannot_execute }}
run: |
mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh
mv /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh
./execution_report.sh 1 ${{ matrix.project.num_runs }}
- name: Generate compilation report with averages
working-directory: ./test-repo/${{ matrix.project.path }}
if: ${{ matrix.project.take_average }}
run: |
mv /home/runner/work/noir/noir/scripts/test_programs/compilation_report.sh ./compilation_report.sh
touch parse_time.sh
chmod +x parse_time.sh
cp /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh
./compilation_report.sh 1 1
- name: Generate execution report without averages
working-directory: ./test-repo/${{ matrix.project.path }}
if: ${{ !matrix.project.is_library && !matrix.project.take_average }}
run: |
mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh
mv /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh
./execution_report.sh 1
- name: Generate execution report with averages
working-directory: ./test-repo/${{ matrix.project.path }}
if: ${{ !matrix.project.is_library && matrix.project.take_average }}
run: |
mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh
mv /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh
./execution_report.sh 1 1
- name: Move compilation report
id: compilation_report
shell: bash
Expand All @@ -400,7 +398,7 @@ jobs:
- name: Move execution report
id: execution_report
shell: bash
if: ${{ !matrix.project.is_library }}
if: ${{ !matrix.project.cannot_execute }}
run: |
PACKAGE_NAME=${{ matrix.project.path }}
PACKAGE_NAME=$(basename $PACKAGE_NAME)
Expand Down Expand Up @@ -478,10 +476,16 @@ jobs:
# TODO: Bring this report back under a flag. The `noir-contracts` report takes just under 30 minutes.
# - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-contracts }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-inner }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-tail }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-tail }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-private }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-public }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-merge }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root-empty, cannot_execute: true }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root-single-tx, flags: "--skip-brillig-constraints-check --skip-underconstrained-check", cannot_execute: true }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root, flags: "--skip-brillig-constraints-check --skip-underconstrained-check" }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-merge }
- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-root }

name: External repo memory report - ${{ matrix.project.repo }}/${{ matrix.project.path }}
steps:
Expand Down Expand Up @@ -520,9 +524,12 @@ jobs:
./memory_report.sh 1
# Rename the memory report as the execution report is about to write to the same file
cp memory_report.json compilation_memory_report.json
env:
FLAGS: ${{ matrix.project.flags }}

- name: Generate execution memory report
working-directory: ./test-repo/${{ matrix.project.path }}
if: ${{ !matrix.project.cannot_execute }}
run: |
./memory_report.sh 1 1
Expand All @@ -545,6 +552,7 @@ jobs:

- name: Move execution report
id: execution_mem_report
if: ${{ !matrix.project.cannot_execute }}
shell: bash
run: |
PACKAGE_NAME=${{ matrix.project.path }}
Expand All @@ -564,7 +572,7 @@ jobs:
name: Upload compilation memory report
needs: [generate_memory_report, external_repo_memory_report]
# We want this job to run even if one variation of the matrix in `external_repo_memory_report` fails
if: always()
if: always()
runs-on: ubuntu-latest
permissions:
pull-requests: write
Expand Down
1 change: 1 addition & 0 deletions noir/noir-repo/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 noir/noir-repo/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 noir/noir-repo/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
23 changes: 20 additions & 3 deletions noir/noir-repo/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 noir/noir-repo/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
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ mod block;
use std::collections::{BTreeMap, BTreeSet};

use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
use vec_collections::VecSet;

use crate::ssa::{
ir::{
Expand Down Expand Up @@ -619,7 +620,7 @@ impl<'f> PerFunctionContext<'f> {
// then those parameters also alias each other.
// We save parameters with repeat arguments to later mark those
// parameters as aliasing one another.
let mut arg_set: HashMap<ValueId, BTreeSet<ValueId>> = HashMap::default();
let mut arg_set = HashMap::default();

// Add an alias for each reference parameter
for (parameter, argument) in destination_parameters.iter().zip(arguments) {
Expand All @@ -632,7 +633,8 @@ impl<'f> PerFunctionContext<'f> {
aliases.insert(*parameter);

// Check if we have seen the same argument
let seen_parameters = arg_set.entry(argument).or_default();
let seen_parameters =
arg_set.entry(argument).or_insert_with(VecSet::empty);
// Add the current parameter to the parameters we have seen for this argument.
// The previous parameters and the current one alias one another.
seen_parameters.insert(*parameter);
Expand Down
Loading

0 comments on commit a3586e0

Please sign in to comment.