Skip to content

feat: only inject "out of bounds" checks in brillig#9200

Merged
vezenovm merged 15 commits intomasterfrom
tf/optimize-array-access-checks
Jul 23, 2025
Merged

feat: only inject "out of bounds" checks in brillig#9200
vezenovm merged 15 commits intomasterfrom
tf/optimize-array-access-checks

Conversation

@TomAFrench
Copy link
Member

@TomAFrench TomAFrench commented Jul 14, 2025

Description

Problem*

Resolves

Summary*

This is a refinement to #7827 which omits the out-of-bounds check in situations where we can rely on the implicit range check from the underlying memory block to prevent out-of-bounds accesses

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@TomAFrench TomAFrench changed the title Tf/optimize array access checks feat: only inject "out of bounds" checks in brillig Jul 14, 2025
@TomAFrench TomAFrench changed the title feat: only inject "out of bounds" checks in brillig feat: only inject "out of bounds" checks in brillig Jul 14, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jul 14, 2025

Changes to circuit sizes

Generated at commit: ceda978354c33416de147286552b6ef256b475f4, compared to commit: d2a51b685ace63a2251d3e0edfc5cda1f034dbef

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
nested_array_dynamic_simple +15 ❌ +1500.00% +2,770 ❌ +16294.12%
lambda_from_array +4 ❌ +0.50% +2,746 ❌ +484.30%
array_sort -24 ✅ -32.88% -2,765 ✅ -95.71%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
nested_array_dynamic_simple 16 (+15) +1500.00% 2,787 (+2,770) +16294.12%
lambda_from_array 809 (+4) +0.50% 3,313 (+2,746) +484.30%
array_oob_regression_7965 31 (+2) +6.90% 2,843 (+8) +0.28%
nested_array_dynamic 3,234 (+4) +0.12% 12,610 (+4) +0.03%
databus_two_calldata_simple 14 (-1) -6.67% 2,794 (-1) -0.04%
regression_7612 15 (-1) -6.25% 2,780 (-1) -0.04%
brillig_uninitialized_arrays 25 (-2) -7.41% 3,510 (-2) -0.06%
regression_struct_array_conditional 66 (-4) -5.71% 3,196 (-3) -0.09%
regression_8236 18 (-4) -18.18% 2,797 (-4) -0.14%
databus_composite_calldata 133 (-6) -4.32% 3,096 (-6) -0.19%
databus_two_calldata 44 (-12) -21.43% 2,885 (-11) -0.38%
array_dynamic 106 (-23) -17.83% 3,720 (-21) -0.56%
hashmap 31,550 (-782) -2.42% 92,728 (-882) -0.94%
nested_dyn_array_regression_5782 27 (-1) -3.57% 99 (-1) -1.00%
conditional_1 2,636 (-118) -4.28% 7,999 (-141) -1.73%
lambda_from_global_array 9 (-1) -10.00% 30 (-1) -3.23%
regression_5252 24,795 (-1,383) -5.28% 75,817 (-3,985) -4.99%
sha512_100_bytes 13,173 (-8,996) -40.58% 39,458 (-11,518) -22.59%
array_sort 49 (-24) -32.88% 124 (-2,765) -95.71%

@TomAFrench TomAFrench requested a review from a team July 15, 2025 12:30
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 81409f2 Previous: d2a51b6 Ratio
sha512-100-bytes 0.105 s 0.054 s 1.94

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 81409f2 Previous: d2a51b6 Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr 5 s 4 s 1.25
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_reset-kernel-lib 2 s 1 s 2
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types 2 s 1 s 2

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@TomAFrench
Copy link
Member Author

There's a couple of outstanding issues here:

  • unused_array_get_known_index_out_of_bounds is now passing rather than halting. It looks like the array access is being removed in DIE rather than being replaced with an out-of-bounds check.
  • regression_8774 fails with "not expected to have Load or Store instruction after DIE in an ACIR function". This is due to the out-of-bounds check which previously constrained the index to be 0 being removed so we're not resolving the reference immediately like we were before.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Opcode count'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.10.

Benchmark suite Current: d8bbbf6 Previous: e7a98f2 Ratio
private-kernel-reset 82510 opcodes 68865 opcodes 1.20

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@TomAFrench
Copy link
Member Author

Merging master into this branch atm.

vezenovm and others added 2 commits July 22, 2025 20:42
…9232)

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 22, 2025

Changes to Brillig bytecode sizes

