Merge branch 'master' into gd/issue_6201

.aztec-sync-commit

@@ -1 +1 @@
-07d6dc29db2eb04154b8f0c66bd1efa74c0e8b9d
+d9de430e4a01d6908a9b1fe5e6ede9309aa8a10d

.github/workflows/gates_report_brillig_execution.yml

@@ -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 }}

.gitignore

@@ -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.

acvm-repo/acir/codegen/acir.cpp

@@ -694,7 +694,7 @@ namespace Program {
         };
 
         struct Trap {
-            Program::HeapArray revert_data;
+            Program::HeapVector revert_data;
 
             friend bool operator==(const Trap&, const Trap&);
             std::vector<uint8_t> bincodeSerialize() const;

acvm-repo/acvm/tests/solver.rs

@@ -1,10 +1,10 @@
 use std::collections::{BTreeMap, HashSet};
 use std::sync::Arc;
 
-use acir::brillig::{BitSize, IntegerBitSize};
+use acir::brillig::{BitSize, HeapVector, IntegerBitSize};
 use acir::{
     acir_field::GenericFieldElement,
-    brillig::{BinaryFieldOp, HeapArray, MemoryAddress, Opcode as BrilligOpcode, ValueOrArray},
+    brillig::{BinaryFieldOp, MemoryAddress, Opcode as BrilligOpcode, ValueOrArray},
     circuit::{
         brillig::{BrilligBytecode, BrilligFunctionId, BrilligInputs, BrilligOutputs},
         opcodes::{BlackBoxFuncCall, BlockId, BlockType, FunctionInput, MemOp},
@@ -667,7 +667,12 @@ fn unsatisfied_opcode_resolved_brillig() {
     let jmp_if_opcode =
         BrilligOpcode::JumpIf { condition: MemoryAddress::direct(2), location: location_of_stop };
 
-    let trap_opcode = BrilligOpcode::Trap { revert_data: HeapArray::default() };
+    let trap_opcode = BrilligOpcode::Trap {
+        revert_data: HeapVector {
+            pointer: MemoryAddress::direct(0),
+            size: MemoryAddress::direct(3),
+        },
+    };
     let stop_opcode = BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 0 };
 
     let brillig_bytecode = BrilligBytecode {
@@ -682,6 +687,11 @@ fn unsatisfied_opcode_resolved_brillig() {
                 bit_size: BitSize::Integer(IntegerBitSize::U32),
                 value: FieldElement::from(0u64),
             },
+            BrilligOpcode::Const {
+                destination: MemoryAddress::direct(3),
+                bit_size: BitSize::Integer(IntegerBitSize::U32),
+                value: FieldElement::from(0u64),
+            },
             calldata_copy_opcode,
             equal_opcode,
             jmp_if_opcode,
@@ -739,7 +749,7 @@ fn unsatisfied_opcode_resolved_brillig() {
         ACVMStatus::Failure(OpcodeResolutionError::BrilligFunctionFailed {
             function_id: BrilligFunctionId(0),
             payload: None,
-            call_stack: vec![OpcodeLocation::Brillig { acir_index: 0, brillig_index: 5 }]
+            call_stack: vec![OpcodeLocation::Brillig { acir_index: 0, brillig_index: 6 }]
         }),
         "The first opcode is not satisfiable, expected an error indicating this"
     );

acvm-repo/brillig/src/opcodes.rs

@@ -305,7 +305,7 @@ pub enum BrilligOpcode<F> {
     BlackBox(BlackBoxOp),
     /// Used to denote execution failure, returning data after the offset
     Trap {
-        revert_data: HeapArray,
+        revert_data: HeapVector,
     },
     /// Stop execution, returning data after the offset
     Stop {

acvm-repo/brillig_vm/src/black_box.rs

@@ -330,18 +330,18 @@ pub(crate) fn evaluate_black_box<F: AcirField, Solver: BlackBoxFunctionSolver<F>
             let mut input = BigUint::from_bytes_be(&input.to_be_bytes());
             let radix = BigUint::from_bytes_be(&radix.to_be_bytes());
 
-            let mut limbs: Vec<MemoryValue<F>> = Vec::with_capacity(output.size);
+            let mut limbs: Vec<MemoryValue<F>> = vec![MemoryValue::default(); output.size];
 
-            for _ in 0..output.size {
+            for i in (0..output.size).rev() {
                 let limb = &input % &radix;
                 if *output_bits {
-                    limbs.push(MemoryValue::new_integer(
+                    limbs[i] = MemoryValue::new_integer(
                         if limb.is_zero() { 0 } else { 1 },
                         IntegerBitSize::U1,
-                    ));
+                    );
                 } else {
                     let limb: u8 = limb.try_into().unwrap();
-                    limbs.push(MemoryValue::new_integer(limb as u128, IntegerBitSize::U8));
+                    limbs[i] = MemoryValue::new_integer(limb as u128, IntegerBitSize::U8);
                 };
                 input /= &radix;
             }

acvm-repo/brillig_vm/src/lib.rs

@@ -347,10 +347,11 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver<F>> VM<'a, F, B> {
                 self.increment_program_counter()
             }
             Opcode::Trap { revert_data } => {
-                if revert_data.size > 0 {
+                let revert_data_size = self.memory.read(revert_data.size).to_usize();
+                if revert_data_size > 0 {
                     self.trap(
                         self.memory.read_ref(revert_data.pointer).unwrap_direct(),
-                        revert_data.size,
+                        revert_data_size,
                     )
                 } else {
                     self.trap(0, 0)
@@ -937,8 +938,18 @@ mod tests {
                 size_address: MemoryAddress::direct(0),
                 offset_address: MemoryAddress::direct(1),
             },
-            Opcode::Jump { location: 5 },
-            Opcode::Trap { revert_data: HeapArray::default() },
+            Opcode::Jump { location: 6 },
+            Opcode::Const {
+                destination: MemoryAddress::direct(0),
+                bit_size: BitSize::Integer(IntegerBitSize::U32),
+                value: FieldElement::from(0u64),
+            },
+            Opcode::Trap {
+                revert_data: HeapVector {
+                    pointer: MemoryAddress::direct(0),
+                    size: MemoryAddress::direct(0),
+                },
+            },
             Opcode::BinaryFieldOp {
                 op: BinaryFieldOp::Equals,
                 lhs: MemoryAddress::direct(0),
@@ -966,6 +977,8 @@ mod tests {
         assert_eq!(status, VMStatus::InProgress);
         let status = vm.process_opcode();
         assert_eq!(status, VMStatus::InProgress);
+        let status = vm.process_opcode();
+        assert_eq!(status, VMStatus::InProgress);
 
         let output_cmp_value = vm.memory.read(MemoryAddress::direct(2));
         assert_eq!(output_cmp_value.to_field(), false.into());
@@ -978,7 +991,7 @@ mod tests {
             status,
             VMStatus::Failure {
                 reason: FailureReason::Trap { revert_data_offset: 0, revert_data_size: 0 },
-                call_stack: vec![4]
+                call_stack: vec![5]
             }
         );
 

compiler/integration-tests/package.json

@@ -13,7 +13,7 @@
     "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore  --max-warnings 0"
   },
   "dependencies": {
-    "@aztec/bb.js": "0.60.0",
+    "@aztec/bb.js": "0.61.0",
     "@noir-lang/noir_js": "workspace:*",
     "@noir-lang/noir_wasm": "workspace:*",
     "@nomicfoundation/hardhat-chai-matchers": "^2.0.0",

compiler/noirc_errors/src/debug_info.rs

@@ -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;
@@ -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>,
@@ -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 {
@@ -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.
@@ -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
-    }
 }