chore: LICM refactors by TomAFrench · Pull Request #9642 · noir-lang/noir

TomAFrench · 2025-08-26T17:53:54Z

Description

Problem*

Resolves

Summary*

This branch is going to accumulate small refactorings of the LICM while I'm working on the audit.

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.

github-actions

⚠️ 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: `8d3a421`	Previous: `f601afe`	Ratio
`test_report_zkpassport_noir-ecdsa_`	`2` s	`1` s	`2`
`test_report_zkpassport_noir_rsa_`	`1` s	`0` s	`+∞`

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

CC: @TomAFrench

github-actions

⚠️ 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: `e9f599b`	Previous: `cc0c20d`	Ratio
`private-kernel-inner`	`0.019` s	`0.014` s	`1.36`

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

CC: @TomAFrench

github-actions

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: `3d1e31a`	Previous: `f601afe`	Ratio
`rollup-block-root-empty`	`25.66` s	`21.24` s	`1.21`

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

CC: @TomAFrench

vezenovm · 2025-08-27T16:18:10Z

Would still be good to see the compilation effect as we did change some of the logic of the pass here.

github-actions

ACVM Benchmarks

Details

Benchmark suite	Current: `605740c`	Previous: `b544e60`	Ratio
`purely_sequential_opcodes`	`249718` ns/iter (`± 647`)	`248430` ns/iter (`± 776`)	`1.01`
`perfectly_parallel_opcodes`	`220014` ns/iter (`± 2194`)	`218561` ns/iter (`± 6896`)	`1.01`
`perfectly_parallel_batch_inversion_opcodes`	`2781240` ns/iter (`± 1569`)	`2782471` ns/iter (`± 11623`)	`1.00`

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

github-actions

Compilation Time

Details

Benchmark suite	Current: `605740c`	Previous: `b544e60`	Ratio
`private-kernel-inner`	`1.77` s	`1.75` s	`1.01`
`private-kernel-reset`	`7.9` s	`7.686` s	`1.03`
`private-kernel-tail`	`1.364` s	`1.426` s	`0.96`
`rollup-base-private`	`15.56` s	`15.36` s	`1.01`
`rollup-base-public`	`13.68` s	`14.16` s	`0.97`
`rollup-block-root-empty`	`21.46` s	`21.32` s	`1.01`
`rollup-block-root-single-tx`	`230` s	`199` s	`1.16`
`rollup-block-root`	`243` s	`210` s	`1.16`
`rollup-merge`	`1.376` s	`1.358` s	`1.01`
`rollup-root`	`1.474` s	`1.47` s	`1.00`
`semaphore-depth-10`	`0.822` s	`0.777` s	`1.06`
`sha512-100-bytes`	`1.689` s	`1.624` s	`1.04`

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

github-actions

Execution Time

Details

Benchmark suite	Current: `605740c`	Previous: `b544e60`	Ratio
`private-kernel-inner`	`0.016` s	`0.019` s	`0.84`
`private-kernel-reset`	`0.155` s	`0.153` s	`1.01`
`private-kernel-tail`	`0.01` s	`0.01` s	`1`
`rollup-base-private`	`0.264` s	`0.264` s	`1`
`rollup-base-public`	`0.165` s	`0.164` s	`1.01`
`rollup-block-root`	`13.3` s	`12.9` s	`1.03`
`rollup-merge`	`0.002` s	`0.002` s	`1`
`rollup-root`	`0.004` s	`0.004` s	`1`
`semaphore-depth-10`	`0.02` s	`0.019` s	`1.05`
`sha512-100-bytes`	`0.103` s	`0.1` s	`1.03`

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

github-actions

Opcode count

Details

