Skip to content

Commit

Permalink
[1 changes] feat(ci): Add report of Brillig opcodes executed (noir-la…
Browse files Browse the repository at this point in the history
…ng/noir#6396)

feat: improve malformed test attribute error (noir-lang/noir#6414)
fix: aliases in path (noir-lang/noir#6399)
feat(profiler): Add Brillig procedure info to debug artifact for more informative profiling (noir-lang/noir#6385)
chore(lsp): Remove profile code lens (noir-lang/noir#6411)
chore(nargo): Remove old profile as part of info cmd (noir-lang/noir#6406)
fix: distinguish TypePath with and without turbofish (noir-lang/noir#6404)
fix: numeric generic doesn't have a default type (noir-lang/noir#6405)
feat: Sync from aztec-packages (noir-lang/noir#6403)
chore: add regression tests for #4372 (noir-lang/noir#6401)
chore: add regression tests for #6314 (noir-lang/noir#6381)
  • Loading branch information
AztecBot committed Oct 31, 2024
1 parent d9de430 commit 1e0a742
Show file tree
Hide file tree
Showing 77 changed files with 924 additions and 518 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
075c3d32481314d900cbdea0d277de83444747ab
e04b02621e3651ddbb8e314563d614171a8a9933
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
name: Report Brillig opcodes executed diff

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
7z a -ttar -so -an ./dist/* | 7z a -si ./nargo-x86_64-unknown-linux-gnu.tar.gz
- name: Upload artifact
uses: actions/upload-artifact@v4
with:
name: nargo
path: ./dist/*
retention-days: 3

compare_brillig_execution_reports:
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 Brillig execution report
working-directory: ./test_programs
run: |
chmod +x gates_report_brillig_execution.sh
./gates_report_brillig_execution.sh
mv gates_report_brillig_execution.json ../gates_report_brillig_execution.json
- name: Compare Brillig execution reports
id: brillig_execution_diff
uses: noir-lang/noir-gates-diff@d88f7523b013b9edd3f31c5cfddaef87a3fe1b48
with:
report: gates_report_brillig_execution.json
header: |
# Changes to number of Brillig opcodes executed
brillig_report: true
summaryQuantile: 0.9 # only display the 10% most significant bytecode size diffs in the summary (defaults to 20%)

- name: Add bytecode size diff to sticky comment
if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target'
uses: marocchino/sticky-pull-request-comment@v2
with:
header: brillig_execution
# delete the comment in case changes no longer impact brillig bytecode sizes
delete: ${{ !steps.brillig_execution_diff.outputs.markdown }}
message: ${{ steps.brillig_execution_diff.outputs.markdown }}
1 change: 1 addition & 0 deletions noir/noir-repo/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ tooling/noir_js/lib

gates_report.json
gates_report_brillig.json
gates_report_brillig_execution.json

# Github Actions scratch space
# This gives a location to download artifacts into the repository in CI without making git dirty.
Expand Down
3 changes: 1 addition & 2 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/acvm-repo/acvm_js/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function run_if_available {
require_command jq
require_command cargo
require_command wasm-bindgen
#require_command wasm-opt
require_command wasm-opt

self_path=$(dirname "$(readlink -f "$0")")
pname=$(cargo read-manifest | jq -r '.name')
Expand Down
2 changes: 1 addition & 1 deletion noir/noir-repo/compiler/integration-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore --max-warnings 0"
},
"dependencies": {
"@aztec/bb.js": "portal:../../../../barretenberg/ts",
"@aztec/bb.js": "0.61.0",
"@noir-lang/noir_js": "workspace:*",
"@noir-lang/noir_wasm": "workspace:*",
"@nomicfoundation/hardhat-chai-matchers": "^2.0.0",
Expand Down
55 changes: 11 additions & 44 deletions noir/noir-repo/compiler/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use serde::Serializer;
use serde_with::serde_as;
use serde_with::DisplayFromStr;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::io::Read;
use std::io::Write;
use std::mem;
Expand Down Expand Up @@ -48,6 +47,9 @@ pub type DebugVariables = BTreeMap<DebugVarId, DebugVariable>;
pub type DebugFunctions = BTreeMap<DebugFnId, DebugFunction>;
pub type DebugTypes = BTreeMap<DebugTypeId, PrintableType>;

#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)]
pub struct ProcedureDebugId(pub u32);

#[derive(Default, Debug, Clone, Deserialize, Serialize)]
pub struct ProgramDebugInfo {
pub debug_infos: Vec<DebugInfo>,
Expand Down Expand Up @@ -104,14 +106,9 @@ pub struct DebugInfo {
pub variables: DebugVariables,
pub functions: DebugFunctions,
pub types: DebugTypes,
}

/// Holds OpCodes Counts for Acir and Brillig Opcodes
/// To be printed with `nargo info --profile-info`
#[derive(Default, Debug, Serialize, Deserialize, Clone)]
pub struct OpCodesCount {
pub acir_size: usize,
pub brillig_size: usize,
/// This a map per brillig function representing the range of opcodes where a procedure is activated.
pub brillig_procedure_locs:
BTreeMap<BrilligFunctionId, BTreeMap<ProcedureDebugId, (usize, usize)>>,
}

impl DebugInfo {
Expand All @@ -124,8 +121,12 @@ impl DebugInfo {
variables: DebugVariables,
functions: DebugFunctions,
types: DebugTypes,
brillig_procedure_locs: BTreeMap<
BrilligFunctionId,
BTreeMap<ProcedureDebugId, (usize, usize)>,
>,
) -> Self {
Self { locations, brillig_locations, variables, functions, types }
Self { locations, brillig_locations, variables, functions, types, brillig_procedure_locs }
}

/// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified.
Expand All @@ -147,38 +148,4 @@ impl DebugInfo {
pub fn opcode_location(&self, loc: &OpcodeLocation) -> Option<Vec<Location>> {
self.locations.get(loc).cloned()
}

pub fn count_span_opcodes(&self) -> HashMap<Location, OpCodesCount> {
let mut accumulator: HashMap<Location, Vec<&OpcodeLocation>> = HashMap::new();

for (opcode_location, locations) in self.locations.iter() {
for location in locations.iter() {
let opcodes = accumulator.entry(*location).or_default();
opcodes.push(opcode_location);
}
}

let counted_opcodes = accumulator
.iter()
.map(|(location, opcodes)| {
let acir_opcodes: Vec<_> = opcodes
.iter()
.filter(|opcode_location| matches!(opcode_location, OpcodeLocation::Acir(_)))
.collect();
let brillig_opcodes: Vec<_> = opcodes
.iter()
.filter(|opcode_location| {
matches!(opcode_location, OpcodeLocation::Brillig { .. })
})
.collect();
let opcodes_count = OpCodesCount {
acir_size: acir_opcodes.len(),
brillig_size: brillig_opcodes.len(),
};
(*location, opcodes_count)
})
.collect();

counted_opcodes
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,8 @@ pub(crate) fn directive_invert<F: AcirField>() -> GeneratedBrillig<F> {
},
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 1 },
],
assert_messages: Default::default(),
locations: Default::default(),
name: "directive_invert".to_string(),
..Default::default()
}
}

Expand Down Expand Up @@ -132,8 +131,7 @@ pub(crate) fn directive_quotient<F: AcirField>() -> GeneratedBrillig<F> {
},
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 2 },
],
assert_messages: Default::default(),
locations: Default::default(),
name: "directive_integer_quotient".to_string(),
..Default::default()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ use acvm::{
};
use debug_show::DebugShow;

use super::ProcedureId;

/// The Brillig VM does not apply a limit to the memory address space,
/// As a convention, we take use 32 bits. This means that we assume that
/// memory has 2^32 memory slots.
Expand Down Expand Up @@ -111,9 +113,14 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {

/// Special brillig context to codegen compiler intrinsic shared procedures
impl<F: AcirField + DebugToString> BrilligContext<F, ScratchSpace> {
pub(crate) fn new_for_procedure(enable_debug_trace: bool) -> BrilligContext<F, ScratchSpace> {
pub(crate) fn new_for_procedure(
enable_debug_trace: bool,
procedure_id: ProcedureId,
) -> BrilligContext<F, ScratchSpace> {
let mut obj = BrilligArtifact::default();
obj.procedure = Some(procedure_id);
BrilligContext {
obj: BrilligArtifact::default(),
obj,
registers: ScratchSpace::new(),
context_label: Label::entrypoint(),
current_section: 0,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use acvm::acir::brillig::Opcode as BrilligOpcode;
use std::collections::{BTreeMap, HashMap};

use crate::brillig::brillig_ir::procedures::ProcedureId;
use crate::ssa::ir::{basic_block::BasicBlockId, dfg::CallStack, function::FunctionId};

use super::procedures::ProcedureId;

/// Represents a parameter or a return value of an entry point function.
#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)]
pub(crate) enum BrilligParameter {
Expand All @@ -24,6 +25,7 @@ pub(crate) struct GeneratedBrillig<F> {
pub(crate) locations: BTreeMap<OpcodeLocation, CallStack>,
pub(crate) assert_messages: BTreeMap<OpcodeLocation, String>,
pub(crate) name: String,
pub(crate) procedure_locations: HashMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
}

#[derive(Default, Debug, Clone)]
Expand Down Expand Up @@ -53,6 +55,14 @@ pub(crate) struct BrilligArtifact<F> {
call_stack: CallStack,
/// Name of the function, only used for debugging purposes.
pub(crate) name: String,

/// This field contains the given procedure id if this artifact originates from as procedure
pub(crate) procedure: Option<ProcedureId>,
/// Procedure ID mapped to the range of its opcode locations
/// This is created as artifacts are linked together and allows us to determine
/// which opcodes originate from reusable procedures.s
/// The range is inclusive for both start and end opcode locations.
pub(crate) procedure_locations: HashMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
}

/// A pointer to a location in the opcode.
Expand Down Expand Up @@ -149,6 +159,7 @@ impl<F: Clone + std::fmt::Debug> BrilligArtifact<F> {
locations: self.locations,
assert_messages: self.assert_messages,
name: self.name,
procedure_locations: self.procedure_locations,
}
}

Expand Down
Loading

0 comments on commit 1e0a742

Please sign in to comment.