fix: relax connectedness requirement on purity analysis pass by TomAFrench · Pull Request #8667 · noir-lang/noir

TomAFrench · 2025-05-23T14:59:36Z

Description

Problem*

Resolves #8666

Summary*

This PR addresses some fuzzing failures which were caused by functions being made unreachable between the remove_unreachable_functions and purity_analysis passes. The purity_analysis assumes that all functions are reachable from main so this resulted in some functions not being analysed.

We now run the callgraph analysis from all functions which have no callers to ensure that we hit every function in the callgraph.

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 · 2025-05-23T15:54:05Z

functions being made unreachable between the remove_unreachable_functions and purity_analysis passes

Could we just run remove_unreachable_functions before the second purity_analysis pass as an alternative? We could then have additional debug assertions that the Ssa input to purity_analysis has no unreachable functions?

TomAFrench · 2025-05-23T16:00:53Z

We could but I thought it best to remove the precondition on this pass rather than require us to always fulfill it.

vezenovm · 2025-05-23T16:28:23Z

We could but I thought it best to remove the precondition on this pass rather than require us to always fulfill it.

Fair enough. I also agree that is nice to get rid of dependency tracking in this pass as well.

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: `88e381e`	Previous: `a16e848`	Ratio
`test_report_zkpassport_noir_rsa_`	`3` s	`2` s	`1.50`

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

CC: @TomAFrench

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

TomAFrench · 2025-05-23T19:06:41Z

