Skip to content

chore(licm): Break things up further in LICM#9683

Merged
aakoshh merged 7 commits intomasterfrom
af/refactor-licm
Aug 29, 2025
Merged

chore(licm): Break things up further in LICM#9683
aakoshh merged 7 commits intomasterfrom
af/refactor-licm

Conversation

@aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Aug 29, 2025

Description

Problem*

LoopContext had a bunch of current_block_* fields which had to be reset for each block we iterated over in hoist_loop_invariants, and assigned new values in various methods that also mutated other fields on LoopContext. Some were unrelated, like is_control_dependent_post_pre_header also set the block purity, seemingly just because all_predecessors was available there. This made it hard to find where various flags on LoopContext came from.

Summary*

  • Breaks out the current_block_* fields from LoopContext into a BlockContext and introduces an LoopInvariantContext::init_block_context method to create it, so we cannot forget to set the values, and there is no risk of reusing something from the previous iteration.
  • Changes all query methods for the BlockContext initialisation to avoid mutating the LoopContext, so it's a bit more obvious where things are set, all clear in the init_block_context method.
  • Renames get_values_defined_in_loop to init_loop_context as it did more than what it said on the tin.
  • Makes LoopContext::pre_header non-optional.
  • Copied some extra explanation for control dependence from the paper to have an intuitive definition.
  • Avoid checking repeatedly whether the bounds in the LoopContext guarantee execution by storing the result in a field.

Additional Context

I haven't confirmed, but the fact that is_control_dependent_post_pre_header exited early if the block was part of a known control dependent nested loop, and then it did not set the is_current_block_impure flag, was possibly a bug. Now that they are separate, the impurity flag is set in all cases.

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.

@aakoshh aakoshh requested a review from a team August 29, 2025 16:07
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.

⚠️ 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: d09fa36 Previous: a38288a Ratio
test_report_zkpassport_noir_rsa_ 2 s 1 s 2

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

CC: @TomAFrench

@vezenovm
Copy link
Contributor

Overall looks good! Just have that one request for removing the ControlDependenceResult variant with the lifetime.

