Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: optimise older opcodes in reverse order #6476

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

guipublic
Copy link
Contributor

Description

Problem*

Related to #6451 and #6461

Summary*

Handle the case where we can optimise an older opcode

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.

Copy link
Contributor

github-actions bot commented Nov 7, 2024

Changes to circuit sizes

Generated at commit: 8694821c0149848b8251e5bfd208c85e0fe40d73, compared to commit: 78e784879716e1a8372fea06cdf7dc6df3b04339

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
to_bytes_consistent +45 ❌ +2250.00% +45 ❌ +264.71%
poseidon_bn254_hash_width_3 +596 ❌ +66.82% +467 ❌ +45.03%
simple_radix +3 ❌ +150.00% +3 ❌ +17.65%
hashmap +32,337 ❌ +50.40% +16,216 ❌ +11.75%
to_bytes_integration +143 ❌ +50.00% +50 ❌ +11.19%
eddsa +5,665 ❌ +8.77% +4,601 ❌ +6.99%
to_be_bytes +14 ❌ +20.00% +8 ❌ +6.84%
array_dynamic_main_output -11 ✅ -25.00% -42 ✅ -19.00%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
to_bytes_consistent 47 (+45) +2250.00% 62 (+45) +264.71%
poseidon_bn254_hash_width_3 1,488 (+596) +66.82% 1,504 (+467) +45.03%
simple_radix 5 (+3) +150.00% 20 (+3) +17.65%
hashmap 96,492 (+32,337) +50.40% 154,175 (+16,216) +11.75%
to_bytes_integration 429 (+143) +50.00% 497 (+50) +11.19%
eddsa 70,294 (+5,665) +8.77% 70,416 (+4,601) +6.99%
to_be_bytes 84 (+14) +20.00% 125 (+8) +6.84%
bench_eddsa_poseidon 17,922 (+1,483) +9.02% 20,762 (+1,176) +6.00%
poseidonsponge_x5_254 1,302 (+262) +25.19% 1,318 (+63) +5.02%
regression_5252 43,422 (+8,826) +25.51% 50,165 (+2,202) +4.59%
regression_6451 17 (+1) +6.25% 29 (+1) +3.57%
bit_shifts_runtime 1,212 (+326) +36.79% 4,849 (+151) +3.21%
poseidon_bn254_hash 1,053 (+161) +18.05% 1,069 (+32) +3.09%
to_le_bytes 83 (+10) +13.70% 121 (+3) +2.54%
nested_array_dynamic 3,822 (+627) +19.62% 13,258 (+317) +2.45%
slice_dynamic_index 1,199 (+212) +21.48% 6,564 (+106) +1.64%
nested_array_in_slice 1,041 (+148) +16.57% 5,587 (+74) +1.34%
7_function 234 (+78) +50.00% 3,007 (+37) +1.25%
array_len 17 (+3) +21.43% 82 (+1) +1.23%
conditional_1 4,403 (+247) +5.94% 12,062 (+136) +1.14%
binary_operator_overloading 383 (+101) +35.82% 4,512 (+49) +1.10%
sha256_var_size_regression 17,789 (+1,628) +10.07% 71,739 (+778) +1.10%
slices 919 (+78) +9.27% 3,994 (+41) +1.04%
u128 782 (+101) +14.83% 4,734 (+44) +0.94%
u16_support 203 (+54) +36.24% 3,016 (+25) +0.84%
sha256_regression 36,275 (+1,182) +3.37% 198,509 (+1,046) +0.53%
hash_to_field 613 (+31) +5.33% 3,629 (+11) +0.30%
regression 158 (+16) +11.27% 3,694 (+10) +0.27%
bench_poseidon_hash_100 40,700 (+3,100) +8.24% 40,716 (+100) +0.25%
bench_poseidon_hash_30 12,210 (+930) +8.24% 12,226 (+30) +0.25%
bench_poseidon_hash 407 (+31) +8.24% 423 (+1) +0.24%
ram_blowup_regression 177,643 (+4,119) +2.37% 678,406 (+1,541) +0.23%
schnorr 1,639 (+120) +7.90% 23,981 (+49) +0.20%
sha256_var_witness_const_regression 1,770 (+93) +5.55% 17,251 (+33) +0.19%
sha256 1,984 (+102) +5.42% 21,345 (+34) +0.16%
bigint 1,185 (+19) +1.63% 8,124 (+7) +0.09%
6_array 415 (+2) +0.48% 4,055 (+3) +0.07%
regression_3607 43 (+4) +10.26% 2,804 (+2) +0.07%
regression_4709 72 (+2) +2.86% 2,919 (+2) +0.07%
array_if_cond_simple 111 (+3) +2.78% 3,134 (+2) +0.06%
conditional_2 22 (+1) +4.76% 2,782 (+1) +0.04%
arithmetic_binary_operations 18 (+2) +12.50% 2,789 (+1) +0.04%
regression_struct_array_conditional 72 (+1) +1.41% 3,204 (+1) +0.03%
regression_mem_op_predicate 58 (+2) +3.57% 3,587 (+1) +0.03%
regression_capacity_tracker 115 (+2) +1.77% 3,965 (+1) +0.03%
regression_4449 9,177 (+630) +7.37% 285,354 (+70) +0.02%
bench_sha256_100 18,409 (+900) +5.14% 408,699 (+100) +0.02%
bench_sha256_30 5,529 (+270) +5.13% 124,711 (+30) +0.02%
keccak256 137 (+12) +9.60% 20,151 (+4) +0.02%
6 364 (+18) +5.20% 11,132 (+2) +0.02%
conditional_regression_short_circuit 392 (+18) +4.81% 11,161 (+2) +0.02%
bench_sha256 193 (+9) +4.89% 7,056 (+1) +0.01%
sha2_byte 15,973 (+33) +0.21% 82,690 (+9) +0.01%
array_dynamic_blackbox_input 1,380 (+18) +1.32% 21,408 (+2) +0.01%
bench_sha256_long 882 (+9) +1.03% 19,467 (+1) +0.01%
sha256_var_padding_regression 3,872 (+54) +1.41% 193,947 (+6) +0.00%
ecdsa_secp256k1 499 (+9) +1.84% 43,519 (+1) +0.00%
integer_array_indexing 3 (+1) +50.00% 2,762 (0) 0.00%
merkle_insert 66 (+12) +22.22% 3,684 (0) 0.00%
pedersen_check 24 (+1) +4.35% 3,176 (0) 0.00%
signed_arithmetic 189 (+1) +0.53% 2,937 (0) 0.00%
signed_div 510 (+15) +3.03% 538 (0) 0.00%
signed_division 207 (+4) +1.97% 3,678 (0) 0.00%
simple_shield 53 (+6) +12.77% 4,872 (0) 0.00%
slice_loop 2 (+1) +100.00% 18 (0) 0.00%
nested_dyn_array_regression_5782 32 (-3) -8.57% 2,859 (-4) -0.14%
array_dynamic 107 (-1) -0.93% 3,716 (-15) -0.40%
aes128_encrypt 143 (+16) +12.60% 1,623 (-20) -1.22%
array_dynamic_nested_blackbox_input 256 (-19) -6.91% 7,316 (-91) -1.23%
array_dynamic_main_output 33 (-11) -25.00% 179 (-42) -19.00%

