Skip to content

fix: Fix if/match tracking in last uses pass#8935

Merged
jfecher merged 4 commits intomasterfrom
jf/fix-ownership
Jun 17, 2025
Merged

fix: Fix if/match tracking in last uses pass#8935
jfecher merged 4 commits intomasterfrom
jf/fix-ownership

Conversation

@jfecher
Copy link
Contributor

@jfecher jfecher commented Jun 16, 2025

Description

Problem*

Resolves #8923

Summary*

The BranchesPath in some situations could get confused as being the same as a prior if. E.g. in

if true {
    if true {
        foo(b);
    }
}

if true {
    if false {
    } else {
        foo(b);
    }
}

We tried to move b both times previously because we tracked Branch0 { Branch0 { use } } and left it intact until the next use which looks compatible, so they were merged into Branch0 { Branch0 { use }, Branch1 { use } }. To fix this I tagged each if and match with its own unique id.

Additional Context

This is a draft currently because I need to add a test

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.

@jfecher jfecher requested a review from a team June 16, 2025 20:34
@github-actions
Copy link
Contributor

github-actions bot commented Jun 16, 2025

Changes to number of Brillig opcodes executed

Generated at commit: f95cfb2b753ddc32a40d07d813c2f9a9e0d23c08, compared to commit: d1e55d93e31cefc8331d8f1174f56db87a995374

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
slices_inliner_max -3 ✅ -0.12%

Full diff report 👇
Program Brillig opcodes (+/-) %
slices_inliner_max 2,490 (-3) -0.12%

@github-actions
Copy link
Contributor

github-actions bot commented Jun 16, 2025

Changes to Brillig bytecode sizes

Generated at commit: f95cfb2b753ddc32a40d07d813c2f9a9e0d23c08, compared to commit: d1e55d93e31cefc8331d8f1174f56db87a995374

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
slice_regex_inliner_zero -12 ✅ -0.74%
slice_regex_inliner_max -24 ✅ -1.19%

Full diff report 👇
Program Brillig opcodes (+/-) %
slices_inliner_min 2,160 (-3) -0.14%
slices_inliner_zero 1,622 (-3) -0.18%
regression_1144_1169_2399_6609_inliner_min 992 (-3) -0.30%
slices_inliner_max 1,684 (-9) -0.53%
slice_regex_inliner_min 1,849 (-12) -0.64%
slice_regex_inliner_zero 1,620 (-12) -0.74%
slice_regex_inliner_max 1,992 (-24) -1.19%

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 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 6cc264a Previous: b04a51c Ratio
rollup-merge 0.004 s 0.003 s 1.33

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

CC: @TomAFrench

@jfecher jfecher marked this pull request as ready for review June 16, 2025 20:43
Copy link
Collaborator

@asterite asterite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! And it seems the code ends up being simpler too.

