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

feat: remove max_bit_size from Instruction::Truncate #7063

Closed
wants to merge 3 commits into from

Conversation

TomAFrench
Copy link
Member

Description

Problem*

Resolves #6998

Summary*

This PR just tries to remove this field as the only place it impacts ACIR seems irrelevant.

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 feat: remove max_bit_size from Instruction::truncate feat: remove max_bit_size from Instruction::Truncate Jan 14, 2025
Copy link
Contributor

github-actions bot commented Jan 14, 2025

Changes to circuit sizes

Generated at commit: 599b3503c8b98f203f1b7622524d0feee662702e, compared to commit: a0ffedfdf445b2dc6655cafa6fe614958ddb9603

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
signed_cmp +10 ❌ +15.87% +2,763 ❌ +1196.10%
signed_comparison +20 ❌ +13.99% +2,790 ❌ +936.24%
signed_arithmetic +50 ❌ +27.17% +834 ❌ +28.45%
unary_operator_overloading +5 ❌ +35.71% +710 ❌ +25.62%
regression_2660 +5 ❌ +27.78% +710 ❌ +25.59%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
signed_cmp 73 (+10) +15.87% 2,994 (+2,763) +1196.10%
signed_comparison 163 (+20) +13.99% 3,088 (+2,790) +936.24%
signed_arithmetic 234 (+50) +27.17% 3,765 (+834) +28.45%
unary_operator_overloading 19 (+5) +35.71% 3,481 (+710) +25.62%
regression_2660 23 (+5) +27.78% 3,485 (+710) +25.59%
simple_shift_left_right 19 (+5) +35.71% 3,528 (+710) +25.20%
sha2_byte 18,664 (+3,435) +22.56% 90,566 (+8,602) +10.49%
hashmap 38,925 (+2,025) +5.49% 105,210 (+6,630) +6.73%
sha256_var_witness_const_regression 1,180 (+105) +9.77% 17,466 (+989) +6.00%
conditional_1 2,875 (+15) +0.52% 10,780 (+491) +4.77%
6 376 (+30) +8.67% 11,648 (+530) +4.77%
conditional_regression_short_circuit 397 (+30) +8.17% 11,669 (+530) +4.76%
sha256 1,571 (+135) +9.40% 25,651 (+1,068) +4.34%
bit_shifts_runtime 344 (+60) +21.13% 4,108 (+163) +4.13%
u16_support 74 (+5) +7.25% 2,995 (+103) +3.56%
bit_shifts_comptime 23 (+10) +76.92% 2,934 (+90) +3.16%
sha256_var_size_regression 10,537 (+345) +3.39% 64,567 (+1,604) +2.55%
bench_sha256 204 (+20) +10.87% 7,193 (+144) +2.04%
array_dynamic_blackbox_input 1,382 (+20) +1.47% 21,805 (+423) +1.98%
regression 322 (+30) +10.27% 3,889 (+62) +1.62%
array_dynamic_nested_blackbox_input 250 (+5) +2.04% 7,406 (+102) +1.40%
bench_sha256_30 5,859 (+600) +11.41% 126,180 (+1,679) +1.35%
bench_sha256_100 19,509 (+2,000) +11.42% 413,388 (+5,389) +1.32%
signed_division 210 (+10) +5.00% 3,702 (+27) +0.73%
sha256_regression 22,532 (+190) +0.85% 181,583 (+1,197) +0.66%
5_over 25 (+5) +25.00% 3,508 (+20) +0.57%
regression_4449 8,897 (+350) +4.10% 286,170 (+1,306) +0.46%
ecdsa_secp256k1 510 (+20) +4.08% 43,704 (+143) +0.33%
sha256_var_padding_regression 3,848 (+30) +0.79% 194,096 (+443) +0.23%
ram_blowup_regression 140,761 (+5) +0.00% 643,632 (+373) +0.06%
binary_operator_overloading 127 (+5) +4.10% 4,190 (-72) -1.69%
regression_struct_array_conditional 71 (+5) +7.58% 3,124 (-72) -2.25%
u128 471 (+5) +1.07% 4,263 (-162) -3.66%

Copy link
Contributor

github-actions bot commented Jan 14, 2025

Compilation Report

Program Compilation Time %
sha256_regression 1.090s 9%
regression_4709 0.822s -2%
ram_blowup_regression 16.400s 1%
rollup-root 3.574s 8%
rollup-merge 1.820s 1%
rollup-block-merge 3.330s -2%
rollup-base-public 31.160s -2%
rollup-base-private 10.760s 1%
private-kernel-tail 0.965s -3%
private-kernel-reset 6.424s 2%
private-kernel-inner 1.984s 0%

Copy link
Contributor

github-actions bot commented Jan 14, 2025

Execution Report

Program Execution Time %
sha256_regression 0.053s 1%
regression_4709 0.001s 0%
ram_blowup_regression 0.609s 0%
rollup-root 0.105s 0%
rollup-merge 0.006s 0%
rollup-block-merge 0.104s 0%
rollup-base-public 1.220s -1%
rollup-base-private 0.449s -1%
private-kernel-tail 0.019s 0%
private-kernel-reset 0.313s -1%
private-kernel-inner 0.068s 0%

Copy link
Contributor

github-actions bot commented Jan 14, 2025

Compilation Memory Report

Program Peak Memory
keccak256 77.72M
workspace 123.47M
regression_4709 424.09M
ram_blowup_regression 1.58G
rollup-base-public 2.51G
rollup-base-private 901.14M
private-kernel-tail 207.18M
private-kernel-reset 669.26M
private-kernel-inner 294.40M

Copy link
Contributor

github-actions bot commented Jan 14, 2025

Execution Memory Report

Program Peak Memory
keccak256 74.67M
workspace 123.46M
regression_4709 316.00M
ram_blowup_regression 512.61M
rollup-base-public 478.93M
rollup-base-private 325.52M
private-kernel-tail 180.47M
private-kernel-reset 245.16M
private-kernel-inner 208.55M

@TomAFrench TomAFrench closed this Jan 14, 2025
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.

Do we need to track max_bit_size in Instruction::Truncate?
1 participant