Skip to content

fix(ssa): Insert checks for possible OOB array accesses during DIE#9280

Merged
TomAFrench merged 13 commits intotf/optimize-array-access-checksfrom
mv/insert-oob-check-for-array-ops
Jul 23, 2025
Merged

fix(ssa): Insert checks for possible OOB array accesses during DIE#9280
TomAFrench merged 13 commits intotf/optimize-array-access-checksfrom
mv/insert-oob-check-for-array-ops

Conversation

@vezenovm
Copy link
Copy Markdown
Contributor

@vezenovm vezenovm commented Jul 22, 2025

Description

Problem*

Builds off of #9200. Fixes failures and regressions as a result of #9200.

Alternative to #9232

Summary*

I originally had started just working off of #9232 so that is labelled as the parent as to avoid a long commit history. I'm going to transfer this over so that #9200 is the parent.

Brings back logic introduced in #5691 and deleted in #7995. #5691 was originally created to create parity between ACIR/Brillig without having massive regressions. So I thought it made sense to bring back in this scenario no that we no longer want explicit array OOB checks in SSA for ACIR.

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.

@vezenovm vezenovm changed the title bring back OOB array insertion to DIE fix(ssa): Insert checks for possible OOB array accesses during DIE Jul 22, 2025
Base automatically changed from mv/array-get-side-effects to tf/optimize-array-access-checks July 22, 2025 19:42
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 22, 2025

Changes to Brillig bytecode sizes

Generated at commit: db11592eb16bbd7f00dd5fa4b96054ce87eab22c, compared to commit: 45a3f08703f314eb1031053b3c0481fe7934688f

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
brillig_rc_regression_6123_inliner_min -30 ✅ -17.34%
a_6_inliner_min -37 ✅ -20.33%
a_6_inliner_max -37 ✅ -21.51%
a_6_inliner_zero -37 ✅ -21.51%
regression_9206_inliner_max -48 ✅ -60.76%
regression_9206_inliner_min -48 ✅ -60.76%
regression_9206_inliner_zero -48 ✅ -60.76%

Full diff report 👇
Program Brillig opcodes (+/-) %
uhashmap_inliner_min 7,284 (-20) -0.27%
hashmap_inliner_min 8,787 (-26) -0.30%
brillig_cow_regression_inliner_min 1,212 (-4) -0.33%
brillig_cow_regression_inliner_max 1,202 (-4) -0.33%
brillig_cow_regression_inliner_zero 1,202 (-4) -0.33%
uhashmap_inliner_zero 6,869 (-25) -0.36%
hashmap_inliner_zero 7,724 (-30) -0.39%
uhashmap_inliner_max 11,628 (-56) -0.48%
poseidon_bn254_hash_width_3_inliner_max 4,858 (-30) -0.61%
poseidon_bn254_hash_width_3_inliner_min 4,742 (-30) -0.63%
poseidon_bn254_hash_width_3_inliner_zero 4,445 (-30) -0.67%
regression_5252_inliner_min 3,384 (-30) -0.88%
hashmap_inliner_max 17,413 (-160) -0.91%
regression_5252_inliner_zero 3,230 (-30) -0.92%
poseidonsponge_x5_254_inliner_min 3,019 (-30) -0.98%
lambda_from_array_inliner_max 2,484 (-25) -1.00%
lambda_from_array_inliner_min 2,484 (-25) -1.00%
lambda_from_array_inliner_zero 2,484 (-25) -1.00%
poseidonsponge_x5_254_inliner_zero 2,898 (-30) -1.02%
slices_inliner_min 2,168 (-29) -1.32%
regression_5252_inliner_max 3,972 (-60) -1.49%
poseidonsponge_x5_254_inliner_max 3,687 (-60) -1.60%
slices_inliner_max 1,691 (-32) -1.86%
slices_inliner_zero 1,650 (-32) -1.90%
wildcard_type_inliner_min 275 (-7) -2.48%
wildcard_type_inliner_max 265 (-7) -2.57%
wildcard_type_inliner_zero 265 (-7) -2.57%
brillig_constant_reference_regression_inliner_max 79 (-3) -3.66%
brillig_constant_reference_regression_inliner_min 79 (-3) -3.66%
brillig_constant_reference_regression_inliner_zero 79 (-3) -3.66%
brillig_nested_arrays_inliner_min 171 (-7) -3.93%
brillig_nested_arrays_inliner_zero 171 (-7) -3.93%
nested_array_in_slice_inliner_min 814 (-35) -4.12%
nested_array_in_slice_inliner_zero 814 (-35) -4.12%
array_dynamic_nested_blackbox_input_inliner_min 328 (-15) -4.37%
array_dynamic_nested_blackbox_input_inliner_max 318 (-15) -4.50%
array_dynamic_nested_blackbox_input_inliner_zero 318 (-15) -4.50%
nested_array_dynamic_inliner_min 1,428 (-75) -4.99%
nested_array_dynamic_inliner_zero 1,428 (-75) -4.99%
nested_array_dynamic_inliner_max 1,724 (-93) -5.12%
brillig_nested_arrays_inliner_max 126 (-7) -5.26%
regression_11294_inliner_max 221 (-14) -5.96%
regression_11294_inliner_zero 221 (-14) -5.96%
nested_array_in_slice_inliner_max 880 (-90) -9.28%
regression_struct_array_conditional_inliner_max 384 (-42) -9.86%
regression_struct_array_conditional_inliner_min 384 (-42) -9.86%
regression_struct_array_conditional_inliner_zero 384 (-42) -9.86%
conditional_regression_short_circuit_inliner_min 235 (-37) -13.60%
conditional_regression_short_circuit_inliner_max 208 (-37) -15.10%
conditional_regression_short_circuit_inliner_zero 208 (-37) -15.10%
brillig_rc_regression_6123_inliner_min 143 (-30) -17.34%
a_6_inliner_min 145 (-37) -20.33%
a_6_inliner_max 135 (-37) -21.51%
a_6_inliner_zero 135 (-37) -21.51%
regression_9206_inliner_max 31 (-48) -60.76%
regression_9206_inliner_min 31 (-48) -60.76%
regression_9206_inliner_zero 31 (-48) -60.76%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 22, 2025

