Skip to content

chore(ssa): Flatten basic Brillig conditionals before inlining of simple functions#9761

Closed
vezenovm wants to merge 2 commits intomasterfrom
mv/flatten-brillig-cond-before-inlining
Closed

chore(ssa): Flatten basic Brillig conditionals before inlining of simple functions#9761
vezenovm wants to merge 2 commits intomasterfrom
mv/flatten-brillig-cond-before-inlining

Conversation

@vezenovm
Copy link
Contributor

@vezenovm vezenovm commented Sep 8, 2025

Description

Problem*

No issue just something I noticed when working on #9487. There are sometimes functions that would have a lot of knockdown simplifications but get muddled as being complex due to having multiple blocks.

Summary*

I am pushing this as a draft to see if it has any large effect on compilation times.

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 added the bench-show Display benchmark results on PR label Sep 8, 2025
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: 7a9096a Previous: d664016 Ratio
purely_sequential_opcodes 247106 ns/iter (± 2781) 251068 ns/iter (± 637) 0.98
perfectly_parallel_opcodes 218221 ns/iter (± 5528) 222699 ns/iter (± 4162) 0.98
perfectly_parallel_batch_inversion_opcodes 2784648 ns/iter (± 24888) 2803001 ns/iter (± 1089) 0.99

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

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2025

Changes to Brillig bytecode sizes

Generated at commit: f7c886545bf9865c6ca9414637f82eb8831c9147, compared to commit: d664016a1d52b6262f0ad378b9bf98d380ff650f

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
trait_impl_base_type_inliner_zero -33 ✅ -52.38%
trait_impl_base_type_inliner_min -71 ✅ -70.30%

Full diff report 👇
Program Brillig opcodes (+/-) %
regression_9538_inliner_max 93 (-1) -1.06%
regression_9538_inliner_min 93 (-1) -1.06%
regression_9538_inliner_zero 93 (-1) -1.06%
nested_if_then_block_same_cond_inliner_max 106 (-9) -7.83%
nested_if_then_block_same_cond_inliner_min 106 (-9) -7.83%
nested_if_then_block_same_cond_inliner_zero 106 (-9) -7.83%
brillig_conditional_inliner_min 35 (-9) -20.45%
trait_impl_base_type_inliner_zero 30 (-33) -52.38%
trait_impl_base_type_inliner_min 30 (-71) -70.30%

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2025

Changes to number of Brillig opcodes executed

Generated at commit: f7c886545bf9865c6ca9414637f82eb8831c9147, compared to commit: d664016a1d52b6262f0ad378b9bf98d380ff650f

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
trait_impl_base_type_inliner_zero -45 ✅ -63.38%
trait_impl_base_type_inliner_min -97 ✅ -78.86%

Full diff report 👇
Program Brillig opcodes (+/-) %
regression_9538_inliner_max 154 (-1) -0.65%
regression_9538_inliner_min 154 (-1) -0.65%
regression_9538_inliner_zero 154 (-1) -0.65%
nested_if_then_block_same_cond_inliner_max 173 (-4) -2.26%
nested_if_then_block_same_cond_inliner_min 173 (-4) -2.26%
nested_if_then_block_same_cond_inliner_zero 173 (-4) -2.26%
brillig_conditional_inliner_min 31 (-13) -29.55%
trait_impl_base_type_inliner_zero 26 (-45) -63.38%
trait_impl_base_type_inliner_min 26 (-97) -78.86%

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: 7a9096a Previous: d664016 Ratio
private-kernel-inner 1.9 s 1.778 s 1.07
private-kernel-reset 8.426 s 8.418 s 1.00
private-kernel-tail 1.362 s 1.334 s 1.02
rollup-base-private 18.82 s 19.02 s 0.99
rollup-base-public 16.54 s 16.08 s 1.03
rollup-block-root-empty 17.44 s 17.42 s 1.00
rollup-block-root-single-tx 198 s 194 s 1.02
rollup-block-root 214 s 210 s 1.02
rollup-merge 1.414 s 1.378 s 1.03
rollup-root 1.474 s 1.508 s 0.98
semaphore-depth-10 0.791 s 0.788 s 1.00
sha512-100-bytes 1.808 s 1.835 s 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.

Artifact Size