@guipublic
Copy link
Contributor Author

The increase in circuit size is not expected. This is worrying and we should not merge the PR which is supposed to improve (only a little bit but still) this number.

Copy link
Contributor

Changes to Brillig bytecode sizes

Generated at commit: 8694821c0149848b8251e5bfd208c85e0fe40d73, compared to commit: 78e784879716e1a8372fea06cdf7dc6df3b04339

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
brillig_array_to_slice +24 ❌ +26.67%
reference_only_used_as_alias +33 ❌ +13.31%
pedersen_hash +24 ❌ +6.70%
regression_struct_array_conditional +30 ❌ +5.73%
merkle_insert +36 ❌ +4.63%
brillig_pedersen +27 ❌ +3.62%
pedersen_check +27 ❌ +3.62%

Full diff report 👇
Program Brillig opcodes (+/-) %
brillig_array_to_slice 114 (+24) +26.67%
reference_only_used_as_alias 281 (+33) +13.31%
pedersen_hash 382 (+24) +6.70%
regression_struct_array_conditional 554 (+30) +5.73%
merkle_insert 814 (+36) +4.63%
brillig_pedersen 772 (+27) +3.62%
pedersen_check 772 (+27) +3.62%
simple_shield 897 (+27) +3.10%
array_to_slice 1,043 (+30) +2.96%
schnorr 1,492 (+36) +2.47%
fold_complex_outputs 573 (+12) +2.14%
import 149 (+3) +2.05%
array_sort 302 (+6) +2.03%
sha2_byte 3,388 (+66) +1.99%
brillig_rc_regression_6123 175 (+3) +1.74%
hashmap 26,656 (+411) +1.57%
nested_array_dynamic 2,181 (+33) +1.54%
pedersen_commitment 200 (+3) +1.52%
brillig_cow_regression 2,410 (+30) +1.26%
databus_two_calldata 270 (+3) +1.12%
nested_array_in_slice 1,202 (+12) +1.01%
array_dynamic 320 (+3) +0.95%
conditional_1 1,337 (+12) +0.91%
regression_capacity_tracker 231 (+2) +0.87%
regression 729 (+6) +0.83%
slice_dynamic_index 2,736 (+22) +0.81%
keccak256 1,778 (+12) +0.68%
brillig_keccak 1,778 (+12) +0.68%
uhashmap 16,310 (+102) +0.63%
array_dynamic_nested_blackbox_input 994 (+6) +0.61%
array_dynamic_blackbox_input 1,176 (+6) +0.51%
6 1,367 (+6) +0.44%
sha256 2,053 (+9) +0.44%
sha256_var_witness_const_regression 1,403 (+6) +0.43%
conditional_regression_short_circuit 1,437 (+6) +0.42%
no_predicates_numeric_generic_poseidon 770 (+3) +0.39%
fold_numeric_generic_poseidon 770 (+3) +0.39%
brillig_sha256 796 (+3) +0.38%
regression_4449 859 (+3) +0.35%
sha256_var_padding_regression 5,248 (+18) +0.34%
ram_blowup_regression 1,009 (+3) +0.30%
ecdsa_secp256k1 1,013 (+3) +0.30%
sha256_var_size_regression 2,046 (+6) +0.29%
strings 1,042 (+3) +0.29%
sha256_regression 7,272 (+18) +0.25%
poseidonsponge_x5_254 4,502 (+6) +0.13%
regression_5252 4,879 (+6) +0.12%
poseidon_bn254_hash_width_3 5,692 (+6) +0.11%
poseidon_bn254_hash 5,692 (+6) +0.11%
u128 2,911 (+3) +0.10%
eddsa 10,894 (+9) +0.08%