Changes to number of Brillig opcodes executed

Generated at commit: db11592eb16bbd7f00dd5fa4b96054ce87eab22c, compared to commit: 45a3f08703f314eb1031053b3c0481fe7934688f

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
regression_struct_array_conditional_inliner_max -97 ✅ -7.91%
regression_struct_array_conditional_inliner_min -97 ✅ -7.91%
regression_struct_array_conditional_inliner_zero -97 ✅ -7.91%
brillig_rc_regression_6123_inliner_min -108 ✅ -34.62%
regression_9206_inliner_max -48 ✅ -65.75%
regression_9206_inliner_min -48 ✅ -65.75%
regression_9206_inliner_zero -48 ✅ -65.75%

Full diff report 👇
Program Brillig opcodes (+/-) %
brillig_cow_regression_inliner_min 194,580 (-64) -0.03%
brillig_cow_regression_inliner_max 194,345 (-64) -0.03%
brillig_cow_regression_inliner_zero 194,345 (-64) -0.03%
uhashmap_inliner_min 176,694 (-230) -0.13%
uhashmap_inliner_zero 168,780 (-264) -0.16%
hashmap_inliner_min 73,352 (-228) -0.31%
slices_inliner_min 3,805 (-13) -0.34%
hashmap_inliner_zero 66,795 (-248) -0.37%
uhashmap_inliner_max 144,538 (-704) -0.48%
slices_inliner_zero 2,757 (-16) -0.58%
slices_inliner_max 2,490 (-16) -0.64%
hashmap_inliner_max 51,954 (-440) -0.84%
poseidon_bn254_hash_width_3_inliner_min 171,524 (-1,580) -0.91%
poseidon_bn254_hash_width_3_inliner_zero 167,819 (-1,580) -0.93%
poseidon_bn254_hash_width_3_inliner_max 137,407 (-1,580) -1.14%
regression_11294_inliner_max 1,126 (-14) -1.23%
regression_11294_inliner_zero 1,126 (-14) -1.23%
array_dynamic_nested_blackbox_input_inliner_min 1,195 (-15) -1.24%
array_dynamic_nested_blackbox_input_inliner_max 1,181 (-15) -1.25%
array_dynamic_nested_blackbox_input_inliner_zero 1,181 (-15) -1.25%
lambda_from_array_inliner_max 1,684 (-25) -1.46%
lambda_from_array_inliner_min 1,684 (-25) -1.46%
lambda_from_array_inliner_zero 1,684 (-25) -1.46%
poseidonsponge_x5_254_inliner_min 188,782 (-3,160) -1.65%
regression_5252_inliner_min 942,355 (-15,800) -1.65%
poseidonsponge_x5_254_inliner_zero 186,877 (-3,160) -1.66%
regression_5252_inliner_zero 930,416 (-15,800) -1.67%
nested_array_dynamic_inliner_min 2,877 (-54) -1.84%
nested_array_dynamic_inliner_zero 2,877 (-54) -1.84%
nested_array_in_slice_inliner_min 1,326 (-27) -2.00%
nested_array_in_slice_inliner_zero 1,326 (-27) -2.00%
poseidonsponge_x5_254_inliner_max 152,998 (-3,160) -2.02%
regression_5252_inliner_max 760,524 (-15,800) -2.04%
conditional_regression_short_circuit_inliner_min 973 (-21) -2.11%
conditional_regression_short_circuit_inliner_max 934 (-21) -2.20%
conditional_regression_short_circuit_inliner_zero 934 (-21) -2.20%
a_6_inliner_min 872 (-21) -2.35%
a_6_inliner_max 858 (-21) -2.39%
a_6_inliner_zero 858 (-21) -2.39%
nested_array_dynamic_inliner_max 2,649 (-69) -2.54%
brillig_constant_reference_regression_inliner_max 79 (-3) -3.66%
brillig_constant_reference_regression_inliner_min 79 (-3) -3.66%
brillig_constant_reference_regression_inliner_zero 79 (-3) -3.66%
brillig_nested_arrays_inliner_min 165 (-7) -4.07%
brillig_nested_arrays_inliner_zero 165 (-7) -4.07%
brillig_nested_arrays_inliner_max 116 (-7) -5.69%
wildcard_type_inliner_min 407 (-28) -6.44%
wildcard_type_inliner_max 393 (-28) -6.65%
wildcard_type_inliner_zero 393 (-28) -6.65%
nested_array_in_slice_inliner_max 1,072 (-78) -6.78%
regression_struct_array_conditional_inliner_max 1,130 (-97) -7.91%
regression_struct_array_conditional_inliner_min 1,130 (-97) -7.91%
regression_struct_array_conditional_inliner_zero 1,130 (-97) -7.91%
brillig_rc_regression_6123_inliner_min 204 (-108) -34.62%
regression_9206_inliner_max 25 (-48) -65.75%
regression_9206_inliner_min 25 (-48) -65.75%
regression_9206_inliner_zero 25 (-48) -65.75%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 22, 2025

