From ad372644677e168a1a734b48ecff96f7874ec0d3 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Aug 2026 10:59:03 -0300 Subject: [PATCH 1/4] fix(ssa): store function purities on the Ssa so entry-point clones keep 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 --- .../src/ssa/function_builder/mod.rs | 13 +-- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 23 +---- .../noirc_evaluator/src/ssa/ir/function.rs | 1 - .../noirc_evaluator/src/ssa/ir/instruction.rs | 15 +++- .../noirc_evaluator/src/ssa/ir/printer.rs | 12 ++- .../noirc_evaluator/src/ssa/ir/target_cost.rs | 12 ++- .../src/ssa/opt/basic_conditional.rs | 29 ++++--- .../src/ssa/opt/brillig_array_get_and_set.rs | 2 +- .../src/ssa/opt/brillig_entry_points.rs | 82 +++++++++++++++--- .../opt/brillig_function_specialization.rs | 11 ++- .../src/ssa/opt/constant_folding/mod.rs | 64 ++++++++++---- .../ssa/opt/constant_folding/result_cache.rs | 12 ++- .../src/ssa/opt/defunctionalize.rs | 63 ++++++-------- compiler/noirc_evaluator/src/ssa/opt/die.rs | 29 +++++-- .../src/ssa/opt/die/array_oob_checks.rs | 2 +- .../src/ssa/opt/loop_invariant.rs | 38 ++++++--- .../src/ssa/opt/normalize_value_ids.rs | 48 +++++------ .../src/ssa/opt/preprocess_fns.rs | 2 +- compiler/noirc_evaluator/src/ssa/opt/pure.rs | 85 ++++++++++--------- .../src/ssa/parser/into_ssa.rs | 8 +- .../src/ssa/ssa_gen/program.rs | 19 ++++- 21 files changed, 344 insertions(+), 226 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 3380fe13a7e..480a9d7f512 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -26,7 +26,6 @@ use super::{ instruction::{ConstrainError, InstructionId, Intrinsic}, types::NumericType, }, - opt::pure::FunctionPurities, ssa_gen::Ssa, }; @@ -57,7 +56,6 @@ pub struct FunctionBuilder { allow_malformed_simplify: bool, globals: Arc, - purities: Arc, } impl FunctionBuilder { @@ -76,7 +74,6 @@ impl FunctionBuilder { simplify: true, allow_malformed_simplify: false, globals: Default::default(), - purities: Default::default(), } } @@ -90,13 +87,11 @@ impl FunctionBuilder { } /// Create a function builder with a new function created with the same - /// name, globals, and function purities taken from an existing function. + /// name and globals taken from an existing function. pub fn from_existing(function: &Function, function_id: FunctionId) -> Self { let mut this = Self::new(function.name().to_owned(), function_id); this.set_globals(function.dfg.globals.clone()); - this.purities = function.dfg.function_purities.clone(); this.current_function.set_runtime(function.runtime()); - this.current_function.dfg.set_function_purities(this.purities.clone()); this.set_allow_malformed_simplify(function.dfg.allow_malformed_simplify); this.current_function.dfg.allow_constant_return = function.dfg.allow_constant_return; this @@ -127,11 +122,6 @@ impl FunctionBuilder { self.current_function.set_globals(self.globals.clone()); } - pub(crate) fn set_purities(&mut self, purities: Arc) { - self.purities = purities.clone(); - self.current_function.dfg.set_function_purities(purities); - } - /// Finish the current function and create a new function. /// /// A `FunctionBuilder` can always only work on one function at a time, so care @@ -154,7 +144,6 @@ impl FunctionBuilder { self.current_function.dfg.call_stack_data.get_or_insert_locations(&call_stack); self.finished_functions.push(old_function); - self.current_function.dfg.set_function_purities(self.purities.clone()); self.current_function.dfg.allow_malformed_simplify = self.allow_malformed_simplify; self.apply_globals(); } diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index f6e643e4693..3ffa45adc8c 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -3,11 +3,8 @@ use std::{borrow::Cow, sync::Arc}; use crate::{ brillig::assert_u32, ssa::{ - RuntimeError, - function_builder::data_bus::DataBus, - ir::function::Function, + RuntimeError, function_builder::data_bus::DataBus, ir::function::Function, ir::instruction::ArrayOffset, - opt::pure::{FunctionPurities, Purity}, }, }; @@ -118,9 +115,6 @@ pub(crate) struct DataFlowGraph { pub(crate) globals: Arc, - #[serde(skip)] - pub(crate) function_purities: Arc, - /// Indicate whether the Brillig array index offset optimizations have been performed. pub(crate) brillig_arrays_offset: bool, @@ -1006,21 +1000,6 @@ impl DataFlowGraph { } } - pub(crate) fn set_function_purities(&mut self, purities: Arc) { - self.function_purities = purities; - } - - /// Returns the purity of `function` as observed from this function (the caller). - /// - /// This is the callee's own purity, except that a pure Brillig function called from an ACIR - /// function is observed as [Purity::PureWithPredicate]: the call lowers to a predicated - /// `Opcode::BrilligCall` whose outputs are left unconstrained when the predicate is disabled, - /// so the result is predicate-dependent from an ACIR caller's perspective. From a Brillig - /// caller (whose calls are not predicated) the function's true purity is observed. - pub(crate) fn purity_of(&self, function: FunctionId) -> Option { - self.function_purities.purity_of(function, self.runtime()) - } - /// Determine the appropriate [`ArrayOffset`] to use for indexing an array or vector. pub(crate) fn array_offset(&self, array: ValueId, index: ValueId) -> ArrayOffset { if !self.runtime.is_brillig() diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 355c752c24e..96d90ddb2cf 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -132,7 +132,6 @@ impl Function { let mut new_function = Function::new(another.name.clone(), id); new_function.set_runtime(another.runtime()); new_function.set_globals(another.dfg.globals.clone()); - new_function.dfg.set_function_purities(another.dfg.function_purities.clone()); new_function.dfg.brillig_arrays_offset = another.dfg.brillig_arrays_offset; new_function.dfg.allow_constant_return = another.dfg.allow_constant_return; new_function diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 06c7ebfa61e..d2ef0c2a8e5 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -10,7 +10,10 @@ use acvm::{ use iter_extended::vecmap; use noirc_frontend::hir_def::types::Type as HirType; -use crate::ssa::{ir::integer::IntegerConstant, opt::pure::Purity}; +use crate::ssa::{ + ir::integer::IntegerConstant, + opt::pure::{FunctionPurities, Purity}, +}; use super::{ basic_block::BasicBlockId, @@ -592,7 +595,11 @@ impl Instruction { } /// Indicates if the instruction has a side effect, ie. it can fail, or it interacts with memory. - pub(crate) fn has_side_effects(&self, dfg: &DataFlowGraph) -> bool { + pub(crate) fn has_side_effects( + &self, + dfg: &DataFlowGraph, + purities: &FunctionPurities, + ) -> bool { use Instruction::*; match self { @@ -608,7 +615,9 @@ impl Instruction { Value::Intrinsic(intrinsic) => intrinsic.has_side_effects(), // Functions known to be pure have no side effects. // `PureWithPredicates` functions may still have side effects. - Value::Function(function) => dfg.purity_of(function) != Some(Purity::Pure), + Value::Function(function) => { + purities.purity_of(function, dfg.runtime()) != Some(Purity::Pure) + } _ => true, // Be conservative and assume other functions can have side effects. }, diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 0a22832bdf8..e6490b33b60 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -13,6 +13,7 @@ use crate::ssa::{ instruction::ArrayOffset, types::{NumericType, Type}, }, + opt::pure::FunctionPurities, }; use super::{ @@ -69,7 +70,7 @@ impl Display for Printer<'_> { } for function in self.ssa.functions.values() { - display_function(function, self.fm, f)?; + display_function(function, self.fm, Some(&self.ssa.function_purities), f)?; writeln!(f)?; } Ok(()) @@ -78,17 +79,22 @@ impl Display for Printer<'_> { impl Display for Function { fn fmt(&self, f: &mut Formatter<'_>) -> Result { - display_function(self, None, f) + display_function(self, None, None, f) } } /// Helper function for Function's Display impl to pretty-print the function with the given formatter. +/// +/// Purities live on the [Ssa][crate::ssa::ssa_gen::Ssa] rather than on each function, so a +/// standalone [Function] display (which has no `purities`) omits the purity keyword. fn display_function( function: &Function, files: Option<&fm::FileManager>, + purities: Option<&FunctionPurities>, f: &mut Formatter, ) -> Result { - if let Some(purity) = function.dfg.purity_of(function.id()) { + let purity = purities.and_then(|purities| purities.intrinsic_purity_of(function.id())); + if let Some(purity) = purity { writeln!(f, "{} {purity} fn {} {} {{", function.runtime(), function.name(), function.id())?; } else { writeln!(f, "{} fn {} {} {{", function.runtime(), function.name(), function.id())?; diff --git a/compiler/noirc_evaluator/src/ssa/ir/target_cost.rs b/compiler/noirc_evaluator/src/ssa/ir/target_cost.rs index 619ae915c7b..24a6d736394 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/target_cost.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/target_cost.rs @@ -8,6 +8,8 @@ //! If ACIR cost estimation is needed in the future, it can be added here alongside the //! Brillig estimates. +use crate::ssa::opt::pure::FunctionPurities; + use super::{ dfg::DataFlowGraph, function::Function, @@ -37,7 +39,11 @@ impl Instruction { /// /// Div/Mod and Shl/Shr are blocked unconditionally — even when `has_side_effects` /// would allow them (e.g. known non-zero divisor), they are rarely worth flattening. - pub(crate) fn can_flatten_in_conditional(&self, dfg: &DataFlowGraph) -> bool { + pub(crate) fn can_flatten_in_conditional( + &self, + dfg: &DataFlowGraph, + purities: &FunctionPurities, + ) -> bool { match self { Instruction::EnableSideEffectsIf { .. } => { if dfg.runtime().is_brillig() { @@ -58,10 +64,10 @@ impl Instruction { Instruction::Binary(binary) => match binary.operator { BinaryOp::Div | BinaryOp::Mod | BinaryOp::Shl | BinaryOp::Shr => false, - _ => !self.has_side_effects(dfg), + _ => !self.has_side_effects(dfg, purities), }, - _ => !self.has_side_effects(dfg), + _ => !self.has_side_effects(dfg, purities), } } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/basic_conditional.rs b/compiler/noirc_evaluator/src/ssa/opt/basic_conditional.rs index 913b6b81160..ffdb767f253 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/basic_conditional.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/basic_conditional.rs @@ -26,7 +26,7 @@ use crate::ssa::{ types::Type, value::ValueId, }, - opt::flatten_cfg::WorkList, + opt::{flatten_cfg::WorkList, pure::FunctionPurities}, }; use super::flatten_cfg::Context; @@ -47,7 +47,7 @@ impl Ssa { self.functions.values().filter(|f| f.is_no_predicates()).map(|f| f.id()).collect(); for function in self.functions.values_mut() { - flatten_function(function, &no_predicates); + flatten_function(function, &no_predicates, &self.function_purities); } self } @@ -76,6 +76,7 @@ fn is_conditional( block: BasicBlockId, cfg: &ControlFlowGraph, function: &Function, + purities: &FunctionPurities, ) -> Option { // A conditional must end with a JmpIf let Some(TerminatorInstruction::JmpIf { @@ -113,8 +114,8 @@ fn is_conditional( // \ / // next_then // We check that the cost of the flattened code is lower than the cost of the branches - let cost_left = block_flatten_cost(*then_destination, &function.dfg)?; - let cost_right = block_flatten_cost(*else_destination, &function.dfg)?; + let cost_left = block_flatten_cost(*then_destination, &function.dfg, purities)?; + let cost_right = block_flatten_cost(*else_destination, &function.dfg, purities)?; // Compute the actual branching overhead for this conditional: // Flattening eliminates: JmpIf + then's Jmp + else's Jmp // Flattening adds merge (IfElse) ops only for exit params where branches differ. @@ -151,7 +152,7 @@ fn is_conditional( if !then_arguments.is_empty() || !else_arguments.is_empty() { return None; } - let cost = block_flatten_cost(*then_destination, &function.dfg)?; + let cost = block_flatten_cost(*then_destination, &function.dfg, purities)?; // Flattening eliminates: JmpIf + then's Jmp; adds IfElse per exit param let then_term_cost = function.dfg[*then_destination].unwrap_terminator().cost(); let merge_cost = function.dfg.block_parameters(*else_destination).len() * 3; @@ -180,7 +181,7 @@ fn is_conditional( if !then_arguments.is_empty() || !else_arguments.is_empty() { return None; } - let cost = block_flatten_cost(*else_destination, &function.dfg)?; + let cost = block_flatten_cost(*else_destination, &function.dfg, purities)?; // Flattening eliminates: JmpIf + else's Jmp; adds IfElse per exit param let else_term_cost = function.dfg[*else_destination].unwrap_terminator().cost(); let merge_cost = function.dfg.block_parameters(*then_destination).len() * 3; @@ -254,7 +255,11 @@ fn differing_merge_cost( /// reaching them as an ICE.) Hoisting an `inc_rc` only ever raises a reference /// count, so the later `array_set` copies rather than mutating in place — sound, /// and guarded by the `rc_invariant::array_set` validator. -fn block_flatten_cost(block: BasicBlockId, dfg: &DataFlowGraph) -> Option { +fn block_flatten_cost( + block: BasicBlockId, + dfg: &DataFlowGraph, + purities: &FunctionPurities, +) -> Option { let mut cost: u32 = 0; for instruction_id in dfg[block].instructions() { let instruction = &dfg[*instruction_id]; @@ -267,7 +272,7 @@ fn block_flatten_cost(block: BasicBlockId, dfg: &DataFlowGraph) -> Option { continue; } - if !instruction.can_flatten_in_conditional(dfg) { + if !instruction.can_flatten_in_conditional(dfg, purities) { return None; } @@ -277,7 +282,11 @@ fn block_flatten_cost(block: BasicBlockId, dfg: &DataFlowGraph) -> Option { } /// Identifies all simple conditionals in the function and flattens them -fn flatten_function(function: &mut Function, no_predicates: &HashSet) { +fn flatten_function( + function: &mut Function, + no_predicates: &HashSet, + purities: &FunctionPurities, +) { // This pass is dedicated to brillig functions if !function.runtime().is_brillig() { return; @@ -297,7 +306,7 @@ fn flatten_function(function: &mut Function, no_predicates: &HashSet processed.insert(block); // Identify the simple conditionals - if let Some(conditional) = is_conditional(block, &cfg, function) { + if let Some(conditional) = is_conditional(block, &cfg, function, purities) { // no need to check the branches, process the join block directly stack.push(conditional.block_exit); conditionals.push(conditional); diff --git a/compiler/noirc_evaluator/src/ssa/opt/brillig_array_get_and_set.rs b/compiler/noirc_evaluator/src/ssa/opt/brillig_array_get_and_set.rs index a0dc84f75eb..bd9ea47ab77 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/brillig_array_get_and_set.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/brillig_array_get_and_set.rs @@ -301,7 +301,7 @@ mod tests { let func = ssa.main(); let b0 = &func.dfg[func.entry_block()]; let instruction = &func.dfg[b0.instructions()[0]]; - instruction.has_side_effects(&func.dfg) + instruction.has_side_effects(&func.dfg, &ssa.function_purities) } let ssa = Ssa::from_str(src).unwrap(); diff --git a/compiler/noirc_evaluator/src/ssa/opt/brillig_entry_points.rs b/compiler/noirc_evaluator/src/ssa/opt/brillig_entry_points.rs index 70a863fb654..c1397999eac 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/brillig_entry_points.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/brillig_entry_points.rs @@ -231,10 +231,18 @@ fn build_calls_to_update( for (entry_point, functions_to_clone) in functions_to_clone_map { for old_id in functions_to_clone { let function = ssa.functions[&old_id].clone(); - ssa.add_fn(|id| { + let new_id = ssa.add_fn(|id| { calls_to_update.insert((entry_point, old_id), id); Function::clone_with_id(id, &function) }); + // A clone's body is instruction-for-instruction identical to its original's, + // so it keeps the original's purity. + if let Some(purity) = ssa.function_purities.intrinsic_purity_of(old_id) { + ssa.function_purities.insert_purity(new_id, purity); + } + if ssa.function_purities.is_brillig_function(old_id) { + ssa.function_purities.insert_brillig_function(new_id); + } } } @@ -839,7 +847,7 @@ mod tests { b3(v2: u1): return v2 } - brillig(inline) fn func_2 f3 { + brillig(inline) predicate_pure fn func_2 f3 { b0(v0: u1, v1: u32): v4 = eq v1, u32 0 jmpif v4 then: b1(), else: b2() @@ -853,7 +861,7 @@ mod tests { b3(v2: u1): return v2 } - brillig(inline) fn func_1 f4 { + brillig(inline) predicate_pure fn func_1 f4 { b0(v0: u1, v1: u32): v4 = eq v1, u32 0 jmpif v4 then: b1(), else: b2() @@ -867,7 +875,7 @@ mod tests { b3(v2: u1): return v2 } - brillig(inline) fn func_2 f5 { + brillig(inline) predicate_pure fn func_2 f5 { b0(v0: u1, v1: u32): v4 = eq v1, u32 0 jmpif v4 then: b1(), else: b2() @@ -881,7 +889,7 @@ mod tests { b3(v2: u1): return v2 } - brillig(inline) fn func_1 f6 { + brillig(inline) predicate_pure fn func_1 f6 { b0(v0: u1, v1: u32): v4 = eq v1, u32 0 jmpif v4 then: b1(), else: b2() @@ -928,7 +936,7 @@ mod tests { // We want no shared callees between entry points. // Each Brillig entry point (f1 and f2 called from f0) should have its own // specialized function call graph. - assert_ssa_snapshot!(ssa, @" + assert_ssa_snapshot!(ssa, @r" acir(inline) predicate_pure fn main f0 { b0(): call f1(Field 1) @@ -946,24 +954,24 @@ mod tests { call f3(Field 1) return } - brillig(inline) fn bar f3 { + brillig(inline) predicate_pure fn bar f3 { b0(v0: Field): call f4(Field 1) call f3(Field 1) return } - brillig(inline) fn foo f4 { + brillig(inline) predicate_pure fn foo f4 { b0(v0: Field): call f3(v0) return } - brillig(inline) fn bar f5 { + brillig(inline) predicate_pure fn bar f5 { b0(v0: Field): call f6(Field 1) call f5(Field 1) return } - brillig(inline) fn foo f6 { + brillig(inline) predicate_pure fn foo f6 { b0(v0: Field): call f5(v0) return @@ -1160,4 +1168,58 @@ mod tests { } "); } + + #[test] + fn cloned_functions_keep_their_purity() { + // Two Brillig entry points share `inner_func`, so the pass clones it once per + // entry point. A clone's body is identical to its original's, so it must keep + // the original's purity status rather than end up with none at all. + let src = " + acir(inline) predicate_pure fn main f0 { + b0(v0: Field): + v2 = call f1(v0) -> Field + v4 = call f2(v0) -> Field + v5 = add v2, v4 + return v5 + } + brillig(inline) pure fn entry_point_one f1 { + b0(v0: Field): + v2 = call f3(v0) -> Field + return v2 + } + brillig(inline) pure fn entry_point_two f2 { + b0(v0: Field): + v2 = call f3(v0) -> Field + return v2 + } + brillig(inline) pure fn inner_func f3 { + b0(v0: Field): + v1 = mul v0, v0 + return v1 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + + let original_purities: std::collections::HashMap = ssa + .functions + .values() + .map(|function| { + let purity = ssa.function_purities.intrinsic_purity_of(function.id()); + (function.name().to_string(), purity) + }) + .collect(); + + let ssa = ssa.brillig_entry_point_analysis(); + + for (id, function) in &ssa.functions { + let purity = ssa.function_purities.intrinsic_purity_of(*id); + assert!(purity.is_some(), "function {} {id} has no purity status", function.name()); + assert_eq!( + purity, + original_purities[function.name()], + "function {} {id} does not have the same purity as its original", + function.name() + ); + } + } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/brillig_function_specialization.rs b/compiler/noirc_evaluator/src/ssa/opt/brillig_function_specialization.rs index 47ce35f4d3d..3201cdb2eb1 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/brillig_function_specialization.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/brillig_function_specialization.rs @@ -49,6 +49,7 @@ use crate::ssa::{ types::NumericType, value::{Value, ValueId, ValueMapping}, }, + opt::pure::FunctionPurities, ssa_gen::Ssa, }; @@ -237,6 +238,7 @@ fn optimize_clone( function: &mut Function, constant_folding_max_iter: usize, all_functions: &BTreeMap, + purities: &FunctionPurities, ) { use crate::ssa::interpreter::{Interpreter, InterpreterOptions}; @@ -261,7 +263,7 @@ fn optimize_clone( ); interpreter.interpret_globals().expect("ICE: Interpreter failed to interpret globals"); - function.constant_fold(false, constant_folding_max_iter, &mut interpreter); + function.constant_fold(false, constant_folding_max_iter, &mut interpreter, purities); // Simplify CFG again after folding to merge any newly-dead blocks. // Note: We skip per-function DIE here because it is intentionally private @@ -307,7 +309,12 @@ fn create_specialized_clones( substitute_constants(&mut clone, &key.constants); // Run per-function optimization passes on the clone. - optimize_clone(&mut clone, constant_folding_max_iter, &ssa.functions); + optimize_clone( + &mut clone, + constant_folding_max_iter, + &ssa.functions, + &ssa.function_purities, + ); let specialized_cost = clone.cost(); let savings_percent = if original_cost > specialized_cost { diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs index 5cedf5b4385..c30519a0ab5 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs @@ -33,7 +33,10 @@ use crate::ssa::{ types::{NumericType, Type}, value::{Value, ValueId, ValueMapping}, }, - opt::{LoopOrder, Loops, pure::Purity}, + opt::{ + LoopOrder, Loops, + pure::{FunctionPurities, Purity}, + }, ssa_gen::Ssa, visit_once_priority_queue::VisitOncePriorityQueue, }; @@ -81,7 +84,7 @@ impl Ssa { interpreter.interpret_globals().expect("ICE: Interpreter failed to interpret globals"); for function in self.functions.values_mut() { - function.constant_fold(false, max_iter, &mut interpreter); + function.constant_fold(false, max_iter, &mut interpreter, &self.function_purities); } self } @@ -111,7 +114,7 @@ impl Ssa { interpreter.interpret_globals().expect("ICE: Interpreter failed to interpret globals"); for function in self.functions.values_mut() { - function.constant_fold(true, max_iter, &mut interpreter); + function.constant_fold(true, max_iter, &mut interpreter, &self.function_purities); } self } @@ -139,6 +142,7 @@ impl Function { use_constraint_info: bool, max_iter: usize, interpreter: &mut Interpreter, + purities: &FunctionPurities, ) { let loops = Loops::find_all(self, LoopOrder::OutsideIn); @@ -160,7 +164,7 @@ impl Function { let dom = loops.dom; let mutated_types = find_mutated_block_param_array_types(self); - let mut context = Context::new(use_constraint_info, mutated_types.clone()); + let mut context = Context::new(use_constraint_info, mutated_types.clone(), purities); context.enqueue(&dom, [self.entry_block()]); @@ -195,7 +199,7 @@ impl Function { // Create a fresh context, so values cached towards the end are not visible to blocks during a revisit. // For example reusing the cache could be problematic when using constraint info, as it could make the // original content simplify out based on its own prior assertion of a value being a constant. - context = Context::new(use_constraint_info, mutated_types.clone()); + context = Context::new(use_constraint_info, mutated_types.clone(), purities); context.values_to_replace = values_to_replace; context.enqueue(&dom, blocks_to_revisit); } @@ -273,7 +277,11 @@ fn find_mutated_block_param_array_types(function: &Function) -> HashSet { result } -struct Context { +struct Context<'a> { + /// The purity of every function in the program, consulted when deciding whether calls + /// can be deduplicated or their cached results reused. + purities: &'a FunctionPurities, + /// Keeps track of visited blocks and blocks to visit. /// Prioritizes them based on their Reverse Post Order rank, which ensures /// that we see them in a consistent order even during restarts. @@ -321,9 +329,14 @@ struct Context { mutated_block_param_array_types: HashSet, } -impl Context { - fn new(use_constraint_info: bool, mutated_block_param_array_types: HashSet) -> Self { +impl<'a> Context<'a> { + fn new( + use_constraint_info: bool, + mutated_block_param_array_types: HashSet, + purities: &'a FunctionPurities, + ) -> Self { Self { + purities, use_constraint_info, block_queue: Default::default(), constraint_simplification_mappings: Default::default(), @@ -417,9 +430,15 @@ impl Context { // If a copy of this instruction exists earlier in the block, then reuse the previous results. let runtime_is_brillig = dfg.runtime().is_brillig(); let predicate = self.cache_predicate(*side_effects_enabled_var, &instruction, dfg); - if let Some(cache_result) = - self.cached_instruction_results.get(dfg, dom, id, &instruction, predicate, block) - { + if let Some(cache_result) = self.cached_instruction_results.get( + dfg, + dom, + id, + &instruction, + predicate, + block, + self.purities, + ) { match cache_result { CacheResult::Cached { results: cached, .. } => { // Guard against self-deduplication: if the cached results are exactly @@ -682,13 +701,16 @@ impl Context { } } - self.cached_instruction_results - .remove_possibly_mutated_cached_make_arrays(instruction, dfg); + self.cached_instruction_results.remove_possibly_mutated_cached_make_arrays( + instruction, + dfg, + self.purities, + ); // If the instruction doesn't have side-effects and if it won't interact with enable_side_effects during acir_gen, // we cache the results so we can reuse them if the same instruction appears again later in the block. // Others have side effects representing failure, which are implicit in the ACIR code and can also be deduplicated. - let can_be_deduplicated = can_be_deduplicated(instruction, dfg); + let can_be_deduplicated = can_be_deduplicated(instruction, dfg, self.purities); let use_constraint_info = self.use_constraint_info; let is_safe_make_array = match instruction { @@ -727,6 +749,7 @@ impl Context { last_instruction, predicate, block, + self.purities, ) { self.blocks_to_revisit.insert(dominator); @@ -828,7 +851,11 @@ enum CanBeDeduplicated { /// These can be deduplicated because they implicitly depend on the predicate, not only when the caller uses the /// predicate variable as a key to cache results. However, to avoid tight coupling between passes, we make the deduplication /// conditional on whether the caller wants the predicate to be taken into account or not. -fn can_be_deduplicated(instruction: &Instruction, dfg: &DataFlowGraph) -> CanBeDeduplicated { +fn can_be_deduplicated( + instruction: &Instruction, + dfg: &DataFlowGraph, + purities: &FunctionPurities, +) -> CanBeDeduplicated { use Instruction::*; match instruction { @@ -848,12 +875,12 @@ fn can_be_deduplicated(instruction: &Instruction, dfg: &DataFlowGraph) -> CanBeD }, // A call to a user-defined function from an ACIR caller lowers to a predicated // `Opcode::Call` or `Opcode::BrilligCall`, which leaves the callee's outputs - // unconstrained when the predicate is disabled. `DataFlowGraph::purity_of` already + // unconstrained when the predicate is disabled. `FunctionPurities::purity_of` already // reflects this: from an ACIR caller a pure Brillig callee is observed as // `PureWithPredicate`, so a callee seen here as `Pure` is one that genuinely does not // depend on the predicate (e.g. any callee from a Brillig caller, whose calls are not // predicated) and can be deduplicated freely. - Value::Function(id) => match dfg.purity_of(id) { + Value::Function(id) => match purities.purity_of(id, dfg.runtime()) { Some(Purity::Pure) if dfg.runtime().is_brillig() => CanBeDeduplicated::Always, Some(Purity::Pure | Purity::PureWithPredicate) => { CanBeDeduplicated::UnderSamePredicate @@ -1713,7 +1740,8 @@ mod test { InterpreterOptions::default(), std::io::empty(), ); - ssa.main_mut().constant_fold(false, 1, &mut empty_interpreter); + let purities = ssa.function_purities.clone(); + ssa.main_mut().constant_fold(false, 1, &mut empty_interpreter, &purities); // 1. v9 is a duplicate of v5 -> hoisted to b0 // 2. v13 is a duplicate of v9 -> immediately deduplicated because it's now in b0 diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding/result_cache.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding/result_cache.rs index 6c5b798b546..aec5bf5cd33 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding/result_cache.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding/result_cache.rs @@ -11,7 +11,7 @@ use crate::ssa::{ types::Type, value::{Value, ValueId}, }, - opt::pure::Purity, + opt::pure::{FunctionPurities, Purity}, }; use rustc_hash::FxHashMap as HashMap; @@ -107,6 +107,7 @@ pub(super) struct InstructionResultCache(HashMap, block: BasicBlockId, + purities: &FunctionPurities, ) -> Option { let results_for_instruction = self.0.get(&CacheKeyRef::from(instruction))?; let cached_results = results_for_instruction.get(&predicate)?.get( block, dom, - instruction.has_side_effects(dfg), + instruction.has_side_effects(dfg, purities), ); cached_results.filter(|results| { @@ -181,6 +183,7 @@ impl InstructionResultCache { &mut self, instruction: &Instruction, dfg: &DataFlowGraph, + purities: &FunctionPurities, ) { use Instruction::{ArraySet, Call, MakeArray, Store}; @@ -256,7 +259,10 @@ impl InstructionResultCache { let mutates_arguments = match &dfg[*func] { // A non-pure user-defined function may mutate its array arguments in place. Value::Function(func_id) => { - matches!(dfg.purity_of(*func_id), None | Some(Purity::Impure)) + matches!( + purities.purity_of(*func_id, dfg.runtime()), + None | Some(Purity::Impure) + ) } // The vector mutators (`push`/`pop`/`insert`/`remove`) write through their // vector argument when its copy-on-write reference count is 1, even though they diff --git a/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs b/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs index fc9840422dd..c0f64de33bc 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs @@ -51,7 +51,7 @@ use crate::ssa::{ types::{NumericType, Type}, value::{Value, ValueId}, }, - opt::pure::{FunctionPurities, Purity}, + opt::pure::Purity, ssa_gen::Ssa, }; use rustc_hash::FxHashMap as HashMap; @@ -105,7 +105,7 @@ impl Ssa { let variants = find_variants(&self); // Generate the apply functions for the provided variants - let (apply_functions, purities) = create_apply_functions(&mut self, variants)?; + let apply_functions = create_apply_functions(&mut self, variants)?; // Setup the pass context let context = DefunctionalizationContext { apply_functions }; @@ -113,11 +113,6 @@ impl Ssa { // Run defunctionalization over all functions in the SSA context.defunctionalize_all(&mut self); - let purities = Arc::new(purities); - for function in self.functions.values_mut() { - function.dfg.set_function_purities(purities.clone()); - } - // Check that we have established the properties expected from this pass. #[cfg(debug_assertions)] self.functions.values().for_each(defunctionalize_post_check); @@ -467,18 +462,11 @@ fn find_dynamic_dispatches(func: &Function) -> BTreeSet { /// - [`ApplyFunctions`] keyed by each function's signature _before_ functions are changed /// into field types. The inner apply function itself will have its defunctionalized type, /// with function values represented as field values. -/// - [FunctionPurities] with purities that must be set to all functions in the SSA, -/// as this function might have created dummy pure functions. fn create_apply_functions( ssa: &mut Ssa, variants_map: Variants, -) -> Result<(ApplyFunctions, FunctionPurities), RuntimeError> { +) -> Result { let mut apply_functions = HashMap::default(); - let mut purities = if ssa.functions.is_empty() { - FunctionPurities::default() - } else { - (*ssa.functions.iter().next().unwrap().1.dfg.function_purities).clone() - }; for ((signature, caller_runtime), variants) in variants_map { // Calling an ACIR function from a Brillig runtime is not allowed. @@ -539,7 +527,7 @@ fn create_apply_functions( // If no variants exist for a dynamic call we leave removing those dead calls and parameters to DIE. // However, we have to construct a dummy function for these dead calls as to keep a well formed SSA // and to not break the semantics of other SSA passes before DIE is reached. - create_dummy_function(ssa, defunctionalized_signature, caller_runtime, &mut purities) + create_dummy_function(ssa, defunctionalized_signature, caller_runtime) }; apply_functions.insert( (signature, caller_runtime), @@ -547,7 +535,7 @@ fn create_apply_functions( ); } - Ok((apply_functions, purities)) + Ok(apply_functions) } /// Collect the function variants that can be called from a given runtime. @@ -767,17 +755,16 @@ fn create_dummy_function( ssa: &mut Ssa, signature: Signature, caller_runtime: RuntimeType, - purities: &mut FunctionPurities, ) -> FunctionId { - ssa.add_fn(|id| { - let mut function_builder = FunctionBuilder::new("apply_dummy".to_string(), id); + // Set the runtime of the dummy function. The dummy function is expect to always be simplified out + // but we let the caller set the runtime here as to match Noir's runtime semantics. + let runtime = match caller_runtime { + RuntimeType::Acir(_) => RuntimeType::Acir(InlineType::InlineAlways), + RuntimeType::Brillig(_) => RuntimeType::Brillig(InlineType::InlineAlways), + }; - // Set the runtime of the dummy function. The dummy function is expect to always be simplified out - // but we let the caller set the runtime here as to match Noir's runtime semantics. - let runtime = match caller_runtime { - RuntimeType::Acir(_) => RuntimeType::Acir(InlineType::InlineAlways), - RuntimeType::Brillig(_) => RuntimeType::Brillig(InlineType::InlineAlways), - }; + let id = ssa.add_fn(|id| { + let mut function_builder = FunctionBuilder::new("apply_dummy".to_string(), id); function_builder.set_runtime(runtime); // The remaining dummy function parameters are the actual parameters of the function call without any variants. @@ -785,22 +772,24 @@ fn create_dummy_function( // was set to be inlined before the call to it was removed by DIE. vecmap(signature.params, |typ| function_builder.add_parameter(typ)); - // We can mark the dummy function pure as all it does is return. - // As the dummy function is just meant to be a placeholder for any calls to - // higher-order functions without variants, we want the function to be marked pure - // so that dead instruction elimination can remove any calls to it. - purities.insert_purity(id, Purity::Pure); - if runtime.is_brillig() { - purities.insert_brillig_function(id); - } - let results = vecmap(signature.returns, |typ| make_dummy_return_data(&mut function_builder, &typ)); function_builder.terminate_with_return(results); function_builder.current_function - }) + }); + + // We can mark the dummy function pure as all it does is return. + // As the dummy function is just meant to be a placeholder for any calls to + // higher-order functions without variants, we want the function to be marked pure + // so that dead instruction elimination can remove any calls to it. + ssa.function_purities.insert_purity(id, Purity::Pure); + if runtime.is_brillig() { + ssa.function_purities.insert_brillig_function(id); + } + + id } /// Construct a dummy value to be returned from the placeholder function for calls to invalid lambda references. @@ -1931,7 +1920,7 @@ mod tests { let variants = find_variants(&ssa); assert_eq!(variants.len(), 2); - let (apply_functions, _purities) = create_apply_functions(&mut ssa, variants).unwrap(); + let apply_functions = create_apply_functions(&mut ssa, variants).unwrap(); // This was 1 before this bug was fixed. assert_eq!(apply_functions.len(), 2); } diff --git a/compiler/noirc_evaluator/src/ssa/opt/die.rs b/compiler/noirc_evaluator/src/ssa/opt/die.rs index af89c83452e..6072da7651f 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -59,7 +59,10 @@ use crate::ssa::{ types::NumericType, value::{Value, ValueId}, }, - opt::{die::array_oob_checks::should_insert_oob_check, pure::Purity}, + opt::{ + die::array_oob_checks::should_insert_oob_check, + pure::{FunctionPurities, Purity}, + }, ssa_gen::Ssa, }; @@ -104,7 +107,8 @@ impl Ssa { .functions .par_iter_mut() .map(|(id, func)| { - let unused_params = func.dead_instruction_elimination(true); + let unused_params = + func.dead_instruction_elimination(true, &self.function_purities); let mut result = DIEResult::default(); result.unused_parameters.insert(*id, unused_params); @@ -141,8 +145,9 @@ impl Function { fn dead_instruction_elimination( &mut self, insert_out_of_bounds_checks: bool, + purities: &FunctionPurities, ) -> HashMap> { - let mut context = Context::new(); + let mut context = Context::new(purities); for call_data in &self.dfg.data_bus.call_data { context.mark_used_instruction_results(&self.dfg, call_data.array_id); @@ -179,7 +184,7 @@ impl Function { // instructions (we don't want to remove those checks, or instructions that are // dependencies of those checks) if inserted_out_of_bounds_checks { - return self.dead_instruction_elimination(false); + return self.dead_instruction_elimination(false, purities); } context.remove_rc_instructions(&mut self.dfg); @@ -193,7 +198,11 @@ struct DIEResult { unused_parameters: HashMap>>, } /// Per function context for tracking unused values and which instructions to remove. -struct Context { +struct Context<'a> { + /// The purity of every function in the program, consulted to decide whether a call + /// can be eliminated when its results are unused. + purities: &'a FunctionPurities, + used_values: HashSet, instructions_to_remove: HashSet, @@ -214,9 +223,10 @@ struct Context { parameter_keep_list: HashMap>, } -impl Context { - fn new() -> Self { +impl<'a> Context<'a> { + fn new(purities: &'a FunctionPurities) -> Self { Self { + purities, used_values: HashSet::default(), instructions_to_remove: HashSet::default(), rc_instructions: Vec::new(), @@ -321,7 +331,7 @@ impl Context { fn is_unused(&self, instruction_id: InstructionId, function: &Function) -> bool { let instruction = &function.dfg[instruction_id]; - can_be_eliminated_if_unused(instruction, function, &self.used_values) && { + can_be_eliminated_if_unused(instruction, function, &self.used_values, self.purities) && { let results = function.dfg.instruction_results(instruction_id); results.iter().all(|result| !self.used_values.contains(result)) } @@ -427,6 +437,7 @@ fn can_be_eliminated_if_unused( instruction: &Instruction, function: &Function, used_values: &HashSet, + purities: &FunctionPurities, ) -> bool { use Instruction::*; match instruction { @@ -492,7 +503,7 @@ fn can_be_eliminated_if_unused( // We use purity to determine whether functions contain side effects. // If we have an impure function, we cannot remove it even if it is unused. - Value::Function(function_id) => match function.dfg.purity_of(function_id) { + Value::Function(function_id) => match purities.purity_of(function_id, function.runtime()) { Some(Purity::Pure) => true, Some(Purity::PureWithPredicate) => false, Some(Purity::Impure) => false, diff --git a/compiler/noirc_evaluator/src/ssa/opt/die/array_oob_checks.rs b/compiler/noirc_evaluator/src/ssa/opt/die/array_oob_checks.rs index a075d0d720e..f74fab099e8 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/die/array_oob_checks.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/die/array_oob_checks.rs @@ -13,7 +13,7 @@ use crate::ssa::{ opt::die::Context, }; -impl Context { +impl Context<'_> { /// Replaces unused ArrayGet/ArraySet instructions with out of bounds checks. /// Returns `true` if at least one check was inserted. /// Because some `ArrayGet` might happen in groups (for composite types), if just diff --git a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs index 3be0398d6a8..26dabce9010 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs @@ -99,7 +99,7 @@ use crate::ssa::{ types::{NumericType, Type}, value::{Value, ValueId}, }, - opt::pure::Purity, + opt::pure::{FunctionPurities, Purity}, }; use acvm::{FieldElement, acir::AcirField}; use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet}; @@ -113,7 +113,7 @@ impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn loop_invariant_code_motion(mut self) -> Ssa { for function in self.functions.values_mut() { - function.loop_invariant_code_motion(); + function.loop_invariant_code_motion(&self.function_purities); } self @@ -121,14 +121,14 @@ impl Ssa { } impl Function { - pub(super) fn loop_invariant_code_motion(&mut self) { - Loops::find_all(self, LoopOrder::OutsideIn).hoist_loop_invariants(self); + pub(super) fn loop_invariant_code_motion(&mut self, purities: &FunctionPurities) { + Loops::find_all(self, LoopOrder::OutsideIn).hoist_loop_invariants(self, purities); } } impl Loops { - fn hoist_loop_invariants(mut self, function: &mut Function) { - let mut context = LoopInvariantContext::new(function, &self.yet_to_unroll); + fn hoist_loop_invariants(mut self, function: &mut Function, purities: &FunctionPurities) { + let mut context = LoopInvariantContext::new(function, &self.yet_to_unroll, purities); // The loops should be sorted by the number of blocks. // We want to access outer nested loops first, which we do by popping @@ -190,6 +190,10 @@ impl Loop { struct LoopInvariantContext<'f> { inserter: FunctionInserter<'f>, + /// The purity of every function in the program, consulted to decide whether a call + /// has side effects. + purities: &'f FunctionPurities, + /// Maps an outer loop's induction variable to its [`LoopBounds`]. /// /// Used by inner loops to reason about operations on an outer loop's induction variable — @@ -373,7 +377,7 @@ impl PostDominanceFrontiers { } impl<'f> LoopInvariantContext<'f> { - fn new(function: &'f mut Function, loops: &[Loop]) -> Self { + fn new(function: &'f mut Function, loops: &[Loop], purities: &'f FunctionPurities) -> Self { let cfg = ControlFlowGraph::with_function(function); let post_dom_frontiers = PostDominanceFrontiers::with_function(function); let true_value = @@ -382,6 +386,7 @@ impl<'f> LoopInvariantContext<'f> { function.dfg.make_constant(FieldElement::zero(), NumericType::Unsigned { bit_size: 1 }); let mut context = Self { inserter: FunctionInserter::new(function), + purities, outer_induction_variables: HashMap::default(), all_induction_variables: HashMap::default(), cfg, @@ -475,7 +480,8 @@ impl<'f> LoopInvariantContext<'f> { // Note that purity is dependent on the instruction ordering, which is expected because // it tells us exactly if there is a side-effect instruction before the current one. if !block_context.is_impure { - block_context.is_impure = dfg[instruction_id].has_side_effects(dfg); + block_context.is_impure = + dfg[instruction_id].has_side_effects(dfg, self.purities); } self.inserter.push_instruction(instruction_id, *block, true); } @@ -657,7 +663,7 @@ impl<'f> LoopInvariantContext<'f> { dfg[*block] .instructions() .iter() - .any(|instruction| dfg[*instruction].has_side_effects(dfg)) + .any(|instruction| dfg[*instruction].has_side_effects(dfg, self.purities)) }); // For control dependence we don't consider the header: all blocks are obviously control @@ -829,7 +835,7 @@ impl<'f> LoopInvariantContext<'f> { return (true, returns_array); } - match can_be_hoisted(&instruction, dfg) { + match can_be_hoisted(&instruction, dfg, self.purities) { Yes => (true, returns_array), No => (false, false), WithPredicate => { @@ -960,7 +966,11 @@ impl From for CanBeHoistedResult { /// This differs from `can_be_deduplicated` as that method assumes there is a matching instruction /// with the same inputs. Hoisting is for lone instructions, meaning a mislabeled hoist could cause /// unexpected failures if the instruction was never meant to be executed. -fn can_be_hoisted(instruction: &Instruction, dfg: &DataFlowGraph) -> CanBeHoistedResult { +fn can_be_hoisted( + instruction: &Instruction, + dfg: &DataFlowGraph, + purities: &FunctionPurities, +) -> CanBeHoistedResult { use CanBeHoistedResult::*; use Instruction::*; @@ -976,7 +986,7 @@ fn can_be_hoisted(instruction: &Instruction, dfg: &DataFlowGraph) -> CanBeHoiste Call { func, .. } => { let purity = match dfg[*func] { Value::Intrinsic(intrinsic) => Some(intrinsic.purity()), - Value::Function(id) => dfg.purity_of(id), + Value::Function(id) => purities.purity_of(id, dfg.runtime()), // A `#[pure]` oracle behaves like `PureWithPredicate`: its return is a // function of its arguments, so hoisting it from a non-empty loop is sound. Value::ForeignFunction { pure: true, .. } => Some(Purity::PureWithPredicate), @@ -2494,7 +2504,7 @@ mod tests { let mut ssa = Ssa::from_str(src).unwrap(); let function = ssa.functions.get_mut(&ssa.main_id).unwrap(); let mut loops = Loops::find_all(function, LoopOrder::OutsideIn); - let ctx = LoopInvariantContext::new(function, &loops.yet_to_unroll); + let ctx = LoopInvariantContext::new(function, &loops.yet_to_unroll, &ssa.function_purities); let pre_header = BasicBlockId::new(0); let loop_ = loops.yet_to_unroll.pop().unwrap(); let mut loop_ctx = LoopContext::new(&ctx.inserter, &ctx.cfg, &loop_, pre_header); @@ -3010,7 +3020,7 @@ mod tests { typ: Type::Array(Arc::new(vec![]), SemanticLength(0)), }; - assert_eq!(can_be_hoisted(&instruction, &function.dfg), result); + assert_eq!(can_be_hoisted(&instruction, &function.dfg, &ssa.function_purities), result); } /// Regression for noir-claude#244, found by the AST fuzzer `pass_vs_prev` on seed diff --git a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs index 6c715c80a2e..46f387d6034 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs @@ -6,7 +6,7 @@ //! may increase the ID counter so that later passes start at different offsets, //! even if they contain the same SSA code. -use std::{collections::BTreeMap, sync::Arc}; +use std::collections::BTreeMap; use crate::ssa::{ ir::{ @@ -16,10 +16,11 @@ use crate::ssa::{ post_order::PostOrder, value::{Value, ValueId}, }, - opt::pure::{FunctionPurities, Purity}, + opt::pure::FunctionPurities, ssa_gen::Ssa, }; use iter_extended::vecmap; + use itertools::Itertools; use rustc_hash::FxHashMap as HashMap; @@ -33,6 +34,8 @@ impl Ssa { context.normalize_ids(function); } self.functions = context.functions.into_btree(); + self.function_purities = + remap_purities(std::mem::take(&mut self.function_purities), &context.new_ids); } } @@ -62,37 +65,12 @@ struct IdMaps { impl Context { fn populate_functions(&mut self, functions: &BTreeMap) { - let Some(old_purities) = functions.iter().next().map(|f| f.1.dfg.function_purities.clone()) - else { - return; - }; - let mut new_purities = FunctionPurities::default(); - let old_intrinsic: HashMap = - old_purities.intrinsic_purities().map(|(id, purity)| (*id, *purity)).collect(); - for (id, function) in functions { self.functions.insert_with_id(|new_id| { self.new_ids.function_ids.insert(*id, new_id); - - if let Some(purity) = old_intrinsic.get(id) { - new_purities.insert_purity(new_id, *purity); - } - Function::clone_signature(new_id, function) }); } - - // Remap the set of Brillig functions onto the new ids. - for old_id in old_purities.brillig_function_ids() { - if let Some(new_id) = self.new_ids.function_ids.get(old_id) { - new_purities.insert_brillig_function(*new_id); - } - } - - let new_purities = Arc::new(new_purities); - for new_id in self.new_ids.function_ids.values() { - self.functions[*new_id].dfg.set_function_purities(new_purities.clone()); - } } fn normalize_ids(&mut self, old_function: &mut Function) { @@ -171,6 +149,22 @@ impl Context { } } +/// Rebuild the purity map against the new function ids. +fn remap_purities(old_purities: FunctionPurities, new_ids: &IdMaps) -> FunctionPurities { + let mut new_purities = FunctionPurities::default(); + for (old_id, purity) in old_purities.intrinsic_purities() { + if let Some(new_id) = new_ids.function_ids.get(old_id) { + new_purities.insert_purity(*new_id, *purity); + } + } + for old_id in old_purities.brillig_function_ids() { + if let Some(new_id) = new_ids.function_ids.get(old_id) { + new_purities.insert_brillig_function(*new_id); + } + } + new_purities +} + impl IdMaps { fn populate_blocks( &mut self, diff --git a/compiler/noirc_evaluator/src/ssa/opt/preprocess_fns.rs b/compiler/noirc_evaluator/src/ssa/opt/preprocess_fns.rs index 1f983ed8793..5c31c9606e8 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/preprocess_fns.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/preprocess_fns.rs @@ -55,7 +55,7 @@ impl Ssa { // Help unrolling determine bounds. function.as_vector_optimization(); // Prepare for unrolling - function.loop_invariant_code_motion(); + function.loop_invariant_code_motion(&self.function_purities); // Clear out constant jmpifs to ensure that loops are properly unrolled. function.simplify_function(); // We might not be able to unroll all loops without fully inlining them, so ignore errors. diff --git a/compiler/noirc_evaluator/src/ssa/opt/pure.rs b/compiler/noirc_evaluator/src/ssa/opt/pure.rs index 65fcabbc7b4..3709bbb2e69 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/pure.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/pure.rs @@ -10,8 +10,6 @@ //! Performing this pass after defunctionalization may also help more function calls be //! identified as calling known pure functions. -use std::sync::Arc; - use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet}; use crate::ssa::ir::call_graph::CallGraph; @@ -31,11 +29,7 @@ impl Ssa { /// This is purely an analysis pass on its own but can help future optimizations. #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn purity_analysis(mut self) -> Ssa { - let purities = Arc::new(compute_function_purities(&self)); - - for function in self.functions.values_mut() { - function.dfg.set_function_purities(purities.clone()); - } + self.function_purities = compute_function_purities(&self); #[cfg(debug_assertions)] purity_analysis_post_check(&self); @@ -119,8 +113,8 @@ fn function_contains_unbounded_loop(function: &Function) -> bool { /// Otherwise panics. #[cfg(debug_assertions)] fn purity_analysis_post_check(ssa: &Ssa) { - if let Some((id, _)) = - ssa.functions.iter().find(|(id, function)| function.dfg.purity_of(**id).is_none()) + if let Some(id) = + ssa.functions.keys().find(|id| ssa.function_purities.intrinsic_purity_of(**id).is_none()) { panic!("Function {id} does not have a purity status") } @@ -133,7 +127,7 @@ fn purity_analysis_post_check(ssa: &Ssa) { /// Brillig function called from an ACIR function lowers to a predicated `Opcode::BrilligCall` /// whose outputs are unconstrained when the predicate is disabled, so from an ACIR caller it /// must be observed as [Purity::PureWithPredicate] even though the function itself is pure. -/// See [crate::ssa::ir::dfg::DataFlowGraph::purity_of]. +/// See [Self::purity_of]. #[derive(Debug, Default, Clone)] pub(crate) struct FunctionPurities { purities: HashMap, @@ -171,6 +165,17 @@ impl FunctionPurities { Some(purity) } + /// The purity recorded for `function`, without the call-site projection applied by + /// [Self::purity_of]. + pub(crate) fn intrinsic_purity_of(&self, function: FunctionId) -> Option { + self.purities.get(&function).copied() + } + + /// Whether `function` is recorded as having a Brillig runtime. + pub(crate) fn is_brillig_function(&self, function: FunctionId) -> bool { + self.brillig_functions.contains(&function) + } + /// Whether no purities have been recorded. pub(crate) fn is_empty(&self) -> bool { self.purities.is_empty() @@ -180,8 +185,7 @@ impl FunctionPurities { /// /// Used only for id-remapping ([crate::ssa::opt::normalize_value_ids]) and for validating /// hand-written purity annotations in the SSA parser — never for optimization decisions, which - /// must observe the call-site purity via [Self::purity_of] (or - /// [crate::ssa::ir::dfg::DataFlowGraph::purity_of]). + /// must observe the call-site purity via [Self::purity_of]. pub(crate) fn intrinsic_purities(&self) -> impl Iterator { self.purities.iter() } @@ -273,7 +277,7 @@ impl Function { // A function's purity reflects the function itself. The predicate sensitivity of calling // a Brillig function from ACIR (its `Opcode::BrilligCall` yields unconstrained outputs // when the predicate is disabled) is a property of the ACIR calling opcode, not of the - // callee, so it is applied at the call site instead (see `DataFlowGraph::purity_of` and + // callee, so it is applied at the call site instead (see `FunctionPurities::purity_of` and // the `Instruction::Call` handling below). let mut result = Purity::Pure; @@ -598,7 +602,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Impure); assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::Impure); assert_eq!(purities.purities[&FunctionId::test_new(2)], Purity::Impure); @@ -703,7 +707,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Impure); assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::Impure); assert_eq!(purities.purities[&FunctionId::test_new(2)], Purity::Impure); @@ -727,7 +731,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; // Empty Brillig functions are genuinely pure. assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Pure); assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::Pure); @@ -757,7 +761,7 @@ mod tests { let ssa = Ssa::from_str_no_validation(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Impure); assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::Impure); } @@ -782,7 +786,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Impure); assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::Impure); } @@ -806,7 +810,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Impure); // Brillig functions have a starting purity of PureWithPredicate assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::PureWithPredicate); @@ -830,7 +834,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::PureWithPredicate); assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::PureWithPredicate); } @@ -876,7 +880,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::PureWithPredicate); assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::PureWithPredicate); assert_eq!(purities.purities[&FunctionId::test_new(2)], Purity::PureWithPredicate); @@ -922,7 +926,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::PureWithPredicate); assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::PureWithPredicate); assert_eq!(purities.purities[&FunctionId::test_new(2)], Purity::PureWithPredicate); @@ -968,7 +972,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; // All must be impure due to the cycle involved f3 when returns a reference. assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::Impure); assert_eq!(purities.purities[&FunctionId::test_new(2)], Purity::Impure); @@ -1006,7 +1010,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; // All must be impure due to the cycle involved f3 when returns a reference. assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::Impure); assert_eq!(purities.purities[&FunctionId::test_new(2)], Purity::Impure); @@ -1035,7 +1039,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; // The ACIR `main` calls a Brillig function, so its own result is predicate-dependent. assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::PureWithPredicate); // The Brillig function itself is genuinely pure. @@ -1044,7 +1048,7 @@ mod tests { // Observed from the ACIR caller, however, the predicated `BrilligCall` opcode means the // pure Brillig callee behaves as `PureWithPredicate`. assert_eq!( - ssa.main().dfg.purity_of(FunctionId::test_new(1)), + ssa.function_purities.purity_of(FunctionId::test_new(1), ssa.main().runtime()), Some(Purity::PureWithPredicate) ); } @@ -1071,12 +1075,15 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Pure); assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::Pure); // A Brillig caller observes the callee's true purity, since its calls are not predicated. - assert_eq!(ssa.main().dfg.purity_of(FunctionId::test_new(1)), Some(Purity::Pure)); + assert_eq!( + ssa.function_purities.purity_of(FunctionId::test_new(1), ssa.main().runtime()), + Some(Purity::Pure) + ); } #[test] @@ -1103,7 +1110,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; // Even though the functions referenced by the function values are pure // we assume the worse case for functions containing calls to function values. assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Impure); @@ -1126,7 +1133,7 @@ mod tests { let ssa = Ssa::from_str(&src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::PureWithPredicate); } @@ -1315,7 +1322,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::PureWithPredicate); } @@ -1334,7 +1341,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Impure); } @@ -1356,7 +1363,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::PureWithPredicate); } @@ -1377,7 +1384,7 @@ mod tests { let ssa = Ssa::from_str_no_validation(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Impure); } @@ -1405,7 +1412,7 @@ mod tests { let ssa = Ssa::from_str_no_validation(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(1)], Purity::PureWithPredicate); } @@ -1432,7 +1439,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Pure); } @@ -1459,7 +1466,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::PureWithPredicate); } @@ -1487,7 +1494,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Pure); } @@ -1514,7 +1521,7 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; + let purities = &ssa.function_purities; assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::PureWithPredicate); } } diff --git a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs index af55f62a86d..05dff613dde 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs @@ -72,7 +72,7 @@ struct Translator { globals_graph: Arc, error_selector_counter: u64, - purities: Arc, + purities: FunctionPurities, } impl Translator { @@ -162,9 +162,6 @@ impl Translator { // Does not matter what ID we use here. let globals = Function::new("globals".to_owned(), main_id); - let purities = Arc::new(purities); - builder.set_purities(purities.clone()); - let mut translator = Self { builder, functions, @@ -209,8 +206,6 @@ impl Translator { } } - self.builder.set_purities(self.purities.clone()); - self.translate_function_body(function) } @@ -690,6 +685,7 @@ impl Translator { fn finish(self) -> Ssa { let mut ssa = self.builder.finish().generate_entry_point_index(); + ssa.function_purities = self.purities; // Normalize the IDs so we have a better chance of matching the SSA we parsed // after the step-by-step reconstruction done during translation. This assumes diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs index 1e59cbf437a..13a1ccf8d65 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs @@ -6,10 +6,13 @@ use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet}; use serde::{Deserialize, Serialize}; use serde_with::serde_as; -use crate::ssa::ir::{ - function::{Function, FunctionId}, - map::AtomicCounter, - value::Value, +use crate::ssa::{ + ir::{ + function::{Function, FunctionId}, + map::AtomicCounter, + value::Value, + }, + opt::pure::FunctionPurities, }; use noirc_frontend::hir_def::types::Type as HirType; @@ -38,6 +41,13 @@ pub struct Ssa { /// ABI not the actual SSA IR. #[serde(skip)] pub error_selector_to_type: BTreeMap, + /// The purity of every function, as computed by [Ssa::purity_analysis][crate::ssa::opt::pure]. + /// + /// This is empty until the first purity analysis runs. Passes that add functions must record + /// a purity for them here (or be followed by another purity analysis) so that the map never + /// goes stale. + #[serde(skip)] + pub(crate) function_purities: FunctionPurities, } impl Ssa { @@ -58,6 +68,7 @@ impl Ssa { next_id: AtomicCounter::starting_after(max_id), entry_point_to_generated_index: BTreeMap::new(), error_selector_to_type: error_types, + function_purities: FunctionPurities::default(), } } From 3da2e88240acb5b35914c1e8172def3bfdb82327 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Aug 2026 11:15:14 -0300 Subject: [PATCH 2/4] Fix docs --- compiler/noirc_evaluator/src/ssa/ir/printer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index e6490b33b60..7f115ece3af 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -85,7 +85,7 @@ impl Display for Function { /// Helper function for Function's Display impl to pretty-print the function with the given formatter. /// -/// Purities live on the [Ssa][crate::ssa::ssa_gen::Ssa] rather than on each function, so a +/// Purities live on the [Ssa] rather than on each function, so a /// standalone [Function] display (which has no `purities`) omits the purity keyword. fn display_function( function: &Function, From 5f0cbd41068c81b1086f1a0e476c87c45ef99651 Mon Sep 17 00:00:00 2001 From: AztecBot Date: Wed, 19 Aug 2026 19:01:44 +0000 Subject: [PATCH 3/4] chore: adopt Ssa-owned FunctionPurities in tests added on master The purity.rs interpreter test and two pure.rs regression tests introduced on master after this branch was cut still reached FunctionPurities through the per-DFG field this branch removed. Rewrite them to the new Ssa-owned shape: - purity.rs: assign directly to `ssa.function_purities` instead of cloning an Arc into every function's DFG. - pure.rs (two tests): assert through `FunctionPurities::intrinsic_purity_of` since the inner map is not part of its public surface. Also thread `Option<&FunctionPurities>` through Interpreter and consult it from call_function; the `new_from_functions` entry point (used by SSA-only tests) leaves it `None`, which the caller safely projects to Impure. --- .../src/ssa/interpreter/tests/purity.rs | 7 +------ compiler/noirc_evaluator/src/ssa/opt/pure.rs | 12 ++++++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/interpreter/tests/purity.rs b/compiler/noirc_evaluator/src/ssa/interpreter/tests/purity.rs index 72cb63236b1..536cfd10f69 100644 --- a/compiler/noirc_evaluator/src/ssa/interpreter/tests/purity.rs +++ b/compiler/noirc_evaluator/src/ssa/interpreter/tests/purity.rs @@ -6,8 +6,6 @@ //! invalidates its results), so these tests inject hand-crafted purities instead of //! running the real analysis, which would classify the functions correctly. -use std::sync::Arc; - use crate::ssa::{ interpreter::{IResults, errors::InterpreterError, intrinsics::check_intrinsic_mutation_label}, ir::{function::FunctionId, instruction::Intrinsic}, @@ -35,10 +33,7 @@ fn interpret_with_injected_purities( } } - let map = Arc::new(map); - for function in ssa.functions.values_mut() { - function.dfg.set_function_purities(map.clone()); - } + ssa.function_purities = map; ssa.interpret(args) } diff --git a/compiler/noirc_evaluator/src/ssa/opt/pure.rs b/compiler/noirc_evaluator/src/ssa/opt/pure.rs index 9a46a7f30ef..ed4a2179a23 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/pure.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/pure.rs @@ -860,8 +860,10 @@ mod tests { let ssa = Ssa::from_str(&src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; - assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::Impure); + assert_eq!( + ssa.function_purities.intrinsic_purity_of(FunctionId::test_new(0)), + Some(Purity::Impure) + ); } #[test] @@ -881,8 +883,10 @@ mod tests { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.purity_analysis(); - let purities = &ssa.main().dfg.function_purities; - assert_eq!(purities.purities[&FunctionId::test_new(0)], Purity::PureWithPredicate); + assert_eq!( + ssa.function_purities.intrinsic_purity_of(FunctionId::test_new(0)), + Some(Purity::PureWithPredicate) + ); } #[test] From badfa16d7fce338e660fa16b83816798b94f91ed Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 20 Aug 2026 09:45:14 -0300 Subject: [PATCH 4/4] Fix docs --- compiler/noirc_evaluator/src/ssa/interpreter/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs b/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs index 82834f3a7f8..b4311a45773 100644 --- a/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs @@ -101,8 +101,7 @@ struct CallContext { /// invalidated its results, so the interpreter reports it as an /// [InterpreterError::PurityViolation] rather than treating it as program behavior. struct PureScope { - /// The purity recorded for the called function, as observed by its caller - /// (see [DataFlowGraph::purity_of]). + /// The purity recorded for the called function, as observed by its caller. purity: Purity, /// Storage identities of everything reachable from the call's arguments at