aakoshh and others added 3 commits August 29, 2025 18:02
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
@aakoshh aakoshh enabled auto-merge August 29, 2025 18:14
@aakoshh aakoshh added this pull request to the merge queue Aug 29, 2025
Merged via the queue into master with commit 3b6b359 Aug 29, 2025
171 of 173 checks passed
@aakoshh aakoshh deleted the af/refactor-licm branch August 29, 2025 18:48
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Sep 2, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: remove duplicated frontend tests (noir-lang/noir#9706)
chore: remove playwright workaround (noir-lang/noir#9704)
fix(licm): Use `Loop::header` in `Loop::is_fully_executed` (noir-lang/noir#9700)
chore: show which type is invalid as program input (noir-lang/noir#9701)
chore: bump deps (noir-lang/noir#9698)
chore: bump external pinned commits (noir-lang/noir#9693)
chore(licm): Break things up further in LICM (noir-lang/noir#9683)
chore(docs): spinning out bb docs (noir-lang/noir#9402)
fix(ssa)!: Signed shift overflow checks rhs < bit_size (noir-lang/noir#9685)
chore: add extra bitshifts tests (noir-lang/noir#9680)
feat: Propagate purities using SCCs (noir-lang/noir#9672)
chore: break `NodeInterner` into chunks (noir-lang/noir#9674)
fix(formatter): don't revert indentation increase after popping it (noir-lang/noir#9673)
feat: hoist safe casts from loops (noir-lang/noir#9645)
chore: fix clippy warnings (noir-lang/noir#9675)
chore(ssa): Refactor flattening (noir-lang/noir#9663)
chore(ssa): Greenlight `brillig_entry_points` and switch to centralized CallGraph (noir-lang/noir#9668)
chore: add two mem2reg regression tests where references are returned (noir-lang/noir#9670)
fix(mem2reg): reuse existing expression and add missing alias (noir-lang/noir#9664)
chore: add tests for bounded_vec (noir-lang/noir#9576)
chore: redact debug info and file maps from snapshots (noir-lang/noir#9666)
chore: pull out interpreter binary evaluation logic into pure functions (noir-lang/noir#9665)
feat: brillig functions can be pure if they are not entry points (noir-lang/noir#9659)
END_COMMIT_OVERRIDE

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Sep 2, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: remove duplicated frontend tests (noir-lang/noir#9706)
chore: remove playwright workaround (noir-lang/noir#9704)
fix(licm): Use `Loop::header` in `Loop::is_fully_executed` (noir-lang/noir#9700)
chore: show which type is invalid as program input (noir-lang/noir#9701)
chore: bump deps (noir-lang/noir#9698)
chore: bump external pinned commits (noir-lang/noir#9693)
chore(licm): Break things up further in LICM (noir-lang/noir#9683)
chore(docs): spinning out bb docs (noir-lang/noir#9402)
fix(ssa)!: Signed shift overflow checks rhs < bit_size (noir-lang/noir#9685)
chore: add extra bitshifts tests (noir-lang/noir#9680)
feat: Propagate purities using SCCs (noir-lang/noir#9672)
chore: break `NodeInterner` into chunks (noir-lang/noir#9674)
fix(formatter): don't revert indentation increase after popping it (noir-lang/noir#9673)
feat: hoist safe casts from loops (noir-lang/noir#9645)
chore: fix clippy warnings (noir-lang/noir#9675)
chore(ssa): Refactor flattening (noir-lang/noir#9663)
chore(ssa): Greenlight `brillig_entry_points` and switch to centralized CallGraph (noir-lang/noir#9668)
chore: add two mem2reg regression tests where references are returned (noir-lang/noir#9670)
fix(mem2reg): reuse existing expression and add missing alias (noir-lang/noir#9664)
chore: add tests for bounded_vec (noir-lang/noir#9576)
chore: redact debug info and file maps from snapshots (noir-lang/noir#9666)
chore: pull out interpreter binary evaluation logic into pure functions (noir-lang/noir#9665)
feat: brillig functions can be pure if they are not entry points (noir-lang/noir#9659)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Sep 2, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: remove duplicated frontend tests
(noir-lang/noir#9706)
chore: remove playwright workaround
(noir-lang/noir#9704)
fix(licm): Use `Loop::header` in `Loop::is_fully_executed`
(noir-lang/noir#9700)
chore: show which type is invalid as program input
(noir-lang/noir#9701)
chore: bump deps (noir-lang/noir#9698)
chore: bump external pinned commits
(noir-lang/noir#9693)
chore(licm): Break things up further in LICM
(noir-lang/noir#9683)
chore(docs): spinning out bb docs
(noir-lang/noir#9402)
fix(ssa)!: Signed shift overflow checks rhs < bit_size
(noir-lang/noir#9685)
chore: add extra bitshifts tests
(noir-lang/noir#9680)
feat: Propagate purities using SCCs
(noir-lang/noir#9672)
chore: break `NodeInterner` into chunks
(noir-lang/noir#9674)
fix(formatter): don't revert indentation increase after popping it
(noir-lang/noir#9673)
feat: hoist safe casts from loops
(noir-lang/noir#9645)
chore: fix clippy warnings (noir-lang/noir#9675)
chore(ssa): Refactor flattening
(noir-lang/noir#9663)
chore(ssa): Greenlight `brillig_entry_points` and switch to centralized
CallGraph (noir-lang/noir#9668)
chore: add two mem2reg regression tests where references are returned
(noir-lang/noir#9670)
fix(mem2reg): reuse existing expression and add missing alias
(noir-lang/noir#9664)
chore: add tests for bounded_vec
(noir-lang/noir#9576)
chore: redact debug info and file maps from snapshots
(noir-lang/noir#9666)
chore: pull out interpreter binary evaluation logic into pure functions
(noir-lang/noir#9665)
feat: brillig functions can be pure if they are not entry points
(noir-lang/noir#9659)
END_COMMIT_OVERRIDE
mralj pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 13, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: remove duplicated frontend tests (noir-lang/noir#9706)
chore: remove playwright workaround (noir-lang/noir#9704)
fix(licm): Use `Loop::header` in `Loop::is_fully_executed` (noir-lang/noir#9700)
chore: show which type is invalid as program input (noir-lang/noir#9701)
chore: bump deps (noir-lang/noir#9698)
chore: bump external pinned commits (noir-lang/noir#9693)
chore(licm): Break things up further in LICM (noir-lang/noir#9683)
chore(docs): spinning out bb docs (noir-lang/noir#9402)
fix(ssa)!: Signed shift overflow checks rhs < bit_size (noir-lang/noir#9685)
chore: add extra bitshifts tests (noir-lang/noir#9680)
feat: Propagate purities using SCCs (noir-lang/noir#9672)
chore: break `NodeInterner` into chunks (noir-lang/noir#9674)
fix(formatter): don't revert indentation increase after popping it (noir-lang/noir#9673)
feat: hoist safe casts from loops (noir-lang/noir#9645)
chore: fix clippy warnings (noir-lang/noir#9675)
chore(ssa): Refactor flattening (noir-lang/noir#9663)
chore(ssa): Greenlight `brillig_entry_points` and switch to centralized CallGraph (noir-lang/noir#9668)
chore: add two mem2reg regression tests where references are returned (noir-lang/noir#9670)
fix(mem2reg): reuse existing expression and add missing alias (noir-lang/noir#9664)
chore: add tests for bounded_vec (noir-lang/noir#9576)
chore: redact debug info and file maps from snapshots (noir-lang/noir#9666)
chore: pull out interpreter binary evaluation logic into pure functions (noir-lang/noir#9665)
feat: brillig functions can be pure if they are not entry points (noir-lang/noir#9659)
END_COMMIT_OVERRIDE
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