Skip to content

Commit

Permalink
chore: free memory for silenced warnings early (noir-lang/noir#6748)
Browse files Browse the repository at this point in the history
chore(stdlib)!: Remove Schnorr (noir-lang/noir#6749)
fix: Improve type error when indexing a variable of unknown type (noir-lang/noir#6744)
fix: println("{{}}") was printing "{{}}" instead of "{}" (noir-lang/noir#6745)
feat: `std::hint::black_box` function. (noir-lang/noir#6529)
feat(ci): Initial compilation report on test_programs (noir-lang/noir#6731)
chore: Cleanup unrolling pass (noir-lang/noir#6743)
fix: allow empty loop headers (noir-lang/noir#6736)
fix: map entry point indexes after all ssa passes (noir-lang/noir#6740)
chore: Update url to 2.5.4 (noir-lang/noir#6741)
feat: Order attribute execution by their source ordering (noir-lang/noir#6326)
feat(test): Check that `nargo::ops::transform_program` is idempotent (noir-lang/noir#6694)
feat: Sync from aztec-packages (noir-lang/noir#6730)
  • Loading branch information
AztecBot committed Dec 10, 2024
2 parents 36fff22 + 2563f49 commit 493106f
Show file tree
Hide file tree
Showing 41 changed files with 606 additions and 402 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
852155dc1c4a910bf9cd4e7af334f3856c1c4643
52bcb0f24b27090e812611c23ed326a541d37153
48 changes: 47 additions & 1 deletion noir/noir-repo/.github/workflows/reports.yml
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ jobs:
- name: Parse memory report
id: memory_report
uses: noir-lang/noir-bench-report@ccb0d806a91d3bd86dba0ba3d580a814eed5673c
uses: noir-lang/noir-bench-report@0d7464a8c39170523932d7846b6e6b458a294aea
with:
report: memory_report.json
header: |
Expand All @@ -233,3 +233,49 @@ jobs:
with:
header: memory
message: ${{ steps.memory_report.outputs.markdown }}

generate_compilation_report:
name: Compilation time
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 Compilation report
working-directory: ./test_programs
run: |
./compilation_report.sh
mv compilation_report.json ../compilation_report.json
- name: Parse compilation report
id: compilation_report
uses: noir-lang/noir-bench-report@0d7464a8c39170523932d7846b6e6b458a294aea
with:
report: compilation_report.json
header: |
# Compilation Report
memory_report: false

- 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: compilation
message: ${{ steps.compilation_report.outputs.markdown }}
1 change: 1 addition & 0 deletions noir/noir-repo/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ tooling/noir_js/lib
gates_report.json
gates_report_brillig.json
gates_report_brillig_execution.json
compilation_report.json

# Github Actions scratch space
# This gives a location to download artifacts into the repository in CI without making git dirty.
Expand Down
12 changes: 6 additions & 6 deletions noir/noir-repo/Cargo.lock

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

2 changes: 1 addition & 1 deletion noir/noir-repo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ serde_json = "1.0"
smol_str = { version = "0.1.17", features = ["serde"] }
thiserror = "1.0.21"
toml = "0.7.2"
url = "2.2.0"
url = "2.5.4"
base64 = "0.21.2"
fxhash = "0.2.1"
build-data = "0.1.3"
Expand Down
2 changes: 1 addition & 1 deletion noir/noir-repo/acvm-repo/acvm/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ 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 = 1;
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.
Expand Down
24 changes: 15 additions & 9 deletions noir/noir-repo/compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use clap::Args;
use fm::{FileId, FileManager};
use iter_extended::vecmap;
use noirc_abi::{AbiParameter, AbiType, AbiValue};
use noirc_errors::{CustomDiagnostic, FileDiagnostic};
use noirc_errors::{CustomDiagnostic, DiagnosticKind, FileDiagnostic};
use noirc_evaluator::create_program;
use noirc_evaluator::errors::RuntimeError;
use noirc_evaluator::ssa::{SsaLogging, SsaProgramArtifact};
Expand Down Expand Up @@ -301,23 +301,29 @@ pub fn check_crate(
crate_id: CrateId,
options: &CompileOptions,
) -> CompilationResult<()> {
let mut errors = vec![];
let error_on_unused_imports = true;
let diagnostics = CrateDefMap::collect_defs(
crate_id,
context,
options.debug_comptime_in_file.as_deref(),
error_on_unused_imports,
);
errors.extend(diagnostics.into_iter().map(|(error, file_id)| {
let diagnostic = CustomDiagnostic::from(&error);
diagnostic.in_file(file_id)
}));
let warnings_and_errors: Vec<FileDiagnostic> = diagnostics
.into_iter()
.map(|(error, file_id)| {
let diagnostic = CustomDiagnostic::from(&error);
diagnostic.in_file(file_id)
})
.filter(|diagnostic| {
// We filter out any warnings if they're going to be ignored later on to free up memory.
!options.silence_warnings || diagnostic.diagnostic.kind != DiagnosticKind::Warning
})
.collect();

if has_errors(&errors, options.deny_warnings) {
Err(errors)
if has_errors(&warnings_and_errors, options.deny_warnings) {
Err(warnings_and_errors)
} else {
Ok(((), errors))
Ok(((), warnings_and_errors))
}
}

Expand Down
25 changes: 19 additions & 6 deletions noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use crate::brillig::{
Brillig,
};
use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
use crate::ssa::ir::instruction::Hint;
use crate::ssa::{
function_builder::data_bus::DataBus,
ir::{
Expand Down Expand Up @@ -821,14 +822,12 @@ impl<'a> Context<'a> {
})
.sum();

let Some(acir_function_id) =
ssa.entry_point_to_generated_index.get(id)
else {
let Some(acir_function_id) = ssa.get_entry_point_index(id) else {
unreachable!("Expected an associated final index for call to acir function {id} with args {arguments:?}");
};

let output_vars = self.acir_context.call_acir_function(
AcirFunctionId(*acir_function_id),
AcirFunctionId(acir_function_id),
inputs,
output_count,
self.current_side_effects_enabled_var,
Expand Down Expand Up @@ -2133,6 +2132,15 @@ impl<'a> Context<'a> {
result_ids: &[ValueId],
) -> Result<Vec<AcirValue>, RuntimeError> {
match intrinsic {
Intrinsic::Hint(Hint::BlackBox) => {
// Identity function; at the ACIR level this is a no-op, it only affects the SSA.
assert_eq!(
arguments.len(),
result_ids.len(),
"ICE: BlackBox input and output lengths should match."
);
Ok(arguments.iter().map(|v| self.convert_value(*v, dfg)).collect())
}
Intrinsic::BlackBox(black_box) => {
// Slices are represented as a tuple of (length, slice contents).
// We must check the inputs to determine if there are slices
Expand Down Expand Up @@ -2979,7 +2987,7 @@ mod test {

build_basic_foo_with_return(&mut builder, foo_id, false, inline_type);

let ssa = builder.finish();
let ssa = builder.finish().generate_entry_point_index();

let (acir_functions, _, _, _) = ssa
.into_acir(&Brillig::default(), ExpressionWidth::default())
Expand Down Expand Up @@ -3087,6 +3095,7 @@ mod test {
let ssa = builder.finish();

let (acir_functions, _, _, _) = ssa
.generate_entry_point_index()
.into_acir(&Brillig::default(), ExpressionWidth::default())
.expect("Should compile manually written SSA into ACIR");
// The expected result should look very similar to the above test expect that the input witnesses of the `Call`
Expand Down Expand Up @@ -3184,7 +3193,7 @@ mod test {

build_basic_foo_with_return(&mut builder, foo_id, false, inline_type);

let ssa = builder.finish();
let ssa = builder.finish().generate_entry_point_index();

let (acir_functions, _, _, _) = ssa
.into_acir(&Brillig::default(), ExpressionWidth::default())
Expand Down Expand Up @@ -3311,6 +3320,7 @@ mod test {
let brillig = ssa.to_brillig(false);

let (acir_functions, brillig_functions, _, _) = ssa
.generate_entry_point_index()
.into_acir(&brillig, ExpressionWidth::default())
.expect("Should compile manually written SSA into ACIR");

Expand Down Expand Up @@ -3375,6 +3385,7 @@ mod test {
// The Brillig bytecode we insert for the stdlib is hardcoded so we do not need to provide any
// Brillig artifacts to the ACIR gen pass.
let (acir_functions, brillig_functions, _, _) = ssa
.generate_entry_point_index()
.into_acir(&Brillig::default(), ExpressionWidth::default())
.expect("Should compile manually written SSA into ACIR");

Expand Down Expand Up @@ -3449,6 +3460,7 @@ mod test {
println!("{}", ssa);

let (acir_functions, brillig_functions, _, _) = ssa
.generate_entry_point_index()
.into_acir(&brillig, ExpressionWidth::default())
.expect("Should compile manually written SSA into ACIR");

Expand Down Expand Up @@ -3537,6 +3549,7 @@ mod test {
println!("{}", ssa);

let (acir_functions, brillig_functions, _, _) = ssa
.generate_entry_point_index()
.into_acir(&brillig, ExpressionWidth::default())
.expect("Should compile manually written SSA into ACIR");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::brillig::brillig_ir::{
BrilligBinaryOp, BrilligContext, ReservedRegisters, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
};
use crate::ssa::ir::dfg::CallStack;
use crate::ssa::ir::instruction::ConstrainError;
use crate::ssa::ir::instruction::{ConstrainError, Hint};
use crate::ssa::ir::{
basic_block::BasicBlockId,
dfg::DataFlowGraph,
Expand Down Expand Up @@ -552,6 +552,10 @@ impl<'block> BrilligBlock<'block> {
false,
);
}
Intrinsic::Hint(Hint::BlackBox) => {
let result_ids = dfg.instruction_results(instruction_id);
self.convert_ssa_identity_call(arguments, dfg, result_ids);
}
Intrinsic::BlackBox(bb_func) => {
// Slices are represented as a tuple of (length, slice contents).
// We must check the inputs to determine if there are slices
Expand Down Expand Up @@ -874,6 +878,30 @@ impl<'block> BrilligBlock<'block> {
self.brillig_context.codegen_call(func_id, &argument_variables, &return_variables);
}

/// Copy the input arguments to the results.
fn convert_ssa_identity_call(
&mut self,
arguments: &[ValueId],
dfg: &DataFlowGraph,
result_ids: &[ValueId],
) {
let argument_variables =
vecmap(arguments, |argument_id| self.convert_ssa_value(*argument_id, dfg));

let return_variables = vecmap(result_ids, |result_id| {
self.variables.define_variable(
self.function_context,
self.brillig_context,
*result_id,
dfg,
)
});

for (src, dst) in argument_variables.into_iter().zip(return_variables) {
self.brillig_context.mov_instruction(dst.extract_register(), src.extract_register());
}
}

fn validate_array_index(
&mut self,
array_variable: BrilligVariable,
Expand Down
Loading

0 comments on commit 493106f

Please sign in to comment.