feat: Sync from noir (#6280)

Automated pull of development from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE feat: add `Not` trait to stdlib (noir-lang/noir#4999) feat: increase default expression width to 4 (noir-lang/noir#4995) chore: adding name shadowing tests template program (noir-lang/noir#4799) feat: implement `ops` traits on `u16`/`i16` (noir-lang/noir#4996) chore: disable `gates_report.yml` (noir-lang/noir#4997) feat: Sync from aztec-packages (noir-lang/noir#4993) fix: defer overflow checks for unsigned integers to acir-gen (noir-lang/noir#4832) feat: add support for u16/i16 (noir-lang/noir#4985) chore: split `ops` into `arith` and `bit` modules (noir-lang/noir#4989) chore(ci): run clippy on benchmarks (noir-lang/noir#4988) feat: remove query to backend to get expression width (noir-lang/noir#4975) END_COMMIT_OVERRIDE --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: Tom French <tom@tomfren.ch>
AztecProtocol · May 8, 2024 · 11cde44 · 11cde44
1 parent 1ca0d28
commit 11cde44
Show file tree

Hide file tree

Showing 54 changed files with 2,347 additions and 1,913 deletions.
diff --git a/.noir-sync-commit b/.noir-sync-commit
@@ -1 +1 @@
-c49d3a9ded819b828cffdfc031e86614da21e329
+95d4d133d1eb5e0eb44cd928d8183d890e970a13
diff --git a/noir/noir-repo/.github/workflows/formatting.yml b/noir/noir-repo/.github/workflows/formatting.yml
@@ -44,7 +44,7 @@ jobs:
           save-if: ${{ github.event_name != 'merge_group' }}
 
       - name: Run `cargo clippy`
-        run: cargo clippy --workspace --locked --release
+        run: cargo clippy --all-targets --workspace --locked --release
 
       - name: Run `cargo fmt`
         run: cargo fmt --all --check

diff --git a/noir/noir-repo/.github/workflows/gates_report.yml b/noir/noir-repo/.github/workflows/gates_report.yml
@@ -1,88 +1,88 @@
-name: Report gates diff
+# name: Report gates diff
 
-on:
-  push:
-    branches:
-      - master
-  pull_request:
+# on:
+#   push:
+#     branches:
+#       - master
+#   pull_request:
 
-jobs:
-  build-nargo:
-    runs-on: ubuntu-latest
-    strategy:
-      matrix:
-        target: [x86_64-unknown-linux-gnu]
+# jobs:
+#   build-nargo:
+#     runs-on: ubuntu-latest
+#     strategy:
+#       matrix:
+#         target: [x86_64-unknown-linux-gnu]
 
-    steps:
-      - name: Checkout Noir repo
-        uses: actions/checkout@v4
+#     steps:
+#       - name: Checkout Noir repo
+#         uses: actions/checkout@v4
 
-      - name: Setup toolchain
-        uses: dtolnay/rust-toolchain@1.74.1
+#       - name: Setup toolchain
+#         uses: dtolnay/rust-toolchain@1.74.1
 
-      - uses: Swatinem/rust-cache@v2
-        with:
-          key: ${{ matrix.target }}
-          cache-on-failure: true
-          save-if: ${{ github.event_name != 'merge_group' }}
+#       - 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: 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: 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
+#       - name: Upload artifact
+#         uses: actions/upload-artifact@v4
+#         with:
+#           name: nargo
+#           path: ./dist/*
+#           retention-days: 3
 
 
-  compare_gas_reports:
-    needs: [build-nargo]
-    runs-on: ubuntu-latest
-    permissions:
-      pull-requests: write
+#   compare_gas_reports:
+#     needs: [build-nargo]
+#     runs-on: ubuntu-latest
+#     permissions:
+#       pull-requests: write
 
-    steps:
-      - uses: actions/checkout@v4
+#     steps:
+#       - uses: actions/checkout@v4
 
-      - name: Download nargo binary
-        uses: actions/download-artifact@v4
-        with:
-          name: nargo
-          path: ./nargo
+#       - 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: 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 gates report
-        working-directory: ./test_programs
-        run: |
-          ./gates_report.sh
-          mv gates_report.json ../gates_report.json
+#       - name: Generate gates report
+#         working-directory: ./test_programs
+#         run: |
+#           ./gates_report.sh
+#           mv gates_report.json ../gates_report.json
 
-      - name: Compare gates reports
-        id: gates_diff
-        uses: vezenovm/noir-gates-diff@acf12797860f237117e15c0d6e08d64253af52b6
-        with:
-          report: gates_report.json
-          summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%)
+#       - name: Compare gates reports
+#         id: gates_diff
+#         uses: vezenovm/noir-gates-diff@acf12797860f237117e15c0d6e08d64253af52b6
+#         with:
+#           report: gates_report.json
+#           summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%)
 
-      - name: Add gates diff to sticky comment
-        if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target'
-        uses: marocchino/sticky-pull-request-comment@v2
-        with:
-          # delete the comment in case changes no longer impact circuit sizes
-          delete: ${{ !steps.gates_diff.outputs.markdown }}
-          message: ${{ steps.gates_diff.outputs.markdown }}
+#       - name: Add gates diff to sticky comment
+#         if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target'
+#         uses: marocchino/sticky-pull-request-comment@v2
+#         with:
+#           # delete the comment in case changes no longer impact circuit sizes
+#           delete: ${{ !steps.gates_diff.outputs.markdown }}
+#           message: ${{ steps.gates_diff.outputs.markdown }}
diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock
diff --git a/noir/noir-repo/acvm-repo/acir/benches/serialization.rs b/noir/noir-repo/acvm-repo/acir/benches/serialization.rs
@@ -33,7 +33,7 @@ fn sample_program(num_opcodes: usize) -> Program {
         functions: vec![Circuit {
             current_witness_index: 4000,
             opcodes: assert_zero_opcodes.to_vec(),
-            expression_width: ExpressionWidth::Bounded { width: 3 },
+            expression_width: ExpressionWidth::Bounded { width: 4 },
             private_parameters: BTreeSet::from([Witness(1), Witness(2), Witness(3), Witness(4)]),
             public_parameters: PublicInputs(BTreeSet::from([Witness(5)])),
             return_values: PublicInputs(BTreeSet::from([Witness(6)])),

diff --git a/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/constant_backpropagation.rs b/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/constant_backpropagation.rs
@@ -282,7 +282,7 @@ mod tests {
     fn test_circuit(opcodes: Vec<Opcode>) -> Circuit {
         Circuit {
             current_witness_index: 1,
-            expression_width: ExpressionWidth::Bounded { width: 3 },
+            expression_width: ExpressionWidth::Bounded { width: 4 },
             opcodes,
             private_parameters: BTreeSet::new(),
             public_parameters: PublicInputs::default(),

diff --git a/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs b/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs
@@ -164,7 +164,7 @@ mod tests {
 
         Circuit {
             current_witness_index: 1,
-            expression_width: ExpressionWidth::Bounded { width: 3 },
+            expression_width: ExpressionWidth::Bounded { width: 4 },
             opcodes,
             private_parameters: BTreeSet::new(),
             public_parameters: PublicInputs::default(),

diff --git a/noir/noir-repo/compiler/noirc_driver/src/lib.rs b/noir/noir-repo/compiler/noirc_driver/src/lib.rs
@@ -54,8 +54,8 @@ pub const NOIR_ARTIFACT_VERSION_STRING: &str =
 #[derive(Args, Clone, Debug, Default)]
 pub struct CompileOptions {
     /// Override the expression width requested by the backend.
-    #[arg(long, value_parser = parse_expression_width)]
-    pub expression_width: Option<ExpressionWidth>,
+    #[arg(long, value_parser = parse_expression_width, default_value = "4")]
+    pub expression_width: ExpressionWidth,
 
     /// Force a full recompilation.
     #[arg(long = "force")]

diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
@@ -1328,7 +1328,15 @@ impl<'block> BrilligBlock<'block> {
 
         self.brillig_context.binary_instruction(left, right, result_variable, brillig_binary_op);
 
-        self.add_overflow_check(brillig_binary_op, left, right, result_variable, is_signed);
+        self.add_overflow_check(
+            brillig_binary_op,
+            left,
+            right,
+            result_variable,
+            binary,
+            dfg,
+            is_signed,
+        );
     }
 
     /// Splits a two's complement signed integer in the sign bit and the absolute value.
@@ -1481,22 +1489,32 @@ impl<'block> BrilligBlock<'block> {
         self.brillig_context.deallocate_single_addr(bias);
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn add_overflow_check(
         &mut self,
         binary_operation: BrilligBinaryOp,
         left: SingleAddrVariable,
         right: SingleAddrVariable,
         result: SingleAddrVariable,
+        binary: &Binary,
+        dfg: &DataFlowGraph,
         is_signed: bool,
     ) {
         let bit_size = left.bit_size;
+        let max_lhs_bits = dfg.get_value_max_num_bits(binary.lhs);
+        let max_rhs_bits = dfg.get_value_max_num_bits(binary.rhs);
 
         if bit_size == FieldElement::max_num_bits() {
             return;
         }
 
         match (binary_operation, is_signed) {
             (BrilligBinaryOp::Add, false) => {
+                if std::cmp::max(max_lhs_bits, max_rhs_bits) < bit_size {
+                    // `left` and `right` have both been casted up from smaller types and so cannot overflow.
+                    return;
+                }
+
                 let condition =
                     SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
                 // Check that lhs <= result
@@ -1511,6 +1529,12 @@ impl<'block> BrilligBlock<'block> {
                 self.brillig_context.deallocate_single_addr(condition);
             }
             (BrilligBinaryOp::Sub, false) => {
+                if dfg.is_constant(binary.lhs) && max_lhs_bits > max_rhs_bits {
+                    // `left` is a fixed constant and `right` is restricted such that `left - right > 0`
+                    // Note strict inequality as `right > left` while `max_lhs_bits == max_rhs_bits` is possible.
+                    return;
+                }
+
                 let condition =
                     SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
                 // Check that rhs <= lhs
@@ -1527,39 +1551,36 @@ impl<'block> BrilligBlock<'block> {
                 self.brillig_context.deallocate_single_addr(condition);
             }
             (BrilligBinaryOp::Mul, false) => {
-                // Multiplication overflow is only possible for bit sizes > 1
-                if bit_size > 1 {
-                    let is_right_zero =
-                        SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
-                    let zero =
-                        self.brillig_context.make_constant_instruction(0_usize.into(), bit_size);
-                    self.brillig_context.binary_instruction(
-                        zero,
-                        right,
-                        is_right_zero,
-                        BrilligBinaryOp::Equals,
-                    );
-                    self.brillig_context.codegen_if_not(is_right_zero.address, |ctx| {
-                        let condition = SingleAddrVariable::new(ctx.allocate_register(), 1);
-                        let division = SingleAddrVariable::new(ctx.allocate_register(), bit_size);
-                        // Check that result / rhs == lhs
-                        ctx.binary_instruction(
-                            result,
-                            right,
-                            division,
-                            BrilligBinaryOp::UnsignedDiv,
-                        );
-                        ctx.binary_instruction(division, left, condition, BrilligBinaryOp::Equals);
-                        ctx.codegen_constrain(
-                            condition,
-                            Some("attempt to multiply with overflow".to_string()),
-                        );
-                        ctx.deallocate_single_addr(condition);
-                        ctx.deallocate_single_addr(division);
-                    });
-                    self.brillig_context.deallocate_single_addr(is_right_zero);
-                    self.brillig_context.deallocate_single_addr(zero);
+                if bit_size == 1 || max_lhs_bits + max_rhs_bits <= bit_size {
+                    // Either performing boolean multiplication (which cannot overflow),
+                    // or `left` and `right` have both been casted up from smaller types and so cannot overflow.
+                    return;
                 }
+
+                let is_right_zero =
+                    SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
+                let zero = self.brillig_context.make_constant_instruction(0_usize.into(), bit_size);
+                self.brillig_context.binary_instruction(
+                    zero,
+                    right,
+                    is_right_zero,
+                    BrilligBinaryOp::Equals,
+                );
+                self.brillig_context.codegen_if_not(is_right_zero.address, |ctx| {
+                    let condition = SingleAddrVariable::new(ctx.allocate_register(), 1);
+                    let division = SingleAddrVariable::new(ctx.allocate_register(), bit_size);
+                    // Check that result / rhs == lhs
+                    ctx.binary_instruction(result, right, division, BrilligBinaryOp::UnsignedDiv);
+                    ctx.binary_instruction(division, left, condition, BrilligBinaryOp::Equals);
+                    ctx.codegen_constrain(
+                        condition,
+                        Some("attempt to multiply with overflow".to_string()),
+                    );
+                    ctx.deallocate_single_addr(condition);
+                    ctx.deallocate_single_addr(division);
+                });
+                self.brillig_context.deallocate_single_addr(is_right_zero);
+                self.brillig_context.deallocate_single_addr(zero);
             }
             _ => {}
         }
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		c49d3a9ded819b828cffdfc031e86614da21e329
		95d4d133d1eb5e0eb44cd928d8183d890e970a13