Generated at commit: ceda978354c33416de147286552b6ef256b475f4, compared to commit: d2a51b685ace63a2251d3e0edfc5cda1f034dbef

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
slice_regex_inliner_zero -23 ✅ -1.42%
slice_regex_inliner_max -168 ✅ -8.43%

Full diff report 👇
Program Brillig opcodes (+/-) %
hashmap_inliner_max 17,413 (+95) +0.55%
slice_regex_inliner_zero 1,600 (-23) -1.42%
slice_regex_inliner_max 1,824 (-168) -8.43%

@github-actions
Copy link
Contributor

github-actions bot commented Jul 22, 2025

Changes to number of Brillig opcodes executed

Generated at commit: ceda978354c33416de147286552b6ef256b475f4, compared to commit: d2a51b685ace63a2251d3e0edfc5cda1f034dbef

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
hashmap_inliner_max +303 ❌ +0.59%
slice_regex_inliner_max -98 ✅ -3.40%

Full diff report 👇
Program Brillig opcodes (+/-) %
hashmap_inliner_max 51,954 (+303) +0.59%
slice_regex_inliner_zero 3,859 (-22) -0.57%
slice_regex_inliner_max 2,782 (-98) -3.40%

@vezenovm vezenovm added the bench-show Display benchmark results on PR label Jul 22, 2025
TomAFrench and others added 2 commits July 23, 2025 14:24
…9280)

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACVM Benchmarks

Details
Benchmark suite Current: 81409f2 Previous: d2a51b6 Ratio
purely_sequential_opcodes 250085 ns/iter (± 1085) 248720 ns/iter (± 479) 1.01
perfectly_parallel_opcodes 220057 ns/iter (± 4321) 218678 ns/iter (± 3906) 1.01
perfectly_parallel_batch_inversion_opcodes 2779819 ns/iter (± 2330) 2779148 ns/iter (± 1982) 1.00

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Execution Time

Details
Benchmark suite Current: 81409f2 Previous: d2a51b6 Ratio
semaphore-depth-10 0.02 s 0.019 s 1.05
sha512-100-bytes 0.105 s 0.054 s 1.94

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Artifact Size

Details
Benchmark suite Current: 81409f2 Previous: d2a51b6 Ratio
semaphore-depth-10 632.4 KB 632.4 KB 1
sha512-100-bytes 525.2 KB 622.9 KB 0.84

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opcode count

Details
Benchmark suite Current: 81409f2 Previous: d2a51b6 Ratio
semaphore-depth-10 5763 opcodes 5763 opcodes 1
sha512-100-bytes 13173 opcodes 22169 opcodes 0.59

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compilation Time

Details
Benchmark suite Current: 81409f2 Previous: d2a51b6 Ratio
semaphore-depth-10 0.783 s 0.777 s 1.01
sha512-100-bytes 1.594 s 1.861 s 0.86

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Execution Memory

Details
Benchmark suite Current: 81409f2 Previous: d2a51b6 Ratio
semaphore_depth_10 69.3 MB 69.3 MB 1
sha512_100_bytes 54.82 MB 55.56 MB 0.99

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compilation Memory

Details
Benchmark suite Current: 81409f2 Previous: d2a51b6 Ratio
semaphore_depth_10 104.55 MB 104.57 MB 1.00
sha512_100_bytes 234.59 MB 255.65 MB 0.92

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test Suite Duration

Details
Benchmark suite Current: 81409f2 Previous: d2a51b6 Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr 5 s 4 s 1.25
test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts 44 s 45 s 0.98
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 2 s 3 s 0.67
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib 1 s 2 s 0.50
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_reset-kernel-lib 2 s 1 s 2
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib 2 s 4 s 0.50
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types 2 s 1 s 2
test_report_noir-lang_noir-bignum_ 411 s 376 s 1.09
test_report_noir-lang_noir_bigcurve_ 299 s 286 s 1.05
test_report_noir-lang_sha512_ 15 s 16 s 0.94
test_report_zkpassport_noir-ecdsa_ 1 s 3 s 0.33
test_report_zkpassport_noir_rsa_ 2 s 2 s 1

This comment was automatically generated by workflow using github-action-benchmark.

