Skip to content

Commit

Permalink
Merge branch 'master' into tf/remove-foreign-call-handler-logic-from-…
Browse files Browse the repository at this point in the history
…compiler

* master:
  chore: update noir-bench-report version (#6675)
  fix: Prevent hoisting binary instructions which can overflow (#6672)
  feat(ssa): Hoisting of array get using known induction variable maximum (#6639)
  feat: better error message when trying to invoke struct function field (#6661)
  feat: add memory report into the CI (#6630)
  feat: allow ignoring test failures from foreign calls (#6660)
  chore: refactor foreign call executors (#6659)
  fix: correct signed integer handling in `noirc_abi` (#6638)
  fix: allow multiple `_` parameters, and disallow `_` as an expression you can read from (#6657)
  feat: allow filtering which SSA passes are printed (#6636)
  fix: use correct type for attribute arguments (#6640)
  fix: always return an array of `u8`s when simplifying `Intrinsic::ToRadix` calls (#6663)
  feat(ssa): Option to set the maximum acceptable Brillig bytecode increase in unrolling (#6641)
  • Loading branch information
TomAFrench committed Dec 2, 2024
2 parents 399db7a + 920077d commit cc9e1a8
Show file tree
Hide file tree
Showing 42 changed files with 1,166 additions and 179 deletions.
88 changes: 88 additions & 0 deletions .github/workflows/memory_report.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
name: Report Peak Memory

on:
push:
branches:
- master
pull_request:

jobs:
build-nargo:
runs-on: ubuntu-latest
strategy:
matrix:
target: [x86_64-unknown-linux-gnu]

steps:
- name: Checkout Noir repo
uses: actions/checkout@v4

- name: Setup toolchain
uses: dtolnay/[email protected]

- uses: Swatinem/rust-cache@v2
with:
key: ${{ matrix.target }}
cache-on-failure: true
save-if: ${{ github.event_name != 'merge_group' }}

- name: Build Nargo
run: cargo build --package nargo_cli --release

- name: Package artifacts
run: |
mkdir dist
cp ./target/release/nargo ./dist/nargo
- name: Upload artifact
uses: actions/upload-artifact@v4
with:
name: nargo
path: ./dist/*
retention-days: 3

generate_memory_report:
needs: [build-nargo]
runs-on: ubuntu-latest
permissions:
pull-requests: write

steps:
- uses: actions/checkout@v4

- name: Download nargo binary
uses: actions/download-artifact@v4
with:
name: nargo
path: ./nargo

- name: Set nargo on PATH
run: |
nargo_binary="${{ github.workspace }}/nargo/nargo"
chmod +x $nargo_binary
echo "$(dirname $nargo_binary)" >> $GITHUB_PATH
export PATH="$PATH:$(dirname $nargo_binary)"
nargo -V
- name: Generate Memory report
working-directory: ./test_programs
run: |
chmod +x memory_report.sh
./memory_report.sh
mv memory_report.json ../memory_report.json
- name: Parse memory report
id: memory_report
uses: noir-lang/noir-bench-report@ccb0d806a91d3bd86dba0ba3d580a814eed5673c
with:
report: memory_report.json
header: |
# Memory Report
memory_report: true

- name: Add memory report to sticky comment
if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target'
uses: marocchino/sticky-pull-request-comment@v2
with:
header: memory
message: ${{ steps.memory_report.outputs.markdown }}
30 changes: 23 additions & 7 deletions .github/workflows/test-js-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -519,12 +519,25 @@ jobs:
fail-fast: false
matrix:
project:
# Disabled as these are currently failing with many visibility errors
- { repo: AztecProtocol/aztec-nr, path: ./ }
- { repo: noir-lang/ec, path: ./ }
- { repo: noir-lang/eddsa, path: ./ }
- { repo: noir-lang/mimc, path: ./ }
- { repo: noir-lang/noir_sort, path: ./ }
- { repo: noir-lang/noir-edwards, path: ./ }
- { repo: noir-lang/noir-bignum, path: ./ }
- { repo: noir-lang/noir_bigcurve, path: ./ }
- { repo: noir-lang/noir_base64, path: ./ }
- { repo: noir-lang/noir_string_search, path: ./ }
- { repo: noir-lang/sparse_array, path: ./ }
- { repo: noir-lang/noir_rsa, path: ./lib }
- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/aztec-nr }
- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-contracts }
# Disabled as aztec-packages requires a setup-step in order to generate a `Nargo.toml`
#- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits }
- { repo: noir-lang/noir-edwards, path: ./, ref: 3188ea74fe3b059219a2ea87899589c266256d74 }
- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/parity-lib }
- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/private-kernel-lib }
- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/reset-kernel-lib }
- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/rollup-lib }
- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/types }

name: Check external repo - ${{ matrix.project.repo }}
steps:
- name: Checkout
Expand Down Expand Up @@ -554,9 +567,12 @@ jobs:
# Github actions seems to not expand "**" in globs by default.
shopt -s globstar
sed -i '/^compiler_version/d' ./**/Nargo.toml
- name: Run nargo check
- name: Run nargo test
working-directory: ./test-repo/${{ matrix.project.path }}
run: nargo check
run: nargo test --silence-warnings
env:
NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true

# 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.
Expand Down
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.

29 changes: 26 additions & 3 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use noirc_abi::{AbiParameter, AbiType, AbiValue};
use noirc_errors::{CustomDiagnostic, FileDiagnostic};
use noirc_evaluator::create_program;
use noirc_evaluator::errors::RuntimeError;
use noirc_evaluator::ssa::SsaProgramArtifact;
use noirc_evaluator::ssa::{SsaLogging, SsaProgramArtifact};
use noirc_frontend::debug::build_debug_crate_file;
use noirc_frontend::hir::def_map::{Contract, CrateDefMap};
use noirc_frontend::hir::Context;
Expand Down Expand Up @@ -70,6 +70,11 @@ pub struct CompileOptions {
#[arg(long, hide = true)]
pub show_ssa: bool,

/// Only show SSA passes whose name contains the provided string.
/// This setting takes precedence over `show_ssa` if it's not empty.
#[arg(long, hide = true)]
pub show_ssa_pass_name: Option<String>,

/// Emit the unoptimized SSA IR to file.
/// The IR will be dumped into the workspace target directory,
/// under `[compiled-package].ssa.json`.
Expand Down Expand Up @@ -126,11 +131,19 @@ pub struct CompileOptions {
#[arg(long)]
pub skip_underconstrained_check: bool,

/// Setting to decide on an inlining strategy for brillig functions.
/// Setting to decide on an inlining strategy for Brillig functions.
/// A more aggressive inliner should generate larger programs but more optimized
/// A less aggressive inliner should generate smaller programs
#[arg(long, hide = true, allow_hyphen_values = true, default_value_t = i64::MAX)]
pub inliner_aggressiveness: i64,

/// Setting the maximum acceptable increase in Brillig bytecode size due to
/// unrolling small loops. When left empty, any change is accepted as long
/// as it required fewer SSA instructions.
/// A higher value results in fewer jumps but a larger program.
/// A lower value keeps the original program if it was smaller, even if it has more jumps.
#[arg(long, hide = true, allow_hyphen_values = true)]
pub max_bytecode_increase_percent: Option<i32>,
}

pub fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::Error> {
Expand Down Expand Up @@ -577,7 +590,16 @@ pub fn compile_no_check(
}
let return_visibility = program.return_visibility;
let ssa_evaluator_options = noirc_evaluator::ssa::SsaEvaluatorOptions {
enable_ssa_logging: options.show_ssa,
ssa_logging: match &options.show_ssa_pass_name {
Some(string) => SsaLogging::Contains(string.clone()),
None => {
if options.show_ssa {
SsaLogging::All
} else {
SsaLogging::None
}
}
},
enable_brillig_logging: options.show_brillig,
force_brillig_output: options.force_brillig,
print_codegen_timings: options.benchmark_codegen,
Expand All @@ -589,6 +611,7 @@ pub fn compile_no_check(
emit_ssa: if options.emit_ssa { Some(context.package_build_path.clone()) } else { None },
skip_underconstrained_check: options.skip_underconstrained_check,
inliner_aggressiveness: options.inliner_aggressiveness,
max_bytecode_increase_percent: options.max_bytecode_increase_percent,
};

let SsaProgramArtifact { program, debug, warnings, names, brillig_names, error_types, .. } =
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 @@ -33,6 +33,7 @@ cfg-if.workspace = true
proptest.workspace = true
similar-asserts.workspace = true
num-traits.workspace = true
test-case.workspace = true

[features]
bn254 = ["noirc_frontend/bn254"]
Expand Down
8 changes: 3 additions & 5 deletions compiler/noirc_evaluator/src/brillig/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use self::{
},
};
use crate::ssa::{
ir::function::{Function, FunctionId, RuntimeType},
ir::function::{Function, FunctionId},
ssa_gen::Ssa,
};
use fxhash::FxHashMap as HashMap;
Expand Down Expand Up @@ -59,17 +59,15 @@ impl std::ops::Index<FunctionId> for Brillig {
}

impl Ssa {
/// Compile to brillig brillig functions and ACIR functions reachable from them
/// Compile Brillig functions and ACIR functions reachable from them
#[tracing::instrument(level = "trace", skip_all)]
pub(crate) fn to_brillig(&self, enable_debug_trace: bool) -> Brillig {
// Collect all the function ids that are reachable from brillig
// That means all the functions marked as brillig and ACIR functions called by them
let brillig_reachable_function_ids = self
.functions
.iter()
.filter_map(|(id, func)| {
matches!(func.runtime(), RuntimeType::Brillig(_)).then_some(*id)
})
.filter_map(|(id, func)| func.runtime().is_brillig().then_some(*id))
.collect::<BTreeSet<_>>();

let mut brillig = Brillig::default();
Expand Down
Loading

0 comments on commit cc9e1a8

Please sign in to comment.