Merge branch 'master' into michaeljklein/regression_test_7195

Cargo.toml

@@ -175,6 +175,7 @@ tracing = "0.1.40"
 tracing-web = "0.1.3"
 tracing-subscriber = { version = "0.3.18", features = ["env-filter", "json"] }
 rust-embed = "6.6.0"
+petgraph = "0.6"
 
 [profile.dev]
 # This is required to be able to run `cargo test` in acvm_js due to the `locals exceeds maximum` error.

compiler/noirc_evaluator/Cargo.toml

@@ -30,6 +30,7 @@ rayon.workspace = true
 cfg-if.workspace = true
 smallvec = { version = "1.13.2", features = ["serde"] }
 vec-collections = "0.4.3"
+petgraph.workspace = true
 
 [dev-dependencies]
 proptest.workspace = true

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs

@@ -71,7 +71,10 @@ impl FunctionContext {
             Type::Slice(_) => {
                 panic!("ICE: Slice parameters cannot be derived from type information")
             }
-            _ => unimplemented!("Unsupported function parameter/return type {typ:?}"),
+            // Treat functions as field values
+            Type::Function => {
+                BrilligParameter::SingleAddr(get_bit_size_from_ssa_type(&Type::field()))
+            }
         }
     }
 

compiler/noirc_evaluator/src/ssa.rs

@@ -170,6 +170,7 @@ fn optimize_all(builder: SsaBuilder, options: &SsaEvaluatorOptions) -> Result<Ss
             Ssa::evaluate_static_assert_and_assert_constant,
             "`static_assert` and `assert_constant`",
         )?
+        .run_pass(Ssa::purity_analysis, "Purity Analysis")
         .run_pass(Ssa::loop_invariant_code_motion, "Loop Invariant Code Motion")
         .try_run_pass(
             |ssa| ssa.unroll_loops_iteratively(options.max_bytecode_increase_percent),
@@ -190,6 +191,7 @@ fn optimize_all(builder: SsaBuilder, options: &SsaEvaluatorOptions) -> Result<Ss
             "Inlining (2nd)",
         )
         .run_pass(Ssa::remove_if_else, "Remove IfElse")
+        .run_pass(Ssa::purity_analysis, "Purity Analysis (2nd)")
         .run_pass(Ssa::fold_constants, "Constant Folding")
         .run_pass(Ssa::remove_enable_side_effects, "EnableSideEffectsIf removal")
         .run_pass(Ssa::fold_constants_using_constraints, "Constraint Folding")

compiler/noirc_evaluator/src/ssa/function_builder/mod.rs

@@ -24,6 +24,7 @@ use super::{
         instruction::{ConstrainError, InstructionId, Intrinsic},
         types::NumericType,
     },
+    opt::pure::FunctionPurities,
     ssa_gen::Ssa,
 };
 
