Skip to content

Commit

Permalink
feat: unchecked math operations in SSA (noir-lang/noir#7011)
Browse files Browse the repository at this point in the history
chore: delete docs for versions which aren't used (noir-lang/noir#7020)
feat!: Disable mocks in `execute` (noir-lang/noir#6869)
chore(docs): backport 1.0.0-beta.0 doc fixes (noir-lang/noir#7014)
feat(cli): Add CLI option to filter by contract function name (noir-lang/noir#7018)
chore: Add more Field use info (noir-lang/noir#7019)
fix: let static_assert fail with the provided message (noir-lang/noir#7005)
chore: mark `aztec-nr` as expected to compile (noir-lang/noir#7015)
chore: clarity fix in docs (noir-lang/noir#7016)
fix: require generic trait impls to be in scope to call them (noir-lang/noir#6913)
feat!: require trait primitive functions/calls to have their trait in scope (noir-lang/noir#6901)
feat(lsp): use trait method docs for trait impl method docs on hover (noir-lang/noir#7003)
chore: turn on averaging for protocol circuits metrics in CI (noir-lang/noir#6999)
feat(comptime): Implement to_be_bits and to_le_bits in the interpreter (noir-lang/noir#7008)
chore: Add short circuit in ssa-gen for known if conditions (noir-lang/noir#7007)
chore: Only resolved globals monomorphization (noir-lang/noir#7006)
chore: Remove resolve_is_unconstrained pass (noir-lang/noir#7004)
chore: require safety doc comment for unsafe instead of `//@safety` (noir-lang/noir#6992)
fix: Reproduce and fix bytecode blowup (noir-lang/noir#6972)
chore: mark casts as able to be deduplicated (noir-lang/noir#6996)
fix: return trait impl method as FuncId if there's only one (noir-lang/noir#6989)
chore(ci): fail properly in `external-repo-checks` (noir-lang/noir#6988)
fix: allow multiple trait impls for the same trait as long as one is in scope (noir-lang/noir#6987)
chore: Use DFG in SSA printer (noir-lang/noir#6986)
chore!: Reserve `enum` and `match` keywords (noir-lang/noir#6961)
  • Loading branch information
AztecBot committed Jan 10, 2025
2 parents 273e594 + 68ba915 commit 3313783
Show file tree
Hide file tree
Showing 943 changed files with 888 additions and 89,976 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
5300ec321fb99ddaad32e83f751aed28e175736f
f6ed6aa9f469c719ca47d01e3118f37cedde0f9e
8 changes: 5 additions & 3 deletions noir/noir-repo/.github/workflows/reports.yml
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ jobs:
name: Upload compilation memory report
needs: [generate_memory_report, external_repo_memory_report]
# We want this job to run even if one variation of the matrix in `external_repo_memory_report` fails
if: always()
if: always()
runs-on: ubuntu-latest
permissions:
pull-requests: write
Expand All @@ -576,17 +576,19 @@ jobs:
run: |
mv ./.github/scripts/merge-bench-reports.sh merge-bench-reports.sh
./merge-bench-reports.sh memory_report
# Rename the memory report as to not clash with the compilation memory report file name
cp memory_report.json execution_memory_report.json
- name: Parse compilation memory report
id: compilation_mem_report
uses: noir-lang/noir-bench-report@6ba151d7795042c4ff51864fbeb13c0a6a79246c
with:
report: memory_report.json
report: execution_memory_report.json
header: |
Compilation Memory Report
memory_report: true

- name: Add memory report to sticky comment
- name: Add execution 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:
Expand Down
4 changes: 0 additions & 4 deletions noir/noir-repo/.github/workflows/test-js-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -610,10 +610,6 @@ jobs:
working-directory: ./noir-repo
run: .github/scripts/check_test_results.sh .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.failures.jsonl .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl

- name: Compare test results
working-directory: ./noir-repo
run: .github/scripts/check_test_results.sh .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.failures.jsonl .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl

# This is a job which depends on all test jobs and reports the overall status.
# This allows us to add/remove test jobs without having to update the required workflows.
tests-end:
Expand Down
23 changes: 20 additions & 3 deletions noir/noir-repo/compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ pub struct CompileOptions {
/// Only show SSA passes whose name contains the provided string.
/// This setting takes precedence over `show_ssa` if it's not empty.
#[arg(long, hide = true)]
pub show_ssa_pass_name: Option<String>,
pub show_ssa_pass: Option<String>,

/// Only show the SSA and ACIR for the contract function with a given name.
#[arg(long, hide = true)]
pub show_contract_fn: Option<String>,

/// Emit the unoptimized SSA IR to file.
/// The IR will be dumped into the workspace target directory,
Expand Down Expand Up @@ -442,6 +446,11 @@ pub fn compile_contract(

if options.print_acir {
for contract_function in &compiled_contract.functions {
if let Some(ref name) = options.show_contract_fn {
if name != &contract_function.name {
continue;
}
}
println!(
"Compiled ACIR for {}::{} (unoptimized):",
compiled_contract.name, contract_function.name
Expand Down Expand Up @@ -486,7 +495,15 @@ fn compile_contract_inner(
continue;
}

let function = match compile_no_check(context, options, function_id, None, true) {
let mut options = options.clone();

if let Some(ref name_filter) = options.show_contract_fn {
let show = name == *name_filter;
options.show_ssa &= show;
options.show_ssa_pass = options.show_ssa_pass.filter(|_| show);
};

let function = match compile_no_check(context, &options, function_id, None, true) {
Ok(function) => function,
Err(new_error) => {
errors.push(FileDiagnostic::from(new_error));
Expand Down Expand Up @@ -642,7 +659,7 @@ pub fn compile_no_check(

let return_visibility = program.return_visibility;
let ssa_evaluator_options = noirc_evaluator::ssa::SsaEvaluatorOptions {
ssa_logging: match &options.show_ssa_pass_name {
ssa_logging: match &options.show_ssa_pass {
Some(string) => SsaLogging::Contains(string.clone()),
None => {
if options.show_ssa {
Expand Down
14 changes: 9 additions & 5 deletions noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1949,9 +1949,9 @@ impl<'a> Context<'a> {
let bit_count = binary_type.bit_size::<FieldElement>();
let num_type = binary_type.to_numeric_type();
let result = match binary.operator {
BinaryOp::Add => self.acir_context.add_var(lhs, rhs),
BinaryOp::Sub => self.acir_context.sub_var(lhs, rhs),
BinaryOp::Mul => self.acir_context.mul_var(lhs, rhs),
BinaryOp::Add { .. } => self.acir_context.add_var(lhs, rhs),
BinaryOp::Sub { .. } => self.acir_context.sub_var(lhs, rhs),
BinaryOp::Mul { .. } => self.acir_context.mul_var(lhs, rhs),
BinaryOp::Div => self.acir_context.div_var(
lhs,
rhs,
Expand Down Expand Up @@ -2070,7 +2070,7 @@ impl<'a> Context<'a> {
Value::Instruction { instruction, .. } => {
if matches!(
&dfg[*instruction],
Instruction::Binary(Binary { operator: BinaryOp::Sub, .. })
Instruction::Binary(Binary { operator: BinaryOp::Sub { .. }, .. })
) {
// Subtractions must first have the integer modulus added before truncation can be
// applied. This is done in order to prevent underflow.
Expand Down Expand Up @@ -3159,7 +3159,11 @@ mod test {
let func_with_nested_call_v1 = builder.add_parameter(Type::field());

let two = builder.field_constant(2u128);
let v0_plus_two = builder.insert_binary(func_with_nested_call_v0, BinaryOp::Add, two);
let v0_plus_two = builder.insert_binary(
func_with_nested_call_v0,
BinaryOp::Add { unchecked: false },
two,
);

let foo_id = Id::test_new(2);
let foo_call = builder.import_function(foo_id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1318,9 +1318,9 @@ impl<'block> BrilligBlock<'block> {
BrilligBinaryOp::Modulo
}
}
BinaryOp::Add => BrilligBinaryOp::Add,
BinaryOp::Sub => BrilligBinaryOp::Sub,
BinaryOp::Mul => BrilligBinaryOp::Mul,
BinaryOp::Add { .. } => BrilligBinaryOp::Add,
BinaryOp::Sub { .. } => BrilligBinaryOp::Sub,
BinaryOp::Mul { .. } => BrilligBinaryOp::Mul,
BinaryOp::Eq => BrilligBinaryOp::Equals,
BinaryOp::Lt => {
if is_signed {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,14 +382,14 @@ mod test {
builder.switch_to_block(b2);

let twenty_seven = builder.field_constant(27u128);
let v7 = builder.insert_binary(v0, BinaryOp::Add, twenty_seven);
let v7 = builder.insert_binary(v0, BinaryOp::Add { unchecked: false }, twenty_seven);
builder.insert_store(v3, v7);

builder.terminate_with_jmp(b3, vec![]);

builder.switch_to_block(b1);

let v6 = builder.insert_binary(v1, BinaryOp::Add, twenty_seven);
let v6 = builder.insert_binary(v1, BinaryOp::Add { unchecked: false }, twenty_seven);
builder.insert_store(v3, v6);

builder.terminate_with_jmp(b3, vec![]);
Expand Down Expand Up @@ -501,7 +501,7 @@ mod test {

builder.switch_to_block(b2);

let v6 = builder.insert_binary(v4, BinaryOp::Mul, v4);
let v6 = builder.insert_binary(v4, BinaryOp::Mul { unchecked: false }, v4);

builder.terminate_with_jmp(b4, vec![v0]);

Expand All @@ -526,7 +526,7 @@ mod test {

let v12 = builder.insert_load(v3, Type::field());

let v13 = builder.insert_binary(v12, BinaryOp::Add, v6);
let v13 = builder.insert_binary(v12, BinaryOp::Add { unchecked: false }, v6);

builder.insert_store(v3, v13);

Expand All @@ -535,13 +535,13 @@ mod test {
builder.switch_to_block(b8);

let one = builder.field_constant(1u128);
let v15 = builder.insert_binary(v7, BinaryOp::Add, one);
let v15 = builder.insert_binary(v7, BinaryOp::Add { unchecked: false }, one);

builder.terminate_with_jmp(b4, vec![v15]);

builder.switch_to_block(b6);

let v16 = builder.insert_binary(v4, BinaryOp::Add, one);
let v16 = builder.insert_binary(v4, BinaryOp::Add { unchecked: false }, one);

builder.terminate_with_jmp(b1, vec![v16]);

Expand Down
6 changes: 3 additions & 3 deletions noir/noir-repo/compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ pub enum RuntimeError {
StaticAssertDynamicMessage { call_stack: CallStack },
#[error("Argument is dynamic")]
StaticAssertDynamicPredicate { call_stack: CallStack },
#[error("Argument is false")]
StaticAssertFailed { call_stack: CallStack },
#[error("{message}")]
StaticAssertFailed { message: String, call_stack: CallStack },
#[error("Nested slices, i.e. slices within an array or slice, are not supported")]
NestedSlice { call_stack: CallStack },
#[error("Big Integer modulus do no match")]
Expand Down Expand Up @@ -165,7 +165,7 @@ impl RuntimeError {
| RuntimeError::AssertConstantFailed { call_stack }
| RuntimeError::StaticAssertDynamicMessage { call_stack }
| RuntimeError::StaticAssertDynamicPredicate { call_stack }
| RuntimeError::StaticAssertFailed { call_stack }
| RuntimeError::StaticAssertFailed { call_stack, .. }
| RuntimeError::IntegerOutOfBounds { call_stack, .. }
| RuntimeError::UnsupportedIntegerSize { call_stack, .. }
| RuntimeError::InvalidBlackBoxInputBitSize { call_stack, .. }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,14 +251,6 @@ impl FunctionBuilder {
operator: BinaryOp,
rhs: ValueId,
) -> ValueId {
let lhs_type = self.type_of_value(lhs);
let rhs_type = self.type_of_value(rhs);
if operator != BinaryOp::Shl && operator != BinaryOp::Shr {
assert_eq!(
lhs_type, rhs_type,
"ICE - Binary instruction operands must have the same type"
);
}
let instruction = Instruction::Binary(Binary { lhs, rhs, operator });
self.insert_instruction(instruction, None).first()
}
Expand Down
19 changes: 19 additions & 0 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ impl DataFlowGraph {
if !self.is_handled_by_runtime(&instruction) {
return InsertInstructionResult::InstructionRemoved;
}

match instruction.simplify(self, block, ctrl_typevars.clone(), call_stack) {
SimplifyResult::SimplifiedTo(simplification) => {
InsertInstructionResult::SimplifiedTo(simplification)
Expand Down Expand Up @@ -525,6 +526,24 @@ impl DataFlowGraph {
}
}

/// If this value points to an array of constant bytes, returns a string
/// consisting of those bytes if they form a valid UTF-8 string.
pub(crate) fn get_string(&self, value: ValueId) -> Option<String> {
let (value_ids, _typ) = self.get_array_constant(value)?;

let mut bytes = Vec::new();
for value_id in value_ids {
let field_value = self.get_numeric_constant(value_id)?;
let u64_value = field_value.try_to_u64()?;
if u64_value > 255 {
return None;
};
let byte = u64_value as u8;
bytes.push(byte);
}
String::from_utf8(bytes).ok()
}

/// A constant index less than the array length is safe
pub(crate) fn is_safe_index(&self, index: ValueId, array: ValueId) -> bool {
#[allow(clippy::match_like_matches_macro)]
Expand Down
24 changes: 16 additions & 8 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,10 +412,15 @@ impl Instruction {

// Some binary math can overflow or underflow
Binary(binary) => match binary.operator {
BinaryOp::Add | BinaryOp::Sub | BinaryOp::Mul | BinaryOp::Div | BinaryOp::Mod => {
true
}
BinaryOp::Eq
BinaryOp::Add { unchecked: false }
| BinaryOp::Sub { unchecked: false }
| BinaryOp::Mul { unchecked: false }
| BinaryOp::Div
| BinaryOp::Mod => true,
BinaryOp::Add { unchecked: true }
| BinaryOp::Sub { unchecked: true }
| BinaryOp::Mul { unchecked: true }
| BinaryOp::Eq
| BinaryOp::Lt
| BinaryOp::And
| BinaryOp::Or
Expand Down Expand Up @@ -566,16 +571,19 @@ impl Instruction {
match self {
Instruction::Binary(binary) => {
match binary.operator {
BinaryOp::Add
| BinaryOp::Sub
| BinaryOp::Mul
BinaryOp::Add { unchecked: false }
| BinaryOp::Sub { unchecked: false }
| BinaryOp::Mul { unchecked: false }
| BinaryOp::Div
| BinaryOp::Mod => {
// Some binary math can overflow or underflow, but this is only the case
// for unsigned types (here we assume the type of binary.lhs is the same)
dfg.type_of_value(binary.rhs).is_unsigned()
}
BinaryOp::Eq
BinaryOp::Add { unchecked: true }
| BinaryOp::Sub { unchecked: true }
| BinaryOp::Mul { unchecked: true }
| BinaryOp::Eq
| BinaryOp::Lt
| BinaryOp::And
| BinaryOp::Or
Expand Down
Loading

0 comments on commit 3313783

Please sign in to comment.