Skip to content

Commit

Permalink
Merge 8de08f2 into f1195ea
Browse files Browse the repository at this point in the history
  • Loading branch information
AztecBot authored May 8, 2024
2 parents f1195ea + 8de08f2 commit aeaa9b0
Show file tree
Hide file tree
Showing 54 changed files with 2,347 additions and 1,913 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
c49d3a9ded819b828cffdfc031e86614da21e329
95d4d133d1eb5e0eb44cd928d8183d890e970a13
2 changes: 1 addition & 1 deletion noir/noir-repo/.github/workflows/formatting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
144 changes: 72 additions & 72 deletions noir/noir-repo/.github/workflows/gates_report.yml
Original file line number Diff line number Diff line change
@@ -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 }}
1 change: 0 additions & 1 deletion 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/acir/benches/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)])),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
4 changes: 2 additions & 2 deletions noir/noir-repo/compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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);
}
_ => {}
}
Expand Down
Loading

0 comments on commit aeaa9b0

Please sign in to comment.