@@ -44,6 +45,9 @@ pub(crate) struct FunctionBuilder {
     /// Whether instructions are simplified as soon as they are inserted into this builder.
     /// This is true by default unless changed to false after constructing a builder.
     pub(crate) simplify: bool,
+
+    globals: Arc<GlobalsGraph>,
+    purities: Arc<FunctionPurities>,
 }
 
 impl FunctionBuilder {
@@ -53,17 +57,29 @@ impl FunctionBuilder {
     /// right after constructing a new FunctionBuilder.
     pub(crate) fn new(function_name: String, function_id: FunctionId) -> Self {
         let new_function = Function::new(function_name, function_id);
-
         Self {
             current_block: new_function.entry_block(),
             current_function: new_function,
             finished_functions: Vec::new(),
             call_stack: CallStackId::root(),
             error_types: BTreeMap::default(),
             simplify: true,
+            globals: Default::default(),
+            purities: Default::default(),
         }
     }
 
+    /// Create a function builder with a new function created with the same
+    /// name, globals, and function purities taken from an existing function.
+    pub(crate) 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 the runtime of the initial function that is created internally after constructing
     /// the FunctionBuilder. A function's default runtime type is `RuntimeType::Acir(InlineType::Inline)`.
     /// This should only be used immediately following construction of a FunctionBuilder
@@ -74,10 +90,20 @@ impl FunctionBuilder {
     }
 
     pub(crate) fn set_globals(&mut self, globals: Arc<GlobalsGraph>) {
-        for (_, value) in globals.values_iter() {
+        self.globals = globals;
+        self.apply_globals();
+    }
+
+    fn apply_globals(&mut self) {
+        for (_, value) in self.globals.values_iter() {
             self.current_function.dfg.make_global(value.get_type().into_owned());
         }
-        self.current_function.set_globals(globals);
+        self.current_function.set_globals(self.globals.clone());
+    }
+
+    pub(crate) fn set_purities(&mut self, purities: Arc<FunctionPurities>) {
+        self.purities = purities.clone();
+        self.current_function.dfg.set_function_purities(purities);
     }
 
     /// Finish the current function and create a new function.
@@ -100,6 +126,9 @@ impl FunctionBuilder {
         self.call_stack =
             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.apply_globals();
     }
 
     /// Finish the current function and create a new ACIR function.

compiler/noirc_evaluator/src/ssa/ir/dfg.rs

@@ -1,6 +1,10 @@
 use std::{borrow::Cow, sync::Arc};
 
-use crate::ssa::{function_builder::data_bus::DataBus, ir::instruction::SimplifyResult};
+use crate::ssa::{
+    function_builder::data_bus::DataBus,
+    ir::instruction::SimplifyResult,
+    opt::pure::{FunctionPurities, Purity},
+};
 
 use super::{
     basic_block::{BasicBlock, BasicBlockId},
@@ -104,6 +108,9 @@ pub(crate) struct DataFlowGraph {
     pub(crate) data_bus: DataBus,
 
     pub(crate) globals: Arc<GlobalsGraph>,
+
+    #[serde(skip)]
+    pub(crate) function_purities: Arc<FunctionPurities>,
 }
 
 /// The GlobalsGraph contains the actual global data.
@@ -757,6 +764,14 @@ impl DataFlowGraph {
             _ => None,
         }
     }
+
+    pub(crate) fn set_function_purities(&mut self, purities: Arc<FunctionPurities>) {
+        self.function_purities = purities;
+    }
+
+    pub(crate) fn purity_of(&self, function: FunctionId) -> Option<Purity> {
+        self.function_purities.get(&function).copied()
+    }
 }
 
 impl std::ops::Index<InstructionId> for DataFlowGraph {

compiler/noirc_evaluator/src/ssa/ir/function.rs

@@ -114,6 +114,7 @@ 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
     }
 

compiler/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -10,7 +10,7 @@ use fxhash::FxHasher64;
 use iter_extended::vecmap;
 use noirc_frontend::hir_def::types::Type as HirType;
 
-use crate::ssa::opt::flatten_cfg::value_merger::ValueMerger;
+use crate::ssa::opt::{flatten_cfg::value_merger::ValueMerger, pure::Purity};
 
 use super::{
     basic_block::BasicBlockId,
@@ -156,6 +156,14 @@ impl Intrinsic {
     /// Intrinsics which only have a side effect due to the chance that
     /// they can fail a constraint can be deduplicated.
     pub(crate) fn can_be_deduplicated(&self, deduplicate_with_predicate: bool) -> bool {
+        match self.purity() {
+            Purity::Pure => true,
+            Purity::PureWithPredicate => deduplicate_with_predicate,
+            Purity::Impure => false,
+        }
+    }
+
+    pub(crate) fn purity(&self) -> Purity {
         match self {
             // These apply a constraint in the form of ACIR opcodes, but they can be deduplicated
             // if the inputs are the same. If they depend on a side effect variable (e.g. because
@@ -170,19 +178,20 @@ impl Intrinsic {
                 BlackBoxFunc::MultiScalarMul
                 | BlackBoxFunc::EmbeddedCurveAdd
                 | BlackBoxFunc::RecursiveAggregation,
-            ) => deduplicate_with_predicate,
+            ) => Purity::PureWithPredicate,
 
             // Operations that remove items from a slice don't modify the slice, they just assert it's non-empty.
             Intrinsic::SlicePopBack | Intrinsic::SlicePopFront | Intrinsic::SliceRemove => {
-                deduplicate_with_predicate
+                Purity::PureWithPredicate
             }
 
             Intrinsic::AssertConstant
             | Intrinsic::StaticAssert
             | Intrinsic::ApplyRangeConstraint
-            | Intrinsic::AsWitness => deduplicate_with_predicate,
+            | Intrinsic::AsWitness => Purity::PureWithPredicate,
 
-            _ => !self.has_side_effects(),
+            _ if self.has_side_effects() => Purity::Impure,
+            _ => Purity::Pure,
         }
     }
 
@@ -405,6 +414,9 @@ impl Instruction {
 
             Call { func, .. } => match dfg[*func] {
                 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),
                 _ => true, // Be conservative and assume other functions can have side effects.
             },
 
@@ -472,6 +484,12 @@ impl Instruction {
                 Value::Intrinsic(intrinsic) => {
                     intrinsic.can_be_deduplicated(deduplicate_with_predicate)
                 }
+                Value::Function(id) => match function.dfg.purity_of(id) {
+                    Some(Purity::Pure) => true,
+                    Some(Purity::PureWithPredicate) => deduplicate_with_predicate,
+                    Some(Purity::Impure) => false,
+                    None => false,
+                },
                 _ => false,
             },
 
@@ -605,7 +623,7 @@ impl Instruction {
             Instruction::EnableSideEffectsIf { .. } | Instruction::ArraySet { .. } => true,
 
             Instruction::Call { func, .. } => match dfg[*func] {
-                Value::Function(_) => true,
+                Value::Function(id) => !matches!(dfg.purity_of(id), Some(Purity::Pure)),
                 Value::Intrinsic(intrinsic) => {
                     matches!(intrinsic, Intrinsic::SliceInsert | Intrinsic::SliceRemove)
                 }