I can deal with the comment in a followup. This PR fixes flakiness which is affecting all PRs.

Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE fix(fmt): correctly format mixed secondary attributes and doc comments (noir-lang/noir#8735) fix!: require types for trait impl associated constants (noir-lang/noir#8734) fix!: Prevent returning references from if expressions (noir-lang/noir#8731) fix: cast signed to u1 follow-up (noir-lang/noir#8730) fix: cast signed to u1 (noir-lang/noir#8720) fix: do not mutate arrays later copied inside other arrays (noir-lang/noir#8701) chore: box `AsTraitPath` and `TypePath` in `ExpressionKind` (noir-lang/noir#8716) fix: general solution for accessing associated constants (noir-lang/noir#8417) fix(ssa): Validate checked signed add/sub is followed by a truncate (noir-lang/noir#8706) chore: add pre- and post-check on `array_set` optimization pass (noir-lang/noir#8714) fix: merge expr bindings with instantiations bindings during monomorphization (noir-lang/noir#8713) fix: better way to do LSP file overrides (noir-lang/noir#8702) fix(fmt): correct indentation when formatting long struct patterns (noir-lang/noir#8711) fix(fuzz): Prevent breaking/continuing out from let blocks in the AST fuzzer (noir-lang/noir#8708) chore: remove override for zero length inputs (noir-lang/noir#8709) feat: Replace converging jmpif with empty block destinations with a single jmp (noir-lang/noir#8613) feat(cli): Add `nargo interpret` command (noir-lang/noir#8700) chore: more 1-tuple printing fixes (noir-lang/noir#8699) chore(fuzz): Fuzz the SSA parser (noir-lang/noir#8647) fix: (SSA parser) translate blocks in logical order rather than syntax order (noir-lang/noir#8668) fix(SSA): show and parse range_check's assert_message (noir-lang/noir#8652) chore: Show the step number in the SSA message (noir-lang/noir#8698) chore(docs): Add pointers to tests (noir-lang/noir#8695) chore(fuzz): Capture comptime print output (noir-lang/noir#8635) fix: nargo expand reexports correctly implemented (noir-lang/noir#8693) feat(cli): Show multiple SSA passes (noir-lang/noir#8692) chore(test): Add regression test for defunctionalize (noir-lang/noir#8691) chore: add an assertion when parsing SSA that all functions are well-formed (noir-lang/noir#8671) feat!: prevent compiling blake3 hashes which barretenberg cannot prove (noir-lang/noir#8690) fix(ssa): Remove the array cache from the function inserter (noir-lang/noir#8607) chore: bump external pinned commits (noir-lang/noir#8684) chore: reenable fuzzing tests in CI (noir-lang/noir#8688) fix: Handle `&mut function` in defunctionalize (noir-lang/noir#8665) fix(defunctionalize): Higher order functions (HOF) dynamic dispatch and HOFs in arrays (noir-lang/noir#8672) chore(ci): avoid running fuzzer tests in the merge queue (noir-lang/noir#8664) fix: Make casts in `comptime` consistent with runtime casts (noir-lang/noir#8669) fix: relax connectedness requirement on purity analysis pass (noir-lang/noir#8667) fix: always use `u32` for indexing arrays in SSA (noir-lang/noir#8633) chore: explicitly pull from `next` branch in aztec-packages (noir-lang/noir#8660) chore(fuzz): Build the dictionary from the SSA (noir-lang/noir#8591) chore(docs): Remove old versioned docs (noir-lang/noir#8061) chore: bump external pinned commits (noir-lang/noir#8634) fix(SSA): don't use string literal if byte is "form feed" ('\f') (noir-lang/noir#8653) chore(defunctionalize): Add regression test for missing lambda variants panic (noir-lang/noir#8642) chore(ci): Do not run ast_fuzzer orig vs. morph in ci (noir-lang/noir#8646) fix: disable underflow fix for fields (noir-lang/noir#8631) fix: revert "fix: error on unused generic in trait impl (noir-lang/noir#8395)" (noir-lang/noir#8636) fix(ssa interpreter): Default to zero when we have an overflowing shl (noir-lang/noir#8638) fix: ensure that purity analysis pass explores all functions (noir-lang/noir#8452) feat: acir_formal_proofs (noir-lang/noir#8140) fix: error on unused generic in trait impl (noir-lang/noir#8395) fix(expand): use re-exports for non-visibile items (noir-lang/noir#8374) END_COMMIT_OVERRIDE --------- Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: Ary Borenszweig <asterite@gmail.com>

fix: relax connectedness requirement on purity analysis pass

25520de

TomAFrench requested a review from a team May 23, 2025 15:00

.

2db138d

vezenovm approved these changes May 23, 2025

View reviewed changes

TomAFrench added this pull request to the merge queue May 23, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 23, 2025

TomAFrench added 5 commits May 23, 2025 17:52

fix: handle unreachable but recursive functions

546f75a

.

ade3ea5

Merge branch 'master' into tf/relax-connectedness-req-on-purity

fc4ddc4

chore: simplify handling of recursive unreachable functions

49e6b09

.

88e381e

TomAFrench enabled auto-merge May 23, 2025 18:47

github-actions bot reviewed May 23, 2025

View reviewed changes

TomAFrench added this pull request to the merge queue May 23, 2025

jfecher reviewed May 23, 2025

View reviewed changes

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

jfecher removed this pull request from the merge queue due to a manual request May 23, 2025

TomAFrench enabled auto-merge May 23, 2025 19:06

TomAFrench added this pull request to the merge queue May 23, 2025

Merged via the queue into master with commit 6cbf5c4 May 23, 2025
118 checks passed

TomAFrench deleted the tf/relax-connectedness-req-on-purity branch May 23, 2025 19:42

noirwhal mentioned this pull request May 23, 2025

chore: Release Noir(1.0.0-beta.7) #8520

Merged

AztecBot mentioned this pull request May 24, 2025

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

Merged

TomAFrench mentioned this pull request May 27, 2025

fix: ensure that purity analysis pass explores all functions #8452

Merged

5 tasks

AztecBot mentioned this pull request Jun 1, 2025

chore(next): release 0.88.0 AztecProtocol/aztec-packages#14495

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: relax connectedness requirement on purity analysis pass#8667

fix: relax connectedness requirement on purity analysis pass#8667
TomAFrench merged 7 commits intomasterfrom
tf/relax-connectedness-req-on-purity

TomAFrench commented May 23, 2025

Uh oh!

vezenovm commented May 23, 2025 •

edited

Loading

Uh oh!

TomAFrench commented May 23, 2025

Uh oh!

vezenovm commented May 23, 2025

Uh oh!

Uh oh!

github-actions bot left a comment

Uh oh!

Uh oh!

Uh oh!

TomAFrench commented May 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

TomAFrench commented May 23, 2025

Description

Problem*

Summary*

Additional Context

Documentation*

PR Checklist*

Uh oh!

vezenovm commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomAFrench commented May 23, 2025

Uh oh!

vezenovm commented May 23, 2025

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

Uh oh!

Uh oh!

TomAFrench commented May 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vezenovm commented May 23, 2025 •

edited

Loading