@jfecher jfecher enabled auto-merge June 17, 2025 17:08
@jfecher jfecher added this pull request to the merge queue Jun 17, 2025
Merged via the queue into master with commit 670063c Jun 17, 2025
118 checks passed
@jfecher jfecher deleted the jf/fix-ownership branch June 17, 2025 17:53
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jun 18, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(LSP): suggest generic type methods
(noir-lang/noir#8948)
fix: Fix if/match tracking in last uses pass
(noir-lang/noir#8935)
fix(ssa): Swap Brillig index shift and DIE in minimal pipeline
(noir-lang/noir#8946)
fix: when macro parse error happens, discard warnings; also preserve
unquoted token locations (noir-lang/noir#8944)
chore: disable noncritical workflow steps which fail on external prs
(noir-lang/noir#8939)
chore(test): Add `--minimal-ssa` to integration tests
(noir-lang/noir#8938)
fix(noirc_evaluator): U128 Binary::And simplification
(noir-lang/noir#8940)
chore: Improve access to data required to extract Noir
(noir-lang/noir#8793)
chore: bump external pinned commits
(noir-lang/noir#8920)
chore(fuzz): Compare print output on failed programs
(noir-lang/noir#8921)
chore(fuzz): Display nightly fuzz status badge
(noir-lang/noir#8932)
fix(fuzz): Consider values returned from Brillig to ACIR as dynamic
(noir-lang/noir#8931)
fix(fuzz): Fix env var name in fuzzing workflow
(noir-lang/noir#8929)
chore(fuzz): Add nightly fuzz workflow with 5 minute budget
(noir-lang/noir#8925)
fix(mem2reg): Keep last store for a used nested array
(noir-lang/noir#8917)
chore(fuzz): Increase the number of deterministic test cases
(noir-lang/noir#8872)
fix: Create calls to `apply` before function values are changed to
fields in defunctionalize (noir-lang/noir#8916)
chore: Add various regression tests for mutable references
(noir-lang/noir#8915)
fix: handle return_data in the interpreter SSA CLI
(noir-lang/noir#8914)
feat(fuzz): Generate references in the AST fuzzer
(noir-lang/noir#8728)
fix(fuzz): Use an inline block to circumvent negation with overflow
(noir-lang/noir#8911)
fix: Inline global arrays with functions at their call site
(noir-lang/noir#8905)
chore: Casting ACIR vs Brillig regression tests
(noir-lang/noir#8903)
fix(mem2reg): Keep last store for reference in array used only in an
array get (noir-lang/noir#8877)
fix: more SSA interpreter fixes
(noir-lang/noir#8904)
chore: Ban dynamic array indices with reference elements
(noir-lang/noir#8888)
chore(ssa_fuzzer): Add SSA validation to the SSA fuzzer
(noir-lang/noir#8901)
chore(fuzz): More metamorphic rules
(noir-lang/noir#8857)
fix: catch unbound type variables during frontend compilation
(noir-lang/noir#8686)
fix: preserve functions which are used in `array_set` instructions
(noir-lang/noir#8891)
fix: assorted SSA interpreter fixes
(noir-lang/noir#8893)
fix(ssa): Signed cast simplification
(noir-lang/noir#8862)
feat: simplify apply function cfg immediately
(noir-lang/noir#8895)
fix: do not hoist control dependent cast
(noir-lang/noir#8886)
fix(licm): Account for negative bounds when checking whether a loop
executes (noir-lang/noir#8889)
chore: Release Noir(1.0.0-beta.7)
(noir-lang/noir#8520)
chore: test noir-ecdsa library in CI
(noir-lang/noir#8859)
chore: Incorrect clippy warning blocking PRs
(noir-lang/noir#8861)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
danielntmd pushed a commit to danielntmd/aztec-packages that referenced this pull request Jul 16, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(LSP): suggest generic type methods
(noir-lang/noir#8948)
fix: Fix if/match tracking in last uses pass
(noir-lang/noir#8935)
fix(ssa): Swap Brillig index shift and DIE in minimal pipeline
(noir-lang/noir#8946)
fix: when macro parse error happens, discard warnings; also preserve
unquoted token locations (noir-lang/noir#8944)
chore: disable noncritical workflow steps which fail on external prs
(noir-lang/noir#8939)
chore(test): Add `--minimal-ssa` to integration tests
(noir-lang/noir#8938)
fix(noirc_evaluator): U128 Binary::And simplification
(noir-lang/noir#8940)
chore: Improve access to data required to extract Noir
(noir-lang/noir#8793)
chore: bump external pinned commits
(noir-lang/noir#8920)
chore(fuzz): Compare print output on failed programs
(noir-lang/noir#8921)
chore(fuzz): Display nightly fuzz status badge
(noir-lang/noir#8932)
fix(fuzz): Consider values returned from Brillig to ACIR as dynamic
(noir-lang/noir#8931)
fix(fuzz): Fix env var name in fuzzing workflow
(noir-lang/noir#8929)
chore(fuzz): Add nightly fuzz workflow with 5 minute budget
(noir-lang/noir#8925)
fix(mem2reg): Keep last store for a used nested array
(noir-lang/noir#8917)
chore(fuzz): Increase the number of deterministic test cases
(noir-lang/noir#8872)
fix: Create calls to `apply` before function values are changed to
fields in defunctionalize (noir-lang/noir#8916)
chore: Add various regression tests for mutable references
(noir-lang/noir#8915)
fix: handle return_data in the interpreter SSA CLI
(noir-lang/noir#8914)
feat(fuzz): Generate references in the AST fuzzer
(noir-lang/noir#8728)
fix(fuzz): Use an inline block to circumvent negation with overflow
(noir-lang/noir#8911)
fix: Inline global arrays with functions at their call site
(noir-lang/noir#8905)
chore: Casting ACIR vs Brillig regression tests
(noir-lang/noir#8903)
fix(mem2reg): Keep last store for reference in array used only in an
array get (noir-lang/noir#8877)
fix: more SSA interpreter fixes
(noir-lang/noir#8904)
chore: Ban dynamic array indices with reference elements
(noir-lang/noir#8888)
chore(ssa_fuzzer): Add SSA validation to the SSA fuzzer
(noir-lang/noir#8901)
chore(fuzz): More metamorphic rules
(noir-lang/noir#8857)
fix: catch unbound type variables during frontend compilation
(noir-lang/noir#8686)
fix: preserve functions which are used in `array_set` instructions
(noir-lang/noir#8891)
fix: assorted SSA interpreter fixes
(noir-lang/noir#8893)
fix(ssa): Signed cast simplification
(noir-lang/noir#8862)
feat: simplify apply function cfg immediately
(noir-lang/noir#8895)
fix: do not hoist control dependent cast
(noir-lang/noir#8886)
fix(licm): Account for negative bounds when checking whether a loop
executes (noir-lang/noir#8889)
chore: Release Noir(1.0.0-beta.7)
(noir-lang/noir#8520)
chore: test noir-ecdsa library in CI
(noir-lang/noir#8859)
chore: Incorrect clippy warning blocking PRs
(noir-lang/noir#8861)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
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.

Min != Full: return value changes from incorrect to correct, inc_rc missing

2 participants