@vezenovm vezenovm added this pull request to the merge queue Jul 23, 2025
Merged via the queue into master with commit 60222bb Jul 23, 2025
101 checks passed
@vezenovm vezenovm deleted the tf/optimize-array-access-checks branch July 23, 2025 15:10
@asterite asterite mentioned this pull request Jul 30, 2025
5 tasks
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 4, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: forbid self-referencing type aliases
(noir-lang/noir#9103)
chore: add a mem2reg test for when all references need to be invalidated
(noir-lang/noir#9377)
fix(ssa): Do not check ArrayGet/Set as unreachable for Brillig
(noir-lang/noir#9376)
chore: use SSA parser in all mem2reg tests
(noir-lang/noir#9372)
fix: trait where clause check fixes
(noir-lang/noir#9369)
fix: Correct doc comments for SSA passes
(noir-lang/noir#9371)
fix: prevent `SignedField::from(i128::MIN)` from crashing
(noir-lang/noir#9366)
fix: allow constants in the type-system to be negative
(noir-lang/noir#9360)
feat: show circuit output as a value of the program's return type
(noir-lang/noir#9364)
feat: add `FunctionDefinition::visibility`
(noir-lang/noir#9363)
chore(docs): Add example for `$crate` in docs
(noir-lang/noir#9361)
fix: Prevent accidental tuple sharing in comptime code
(noir-lang/noir#9313)
fix: perserve purities after SSA normalization
(noir-lang/noir#9355)
fix: modulo overflow in comptime
(noir-lang/noir#9348)
fix: handle short-syntax for trait constraints on trait generics
(noir-lang/noir#9167)
chore: enhance trait constraint comment
(noir-lang/noir#9358)
fix: replace implicitly added named generics with fresh type vars in
check_trait_impl_where_clause_matches_trait_where_clause
(noir-lang/noir#9352)
fix: push definition trait constraints after trait item constraint
(noir-lang/noir#9354)
chore(ci): Update status of noir_json_parser
(noir-lang/noir#9351)
fix(ssa): Keep reference count increments for array set values
(noir-lang/noir#9344)
chore: remove unused `compile_workspace`
(noir-lang/noir#9353)
chore: try printing byte arrays as strings in the SSA interpreter
(noir-lang/noir#9346)
feat(lsp): allow opening noir stdlib files
(noir-lang/noir#9339)
fix: do u128 operations with u128, not i128
(noir-lang/noir#9345)
chore(acir): ACIR parser error handling for blackbox inputs/outputs
(noir-lang/noir#9342)
fix: prevent invalid types in test/fuzz functions
(noir-lang/noir#9343)
chore(lsp): avoid redundant type checking
(noir-lang/noir#9337)
feat(acir): Parse ACIR memory and call opcodes
(noir-lang/noir#9331)
fix(ssa_gen): Add constraint on slice length before popping
(noir-lang/noir#9323)
chore: impl for u16 conversions
(noir-lang/noir#9314)
fix: substitute bindings in type before canonicalization
(noir-lang/noir#9328)
fix(ssa_interpreter): `push_back` and `pop_back` to slices with padding
(noir-lang/noir#9320)
fix: wildcard type should be allowed in lambda parameter types
(noir-lang/noir#9325)
chore: graceful handling of SIGPIPE
(noir-lang/noir#9075)
feat: return unsolvable opcode from `CircuitSimulator`
(noir-lang/noir#8943)
fix: allow nested fmtstr (noir-lang/noir#9309)
feat: Initial ACIR parser (arithmetic exprs and black box functions)
(noir-lang/noir#9316)
fix(mem2reg): Register aliases when the `IfElse` result in a reference
(noir-lang/noir#9305)
fix: Make Ssa-gen use existing reference when compiling `&mut
foo.bar.baz` (noir-lang/noir#9307)
fix: top-level item in dependency isn't always visible
(noir-lang/noir#9295)
fix(ssa-interpreter): Return error if slice length is 0 during popping
(noir-lang/noir#9308)
chore: Release Noir(1.0.0-beta.9)
(noir-lang/noir#9184)
chore(LSP): simplify code lens request handling
(noir-lang/noir#9279)
chore: add regression tests for #6383
(noir-lang/noir#9302)
fix: disallow `_` in signatures and struct members
(noir-lang/noir#9301)
fix: check associated types after validating where clause when looking
up trait impls, plus some unification fixes
(noir-lang/noir#9265)
chore: Add fmtstr to coercions list
(noir-lang/noir#9300)
chore: Add a helper function `fmtstr::as_quoted_str`
(noir-lang/noir#9293)
chore(docs): Copy Type Coercions docs into v1.0.0-beta.8 versioned docs
(noir-lang/noir#9298)
feat: only inject "out of bounds" checks in brillig
(noir-lang/noir#9200)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bench-show Display benchmark results on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants