Skip to content

chore: Allow control dependent LICM in ACIR#7700

Merged
vezenovm merged 7 commits intomv/control-dep-licmfrom
mv/control-dep-licm-acir
Mar 20, 2025
Merged

chore: Allow control dependent LICM in ACIR#7700
vezenovm merged 7 commits intomv/control-dep-licmfrom
mv/control-dep-licm-acir

Conversation

@vezenovm
Copy link
Contributor

@vezenovm vezenovm commented Mar 13, 2025

Description

Problem*

No issue. I just wanted to test the effects of activating control dependent on Brillig only and then also ACIR.

Summary*

Similar to #6785 I am expecting some kind of improvement in ACIR compilation. I expect an improvement as we will be potentially hoisting more instructions out of loops, thus reducing the work of loop unrolling which is required in the ACIR runtime. We do not expect any gate count improvements as ACIR ultimately unrolls all loop. The only real benefit is potentially unrolling less instructions.

The initial implementation of #7660 was quite inefficient and led to large compilation time regressions. After switching to the usage of post-dominance frontiers there are no longer an compilation regression in #7660 so I want to see the benefits we get from activating control dependent LICM on ACIR as well.

Additional Context

Good memory reductions on the rollup circuits:

rollup-block-root-single-tx 7190 MB 7860 MB 0.91
rollup-block-root 7190 MB 7870 MB 0.91

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 Mar 13, 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.

Execution Time

Details
Benchmark suite Current: d6a3bf7 Previous: 58be0cd Ratio
private-kernel-inner 0.069 s 0.069 s 1
private-kernel-reset 0.287 s 0.283 s 1.01
private-kernel-tail 0.026 s 0.026 s 1
rollup-base-private 0.737 s 0.73 s 1.01
rollup-base-public 0.487 s 0.487 s 1
rollup-block-root 16.7 s 16.7 s 1
rollup-merge 0.006 s 0.006 s 1
rollup-root 0.025 s 0.025 s 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.

Compilation Time

Details
Benchmark suite Current: d6a3bf7 Previous: 58be0cd Ratio
regression_4709 0.729 s 0.67 s 1.09
ram_blowup_regression 14.9 s 14.6 s 1.02
global_var_regression_entry_points 0.5 s 0.484 s 1.03
private-kernel-inner 2.278 s 2.388 s 0.95
private-kernel-reset 6.85 s 6.782 s 1.01
private-kernel-tail 1.102 s 1.104 s 1.00
rollup-base-private 16.86 s 15.2 s 1.11
rollup-base-public 12.26 s 11.34 s 1.08
rollup-block-root-empty 0.963 s 0.933 s 1.03
rollup-block-root-single-tx 124 s 129 s 0.96
rollup-block-root 126 s 133 s 0.95
rollup-merge 0.961 s 0.928 s 1.04
rollup-root 1.558 s 1.57 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.

Test Suite Duration

Details
Benchmark suite Current: d6a3bf7 Previous: 58be0cd Ratio
AztecProtocol_aztec-packages_noir-projects_aztec-nr 37 s 37 s 1
AztecProtocol_aztec-packages_noir-projects_noir-contracts 83 s 78 s 1.06
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 49 s 44 s 1.11
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib 185 s 170 s 1.09
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_reset-kernel-lib 10 s 9 s 1.11
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib 159 s 168 s 0.95
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types 53 s 54 s 0.98
noir-lang_noir-bignum_ 80 s 86 s 0.93
noir-lang_noir_bigcurve_ 215 s 209 s 1.03
noir-lang_noir_json_parser_ 9 s 8 s 1.13
noir-lang_sha512_ 25 s 23 s 1.09

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: d6a3bf7 Previous: 58be0cd Ratio
private-kernel-inner 301.67 MB 301.63 MB 1.00
private-kernel-reset 611.54 MB 611.54 MB 1
private-kernel-tail 228.23 MB 228.23 MB 1
rollup-base-private 1250 MB 1250 MB 1
rollup-base-public 1330 MB 1330 MB 1
rollup-block-root-empty 305.08 MB 305.11 MB 1.00
rollup-block-root-single-tx 7860 MB 7860 MB 1
rollup-block-root 7870 MB 7870 MB 1
rollup-merge 303.51 MB 303.53 MB 1.00
rollup-root 351.06 MB 351.05 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: d6a3bf7 Previous: 58be0cd Ratio
private-kernel-inner 241.44 MB 241.44 MB 1
private-kernel-reset 275.87 MB 275.87 MB 1
private-kernel-tail 215.09 MB 215.09 MB 1
rollup-base-private 509.27 MB 509.27 MB 1
rollup-base-public 418.07 MB 418.07 MB 1
rollup-block-root 1420 MB 1420 MB 1
rollup-merge 289.01 MB 289 MB 1.00
rollup-root 295.48 MB 295.48 MB 1

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

@vezenovm
Copy link
Contributor Author

vezenovm commented Mar 13, 2025

rollup-block-root-single-tx 122 s 126 s 0.97
rollup-block-root 134 s 125 s 1.07

So compilation time seems to grow slightly for rollup-block-root

We look to drop by 680 MB in peak memory.

rollup-block-root-single-tx 7190 MB 7860 MB 0.91
rollup-block-root 7190 MB 7870 MB 0.91

I have not profiled memory consumption on these programs yet, but it is mostly likely due to the peak memory usage occurring during loop unrolling and LICM reducing the number of instructions we need to unroll.

EDIT: It looks like we no longer have much of a compilation time regression (and in the case of rollup-block-root an improvement). So I am also marking this ready for review.

rollup-block-root-single-tx 127 s 121 s 1.05
rollup-block-root 125 s 134 s 0.93

@vezenovm vezenovm marked this pull request as ready for review March 14, 2025 17:51
@vezenovm vezenovm requested a review from a team March 14, 2025 17:53
@vezenovm
Copy link
Contributor Author

vezenovm commented Mar 14, 2025

rollup-base-private 16.54 s 15.36 s 1.08
rollup-base-public 12.2 s 11.02 s 1.11

That main regression is in the rollup base circuits.

@vezenovm vezenovm merged commit 52c1d85 into mv/control-dep-licm Mar 20, 2025
104 checks passed
@vezenovm vezenovm deleted the mv/control-dep-licm-acir branch March 20, 2025 18:09
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