Copy link
Contributor

Changes to number of Brillig opcodes executed

Generated at commit: 8694821c0149848b8251e5bfd208c85e0fe40d73, compared to commit: 78e784879716e1a8372fea06cdf7dc6df3b04339

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
reference_only_used_as_alias +163 ❌ +66.53%
regression_capacity_tracker +336 ❌ +58.95%
brillig_array_to_slice +31 ❌ +37.80%
regression_struct_array_conditional +270 ❌ +20.87%
pedersen_hash +99 ❌ +17.22%
merkle_insert +486 ❌ +14.48%

Full diff report 👇
Program Brillig opcodes (+/-) %
reference_only_used_as_alias 408 (+163) +66.53%
regression_capacity_tracker 906 (+336) +58.95%
brillig_array_to_slice 113 (+31) +37.80%
regression_struct_array_conditional 1,564 (+270) +20.87%
pedersen_hash 674 (+99) +17.22%
merkle_insert 3,842 (+486) +14.48%
simple_shield 2,885 (+306) +11.87%
sha2_byte 52,231 (+5,049) +10.70%
brillig_pedersen 1,252 (+117) +10.31%
pedersen_check 1,252 (+117) +10.31%
array_sort 538 (+36) +7.17%
pedersen_commitment 296 (+18) +6.47%
import 153 (+9) +6.25%
array_to_slice 2,475 (+133) +5.68%
fold_complex_outputs 889 (+36) +4.22%
hashmap 58,421 (+2,079) +3.69%
brillig_rc_regression_6123 289 (+9) +3.21%
slice_dynamic_index 4,355 (+132) +3.13%
array_dynamic_blackbox_input 19,955 (+594) +3.07%
schnorr 10,396 (+216) +2.12%
uhashmap 153,618 (+2,796) +1.85%
databus_two_calldata 500 (+9) +1.83%
nested_array_dynamic 3,116 (+54) +1.76%
nested_array_in_slice 1,469 (+24) +1.66%
array_dynamic_nested_blackbox_input 4,629 (+45) +0.98%
regression 2,703 (+18) +0.67%
poseidon_bn254_hash 169,552 (+1,053) +0.62%
poseidon_bn254_hash_width_3 169,552 (+1,053) +0.62%
conditional_1 5,860 (+36) +0.62%
regression_5252 951,206 (+5,400) +0.57%
poseidonsponge_x5_254 190,917 (+1,080) +0.57%
strings 1,897 (+9) +0.48%
regression_4449 219,762 (+630) +0.29%
u128 25,428 (+72) +0.28%
sha256_var_witness_const_regression 7,036 (+18) +0.26%
sha256 10,844 (+27) +0.25%
6 7,357 (+18) +0.25%
conditional_regression_short_circuit 7,432 (+18) +0.24%
eddsa 737,279 (+1,620) +0.22%
brillig_sha256 4,200 (+9) +0.21%
fold_numeric_generic_poseidon 5,239 (+9) +0.17%
no_predicates_numeric_generic_poseidon 5,239 (+9) +0.17%
ecdsa_secp256k1 7,075 (+9) +0.13%
sha256_var_size_regression 18,331 (+18) +0.10%
brillig_keccak 37,099 (+36) +0.10%
keccak256 37,099 (+36) +0.10%
brillig_cow_regression 583,839 (+252) +0.04%
sha256_regression 126,254 (+45) +0.04%
sha256_var_padding_regression 234,837 (+54) +0.02%
ram_blowup_regression 877,404 (+9) +0.00%

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