Details
Benchmark suite Current: 7a9096a Previous: d664016 Ratio
private-kernel-inner 709.8 KB 709.5 KB 1.00
private-kernel-reset 2025.1 KB 2025.1 KB 1
private-kernel-tail 531.6 KB 531.7 KB 1.00
rollup-base-private 4349 KB 4349 KB 1
rollup-base-public 3353.3 KB 3353.3 KB 1
rollup-block-root-empty 3813.5 KB 3813.5 KB 1
rollup-block-root-single-tx 30682.4 KB 30682.4 KB 1
rollup-block-root 30720.4 KB 30720.4 KB 1
rollup-merge 188.1 KB 188.1 KB 1
rollup-root 390.6 KB 390.6 KB 1
semaphore-depth-10 631.9 KB 631.9 KB 1
sha512-100-bytes 525.5 KB 525.5 KB 1

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: 7a9096a Previous: d664016 Ratio
private-kernel-inner 14792 opcodes 14792 opcodes 1
private-kernel-reset 68868 opcodes 68868 opcodes 1
private-kernel-tail 11177 opcodes 11177 opcodes 1
rollup-base-private 221339 opcodes 221339 opcodes 1
rollup-base-public 159930 opcodes 159930 opcodes 1
rollup-block-root-empty 68108 opcodes 68108 opcodes 1
rollup-block-root-single-tx 964515 opcodes 964515 opcodes 1
rollup-block-root 965801 opcodes 965801 opcodes 1
rollup-merge 1409 opcodes 1409 opcodes 1
rollup-root 2631 opcodes 2631 opcodes 1
semaphore-depth-10 5700 opcodes 5700 opcodes 1
sha512-100-bytes 13173 opcodes 13173 opcodes 1

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: 7a9096a Previous: d664016 Ratio
private-kernel-inner 0.017 s 0.015 s 1.13
private-kernel-reset 0.162 s 0.163 s 0.99
private-kernel-tail 0.011 s 0.011 s 1
rollup-base-private 0.267 s 0.266 s 1.00
rollup-base-public 0.166 s 0.161 s 1.03
rollup-block-root 14.5 s 14.4 s 1.01
rollup-merge 0.002 s 0.002 s 1
rollup-root 0.004 s 0.004 s 1
semaphore-depth-10 0.019 s 0.021 s 0.90
sha512-100-bytes 0.102 s 0.092 s 1.11

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

@vezenovm
Copy link
Contributor Author

vezenovm commented Sep 8, 2025

The results are not wide reaching enough to make me want to merge this as is so I am closing the PR. The main takeaway is that the trait_impl_base_type_inliner_zero results indicate our inliner still has some room for improvement when determining "simple" functions.

@vezenovm vezenovm closed this Sep 8, 2025
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: 7a9096a Previous: d664016 Ratio
private-kernel-inner 284.51 MB 284.57 MB 1.00
private-kernel-reset 593.53 MB 593.53 MB 1
private-kernel-tail 259.08 MB 259.08 MB 1
rollup-base-private 1360 MB 1360 MB 1
rollup-base-public 1420 MB 1420 MB 1
rollup-block-root-empty 998.82 MB 998.82 MB 1
rollup-block-root-single-tx 9680 MB 9680 MB 1
rollup-block-root 9690 MB 9690 MB 1
rollup-merge 332.74 MB 332.74 MB 1
rollup-root 343.48 MB 343.48 MB 1
semaphore_depth_10 106.96 MB 106.96 MB 1
sha512_100_bytes 254.08 MB 254.12 MB 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 Memory

Details
Benchmark suite Current: 7a9096a Previous: d664016 Ratio
private-kernel-inner 258.16 MB 258.16 MB 1
private-kernel-reset 291.32 MB 291.32 MB 1
private-kernel-tail 242.9 MB 242.9 MB 1
rollup-base-private 506.04 MB 506.04 MB 1
rollup-base-public 438.42 MB 438.42 MB 1
rollup-block-root 1500 MB 1500 MB 1
rollup-merge 330.5 MB 330.5 MB 1
rollup-root 333.02 MB 333.02 MB 1
semaphore_depth_10 72.45 MB 72.45 MB 1
sha512_100_bytes 58.23 MB 58.23 MB 1

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: 7a9096a Previous: d664016 Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr 99 s 100 s 0.99
test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts 129 s 127 s 1.02
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 218 s 223 s 0.98
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib 214 s 219 s 0.98
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_reset-kernel-lib 33 s 34 s 0.97
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib 641 s 592 s 1.08
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types 98 s 95 s 1.03
test_report_noir-lang_noir-bignum_ 133 s 139 s 0.96
test_report_noir-lang_noir_bigcurve_ 318 s 349 s 0.91
test_report_noir-lang_sha256_ 15 s 15 s 1
test_report_noir-lang_sha512_ 12 s 13 s 0.92
test_report_zkpassport_noir-ecdsa_ 1 s 2 s 0.50
test_report_zkpassport_noir_rsa_ 1 s 1 s 1

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

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.

1 participant