Changes to circuit sizes

Generated at commit: db11592eb16bbd7f00dd5fa4b96054ce87eab22c, compared to commit: 45a3f08703f314eb1031053b3c0481fe7934688f

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
lambda_from_array +1 ❌ +0.12% +2,721 ❌ +459.63%
no_predicates_numeric_generic_poseidon -171 ✅ -76.34% -1,130 ✅ -48.33%
a_6 -1 ✅ -1.41% -2,051 ✅ -48.46%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
lambda_from_array 809 (+1) +0.12% 3,313 (+2,721) +459.63%
array_oob_regression_7965 31 (+6) +24.00% 2,843 (+12) +0.42%
databus_composite_calldata 133 (+4) +3.10% 3,096 (+4) +0.13%
sha512_100_bytes 13,173 (+48) +0.37% 39,458 (+49) +0.12%
regression_7612 15 (-2) -11.76% 2,780 (-3) -0.11%
hashmap 31,550 (-512) -1.60% 92,728 (-1,052) -1.12%
array_dynamic_nested_blackbox_input 130 (-24) -15.58% 5,172 (-67) -1.28%
regression_struct_array_conditional 66 (-32) -32.65% 3,196 (-48) -1.48%
nested_array_dynamic 3,234 (-202) -5.88% 12,610 (-395) -3.04%
nested_array_in_slice 958 (-94) -8.94% 5,462 (-201) -3.55%
regression_capacity_tracker 53 (-79) -59.85% 3,725 (-194) -4.95%
conditional_1 2,636 (-136) -4.91% 7,999 (-626) -7.26%
regression_5252 24,795 (-1,929) -7.22% 75,817 (-6,434) -7.82%
slice_dynamic_index 808 (-161) -16.62% 5,073 (-464) -8.38%
slices 670 (-71) -9.58% 3,769 (-370) -8.94%
conditional_regression_short_circuit 89 (-1) -1.11% 4,942 (-2,051) -29.33%
fold_2_to_17 6,214 (-15,540) -71.44% 124,332 (-104,110) -45.57%
bench_2_to_17 6,534 (-16,340) -71.43% 130,733 (-109,470) -45.57%
no_predicates_numeric_generic_poseidon 53 (-171) -76.34% 1,208 (-1,130) -48.33%
a_6 70 (-1) -1.41% 2,181 (-2,051) -48.46%

Copy link
Copy Markdown
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: cbc53e1 Previous: ad16599 Ratio
sha512-100-bytes 0.104 s 0.056 s 1.86

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

CC: @TomAFrench

Copy link
Copy Markdown
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: cbc53e1 Previous: ad16599 Ratio
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

@vezenovm vezenovm marked this pull request as ready for review July 22, 2025 20:14
@vezenovm vezenovm requested review from TomAFrench and jfecher July 22, 2025 20:15
@vezenovm
Copy link
Copy Markdown
Contributor Author

Some last arb tests are failing, currently investigating.

vezenovm and others added 2 commits July 23, 2025 08:46
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
@TomAFrench TomAFrench merged commit 81409f2 into tf/optimize-array-access-checks Jul 23, 2025
96 of 97 checks passed
@TomAFrench TomAFrench deleted the mv/insert-oob-check-for-array-ops branch July 23, 2025 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants