Skip to content

fix(ssa): store function purities on the Ssa so entry-point clones keep their purity - #13493

Open
asterite wants to merge 3 commits into
masterfrom
ab/purities-on-ssa
Open

fix(ssa): store function purities on the Ssa so entry-point clones keep their purity#13493
asterite wants to merge 3 commits into
masterfrom
ab/purities-on-ssa

Conversation

@asterite

@asterite asterite commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

This ends up in a bit more code, but like this there's no chance of us forgetting to copy purities across all function dfgs.

Problem Resolved

Resolves https://github.com/noir-lang/noir-claude/issues/1652

brillig_entry_point_analysis clones Brillig functions under fresh FunctionIds, but function purities are stored in an id-keyed table and the pass runs after the last purity_analysis, so nothing ever registers the clones: they finish compilation with purity_of() == None. Every consumer treats a missing purity as impure, so the drift is only a missed optimization, but it silently breaks the "every function has a purity status" invariant that purity_analysis_post_check asserts — the check just never runs after the cloning pass. The in-tree duplicate_recursive_shared_entry_points snapshot had the drift baked in (clones printed without predicate_pure).

Summary of Changes

This revives the design of #9357: FunctionPurities now lives once on Ssa as a plain field rather than behind an Arc shared into every DataFlowGraph. That removes the whole class of "forgot to redistribute the purity Arc" bugs and lets passes that mint functions keep the map in sync directly:

  • Ssa::purity_analysis writes self.function_purities; the per-DFG function_purities/set_function_purities/purity_of and FunctionBuilder::set_purities plumbing is gone.
  • Instruction::has_side_effects and Instruction::can_flatten_in_conditional take a &FunctionPurities parameter, threaded from each pass (requires_acir_gen_predicate no longer consults purity, so it is unchanged).
  • defunctionalize and normalize_value_ids update/remap ssa.function_purities directly instead of cloning and redistributing the shared Arc.
  • The fix: the clone loop in brillig_entry_point_analysis copies each original's intrinsic purity and Brillig membership onto the clone's new id — sound because a clone's body is instruction-for-instruction identical to its original's at creation.
  • Regression test cloned_functions_keep_their_purity asserts that after the pass every function has a purity equal to its original's (verified failing before the fix), and the duplicate_recursive_shared_entry_points* snapshots now show the clones keeping predicate_pure.

One cosmetic consequence: the standalone impl Display for Function no longer prints a purity keyword (a lone Function cannot reach the Ssa-owned map); the Ssa-level printer is unaffected.

User Documentation

Check one:

  • No user documentation needed.
  • Documented in docs/.
  • [For Experimental Features] Documentation tracking issue created:

PR Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

🤖 Generated with Claude Code

…ep their purity

`brillig_entry_point_analysis` clones Brillig functions under fresh
`FunctionId`s, but the purity table is keyed by id and the pass runs after
the last `purity_analysis`, so every clone finished compilation with
`purity_of() == None`. All consumers treat a missing purity as impure, so
the drift was only a missed optimization, but it silently broke the
"every function has a purity status" invariant that
`purity_analysis_post_check` asserts.

Fix this structurally by reviving the design of #9357: `FunctionPurities`
now lives once on `Ssa` instead of behind an `Arc` shared into every
`DataFlowGraph`, so passes that mint functions can keep the map in sync
directly. `brillig_entry_point_analysis` now copies each original's purity
and Brillig membership onto its clones, and the
`duplicate_recursive_shared_entry_points*` snapshots show the clones
keeping `predicate_pure`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@asterite
asterite marked this pull request as draft August 10, 2026 14:00
@asterite
asterite marked this pull request as ready for review August 10, 2026 14:18
@asterite
asterite requested a review from TomAFrench August 10, 2026 14:43
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.

1 participant