Benchmark suite	Current: `605740c`	Previous: `b544e60`	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`	`221335` opcodes	`221335` opcodes	`1`
`rollup-base-public`	`159954` opcodes	`159954` opcodes	`1`
`rollup-block-root-empty`	`68106` opcodes	`68106` opcodes	`1`
`rollup-block-root-single-tx`	`964509` opcodes	`964509` opcodes	`1`
`rollup-block-root`	`965795` opcodes	`965795` 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.

github-actions

Artifact Size

Details

Benchmark suite	Current: `605740c`	Previous: `b544e60`	Ratio
`private-kernel-inner`	`708.9` KB	`708.9` KB	`1`
`private-kernel-reset`	`2032.7` KB	`2032.7` KB	`1`
`private-kernel-tail`	`536.5` KB	`536.5` KB	`1`
`rollup-base-private`	`4320.9` KB	`4320.9` KB	`1`
`rollup-base-public`	`3334.3` KB	`3334.3` KB	`1`
`rollup-block-root-empty`	`3857` KB	`3857` KB	`1`
`rollup-block-root-single-tx`	`30756.4` KB	`30756.4` KB	`1`
`rollup-block-root`	`30786.1` KB	`30786.1` KB	`1`
`rollup-merge`	`188.2` KB	`188.2` KB	`1`
`rollup-root`	`390.5` KB	`390.5` KB	`1`
`semaphore-depth-10`	`631.5` KB	`631.5` KB	`1`
`sha512-100-bytes`	`525.5` KB	`525.5` KB	`1`

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

github-actions

Test Suite Duration

Details

Benchmark suite	Current: `605740c`	Previous: `b544e60`	Ratio
`test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr`	`96` s	`99` s	`0.97`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts`	`108` s	`109` s	`0.99`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob`	`182` s	`188` s	`0.97`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib`	`226` s	`208` s	`1.09`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_reset-kernel-lib`	`34` s	`33` s	`1.03`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib`	`607` s	`593` s	`1.02`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types`	`95` s	`93` s	`1.02`
`test_report_noir-lang_noir-bignum_`	`133` s	`136` s	`0.98`
`test_report_noir-lang_noir_bigcurve_`	`328` s	`279` s	`1.18`
`test_report_noir-lang_sha256_`	`16` s	`15` s	`1.07`
`test_report_noir-lang_sha512_`	`14` s	`15` s	`0.93`
`test_report_zkpassport_noir-ecdsa_`	`2` s	`2` s	`1`
`test_report_zkpassport_noir_rsa_`	`2` s	`2` s	`1`

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

github-actions

Compilation Memory

Details

Benchmark suite	Current: `605740c`	Previous: `b544e60`	Ratio
`private-kernel-inner`	`239.58` MB	`239.58` MB	`1`
`private-kernel-reset`	`549.15` MB	`549.15` MB	`1`
`private-kernel-tail`	`213.94` MB	`213.94` MB	`1`
`rollup-base-private`	`1350` MB	`1350` MB	`1`
`rollup-base-public`	`1400` MB	`1400` MB	`1`
`rollup-block-root-empty`	`1010` MB	`1010` MB	`1`
`rollup-block-root-single-tx`	`9690` MB	`9690` MB	`1`
`rollup-block-root`	`9690` MB	`9690` MB	`1`
`rollup-merge`	`330.59` MB	`330.59` MB	`1`
`rollup-root`	`341.33` MB	`341.33` MB	`1`
`semaphore_depth_10`	`104.73` MB	`104.73` MB	`1`
`sha512_100_bytes`	`233.08` MB	`232.98` MB	`1.00`

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

github-actions

Execution Memory

Details

Benchmark suite	Current: `605740c`	Previous: `b544e60`	Ratio
`private-kernel-inner`	`212.98` MB	`212.98` MB	`1`
`private-kernel-reset`	`246.18` MB	`246.18` MB	`1`
`private-kernel-tail`	`197.81` MB	`197.81` MB	`1`
`rollup-base-private`	`501.57` MB	`501.57` MB	`1`
`rollup-base-public`	`434.03` MB	`434.03` MB	`1`
`rollup-block-root`	`1500` MB	`1500` MB	`1`
`rollup-merge`	`328.36` MB	`328.36` MB	`1`
`rollup-root`	`330.89` MB	`330.89` MB	`1`
`semaphore_depth_10`	`69.53` MB	`69.53` MB	`1`
`sha512_100_bytes`	`55.03` MB	`55.03` MB	`1`

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

TomAFrench · 2025-08-27T16:46:01Z

Would still be good to see the compilation effect as we did change some of the logic of the pass here.

I don't think there's been any significant changes to the main logic of the pass, mostly encapsulating logic and data a bit more.

vezenovm · 2025-08-27T17:02:15Z

I don't think there's been any significant changes to the main logic of the pass, mostly encapsulating logic and data a bit more.

Yes agreed, I noticed the alert so I wanted to see if it affected benchmarks more generally. I was mainly thinking of the current_induction_variables which have a new type signature. Agreed it looks like there are no other logic changes and the benchmarks are more in line with what I expected now. Most likely we were just out of date with master.

compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs

Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>

Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE fix: make Ord for slices lexicographic (elements first, then length) (noir-lang/noir#9555) chore(ssa): Refactor `unrolling` (noir-lang/noir#9653) chore(docs): Update dependency page's examples (noir-lang/noir#9634) fix(ssa): Constant fold Brillig calls using the SSA interpreter (noir-lang/noir#9655) chore: LICM refactors (noir-lang/noir#9642) chore: add test for trait bound on implementing type (noir-lang/noir#9652) chore: pass `DataFlowGraph` instead of `Function` as arg (noir-lang/noir#9656) feat: Group one audit tests (noir-lang/noir#9445) fix(expand): better handling of dereferences (again) (noir-lang/noir#9654) feat(mem2reg): address last known value is independent of its aliases (take three) (noir-lang/noir#9633) chore: remove handling for slice arguments to MSM (noir-lang/noir#9648) fix: validate binary operations which do not allow fields (noir-lang/noir#9649) END_COMMIT_OVERRIDE

TomAFrench added 8 commits August 27, 2025 14:12

chore: create a new type for storing the PDF

832aab7

chore: calculate header boolean earlier

54c3cad

chore: move simplification into submodule

0250886

chore: remove unnecessary map

ee45ba8

fix

dffa8bf

.

cac5bf3

chore: use immutable references

8780573

chore: separate codepaths related to induction variables

1d0ebef

TomAFrench force-pushed the tf/licm-refactor branch from 5b21bab to 1d0ebef Compare August 27, 2025 14:12

TomAFrench added 3 commits August 27, 2025 14:17

chore: improve structure of current_induction_variable

42e3262

chore: bubble up assertion on current_block_executes

3cd0249

chore: elide lifetime

75efb68

github-actions bot reviewed Aug 27, 2025

View reviewed changes

chore: separate state which is local to a loop from persistent state

844d988

github-actions bot reviewed Aug 27, 2025

View reviewed changes

TomAFrench added 2 commits August 27, 2025 15:24

.

e9f599b

.

b5962fa

TomAFrench marked this pull request as ready for review August 27, 2025 16:09

TomAFrench requested a review from a team August 27, 2025 16:09

vezenovm added the bench-show Display benchmark results on PR label Aug 27, 2025

Merge branch 'master' into tf/licm-refactor

663eaa0

github-actions bot reviewed Aug 27, 2025

View reviewed changes

chore: split off small helpers into free functions

936dd17

chore: move LoopContext out of LoopInvariantContext

3d1e31a

vezenovm approved these changes Aug 27, 2025

View reviewed changes

compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs Outdated Show resolved Hide resolved

compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs Outdated Show resolved Hide resolved

compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs Show resolved Hide resolved

TomAFrench and others added 2 commits August 27, 2025 18:46

Update compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs

2f550db

Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>

Update compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs

8d3a421

Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>

TomAFrench enabled auto-merge August 27, 2025 17:52

TomAFrench added this pull request to the merge queue Aug 27, 2025

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Aug 27, 2025

Merge branch 'master' into tf/licm-refactor

605740c

TomAFrench enabled auto-merge August 27, 2025 19:02

TomAFrench added this pull request to the merge queue Aug 27, 2025

Merged via the queue into master with commit 70bc893 Aug 27, 2025
122 checks passed

TomAFrench deleted the tf/licm-refactor branch August 27, 2025 19:53

AztecBot mentioned this pull request Aug 28, 2025

chore: Bump Noir reference AztecProtocol/aztec-packages#16627

Merged

Conversation

TomAFrench commented Aug 26, 2025

Description

Problem*

Summary*

Additional Context

Documentation*

PR Checklist*

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

vezenovm commented Aug 27, 2025

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

ACVM Benchmarks

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Compilation Time

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Execution Time

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Opcode count

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Artifact Size

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Test Suite Duration

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Compilation Memory

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Execution Memory

Uh oh!

TomAFrench commented Aug 27, 2025

Uh oh!

vezenovm commented Aug 27, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading