chore: do not inline acir calls in brillig

compiler/noirc_evaluator/src/errors.rs

@@ -85,6 +85,14 @@ pub enum RuntimeError {
         "Only constant indices are supported when indexing an array containing reference values"
     )]
     DynamicIndexingWithReference { call_stack: CallStack },
+    #[error(
+        "Calling constrained function '{constrained}' from the unconstrained function '{unconstrained}'"
+    )]
+    UnconstrainedCallingConstrained {
+        call_stack: CallStack,
+        constrained: String,
+        unconstrained: String,
+    },
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, Hash)]
@@ -203,7 +211,8 @@ impl RuntimeError {
             | RuntimeError::BreakOrContinue { call_stack }
             | RuntimeError::DynamicIndexingWithReference { call_stack }
             | RuntimeError::UnknownReference { call_stack }
-            | RuntimeError::RecursionLimit { call_stack, .. } => call_stack,
+            | RuntimeError::RecursionLimit { call_stack, .. }
+            | RuntimeError::UnconstrainedCallingConstrained { call_stack, .. } => call_stack,
         }
     }
 }

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

@@ -63,10 +63,17 @@ impl CallGraph {
         let mut ids_to_indices = HashMap::default();
         let mut indices_to_ids = HashMap::default();
 
-        for function in dependencies.keys() {
-            let index = graph.add_node(*function);
-            ids_to_indices.insert(*function, index);
-            indices_to_ids.insert(index, *function);
+        for dep in &dependencies {
+            let index = graph.add_node(*(dep.0));
+            ids_to_indices.insert(*(dep.0), index);
+            indices_to_ids.insert(index, *(dep.0));
+            for callee in dep.1.keys() {
+                if !ids_to_indices.contains_key(callee) {
+                    let callee_index = graph.add_node(*callee);
+                    ids_to_indices.insert(*callee, callee_index);
+                    indices_to_ids.insert(callee_index, *callee);
+                }
+            }
         }
 
         // Create edges from caller -> called

compiler/noirc_evaluator/src/ssa/mod.rs

@@ -110,7 +110,7 @@ pub fn primary_passes(options: &SsaEvaluatorOptions) -> Vec<SsaPass> {
         SsaPass::new(Ssa::expand_signed_checks, "expand signed checks"),
         SsaPass::new(Ssa::remove_unreachable_functions, "Removing Unreachable Functions"),
         SsaPass::new(Ssa::defunctionalize, "Defunctionalization"),
-        SsaPass::new(Ssa::inline_simple_functions, "Inlining simple functions")
+        SsaPass::new_try(Ssa::inline_simple_functions, "Inlining simple functions")
             .and_then(Ssa::remove_unreachable_functions),
         // BUG: Enabling this mem2reg causes an integration test failure in aztec-package; see:
         // https://github.com/AztecProtocol/aztec-packages/pull/11294#issuecomment-2622809518

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

@@ -6,9 +6,11 @@
 //! - Contains no more than [MAX_INSTRUCTIONS] instructions
 //! - The function only has a single block (e.g. no control flow or conditional branches)
 //! - It is not marked with the [no predicates inline type][noirc_frontend::monomorphization::ast::InlineType::NoPredicates]
+
 use iter_extended::btree_map;
 
 use crate::ssa::{
+    RuntimeError,
     ir::{
         call_graph::CallGraph,
         function::{Function, RuntimeType},
@@ -28,7 +30,7 @@ const MAX_INSTRUCTIONS: usize = 10;
 
 impl Ssa {
     /// See the [`inline_simple_functions`][self] module for more information.
-    pub(crate) fn inline_simple_functions(mut self: Ssa) -> Ssa {
+    pub(crate) fn inline_simple_functions(mut self: Ssa) -> Result<Ssa, RuntimeError> {
         let call_graph = CallGraph::from_ssa(&self);
         let recursive_functions = call_graph.get_recursive_functions();
 
@@ -67,16 +69,24 @@ impl Ssa {
             true
         };
 
+        let mut result: Option<RuntimeError> = None;
         self.functions = btree_map(&self.functions, |(id, function)| {
+            let inlined = function.inlined(&self, &should_inline_call);
             (
                 *id,
-                function
-                    .inlined(&self, &should_inline_call)
-                    .expect("simple function should not be recursive"),
+                if let Ok(new_function) = inlined {
+                    new_function
+                } else {
+                    result = inlined.err();
+                    function.clone()
+                },
             )
         });
 
-        self
+        if let Some(result) = result {
+            return Err(result);
+        }
+        Ok(self)
     }
 }
 
@@ -89,7 +99,7 @@ mod test {
 
     fn assert_does_not_inline(src: &str) {
         let ssa = Ssa::from_str(src).unwrap();
-        let ssa = ssa.inline_simple_functions();
+        let ssa = ssa.inline_simple_functions().unwrap();
         assert_normalized_ssa_equals(ssa, src);
     }
 
@@ -111,7 +121,7 @@ mod test {
         ";
         let ssa = Ssa::from_str(src).unwrap();
 
-        let ssa = ssa.inline_simple_functions();
+        let ssa = ssa.inline_simple_functions().unwrap();
         assert_ssa_snapshot!(ssa, @r"
         acir(inline) fn main f0 {
           b0(v0: Field):
@@ -161,7 +171,7 @@ mod test {
         let ssa = Ssa::from_str(src).unwrap();
 
         // In the first pass it won't recognize that `main` could be simplified.
-        let mut ssa = ssa.inline_simple_functions();
+        let mut ssa = ssa.inline_simple_functions().unwrap();
         assert_ssa_snapshot!(&mut ssa, @r"
         acir(inline) fn main f0 {
           b0(v0: Field):
@@ -180,7 +190,7 @@ mod test {
         ");
 
         // After `bar` has been simplified, it does `main` as well.
-        ssa = ssa.inline_simple_functions();
+        ssa = ssa.inline_simple_functions().unwrap();
         assert_ssa_snapshot!(ssa, @r"
         acir(inline) fn main f0 {
           b0(v0: Field):
@@ -218,7 +228,7 @@ mod test {
         ";
         let ssa = Ssa::from_str(src).unwrap();
 
-        let ssa = ssa.inline_simple_functions();
+        let ssa = ssa.inline_simple_functions().unwrap();
         assert_ssa_snapshot!(ssa, @r"
         acir(inline) fn main f0 {
           b0(v0: Field):

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

@@ -8,7 +8,7 @@ use crate::errors::RuntimeError;
 use acvm::acir::AcirField;
 use im::HashMap;
 use iter_extended::vecmap;
-use noirc_errors::call_stack::CallStackId;
+use noirc_errors::{Location, call_stack::CallStackId};
 
 use crate::ssa::{
     function_builder::FunctionBuilder,
@@ -125,8 +125,8 @@ impl Ssa {
             let inlined = function.inlined(&self, &should_inline_call)?;
             new_functions.insert(entry_point, inlined);
         }
-        self.functions = new_functions;
 
+        self.functions = new_functions;
         Ok(self)
     }
 }
@@ -495,7 +495,8 @@ impl<'function> PerFunctionContext<'function> {
             match &self.source_function.dfg[*id] {
                 Instruction::Call { func, arguments } => match self.get_function(*func) {
                     Some(func_id) => {
-                        if let Some(callee) = self.should_inline_call(ssa, func_id) {
+                        let call_stack = self.source_function.dfg.get_instruction_call_stack(*id);
+                        if let Some(callee) = self.should_inline_call(ssa, func_id, call_stack)? {
                             if should_inline_call(callee) {
                                 self.inline_function(
                                     ssa,
@@ -539,32 +540,42 @@ impl<'function> PerFunctionContext<'function> {
         &self,
         ssa: &'a Ssa,
         called_func_id: FunctionId,
-    ) -> Option<&'a Function> {
+        call_stack: Vec<Location>,
+    ) -> Result<Option<&'a Function>, RuntimeError> {
         // Do not inline self-recursive functions on the top level.
         // Inlining a self-recursive function works when there is something to inline into
         // by importing all the recursive blocks, but for the entry function there is no wrapper.
         if self.entry_function.id() == called_func_id {
-            return None;
+            return Ok(None);
         }
 
         let callee = &ssa.functions[&called_func_id];
 
         match callee.runtime() {
             RuntimeType::Acir(inline_type) => {
                 // If the called function is acir, we inline if it's not an entry point
+                // If it is called from brillig, it cannot be inlined because runtimes do not share the same semantic
+                if self.entry_function.runtime().is_brillig() {
+                    return Err(RuntimeError::UnconstrainedCallingConstrained {
+                        call_stack,
+                        constrained: callee.name().to_string(),
+                        unconstrained: self.entry_function.name().to_string(),
+                    });
+                }
                 if inline_type.is_entry_point() {
-                    return None;
+                    assert!(!self.entry_function.runtime().is_brillig());
+                    return Ok(None);
                 }
             }
             RuntimeType::Brillig(_) => {
                 if self.entry_function.runtime().is_acir() {
                     // We never inline a brillig function into an ACIR function.
-                    return None;
+                    return Ok(None);
                 }
             }
         }
 
-        Some(callee)
+        Ok(Some(callee))
     }
 
     /// Inline a function call and remember the inlined return values in the values map
@@ -772,6 +783,7 @@ impl<'function> PerFunctionContext<'function> {
 mod test {
     use crate::{
         assert_ssa_snapshot,
+        errors::RuntimeError,
         ssa::{Ssa, ir::instruction::TerminatorInstruction, opt::assert_normalized_ssa_equals},
     };
 
@@ -1374,4 +1386,27 @@ mod test {
         let ssa = Ssa::from_str(src).unwrap();
         let _ = ssa.inline_functions(i64::MAX).unwrap();
     }
+
+    #[test]
+    // We should not inline an ACIR function called from a Brillig function because ACIR and Brillig semantics are different.
+    fn inlining_acir_into_brillig_function() {
+        let src = "
+        brillig(inline) fn main f0 {
+          b0(v0: u32):
+            call f1(v0)
+            return
+        }
+        acir(inline) fn foo f1 {
+          b0(v0: u32):
+            v4 = make_array [Field 1, Field 2, Field 3] : [Field; 3]
+            v5 = array_get v4, index v0 -> Field
+            return
+        }
+        ";
+        let ssa = Ssa::from_str_no_validation(src).unwrap();
+        let ssa = ssa.inline_functions(i64::MAX);
+        if !matches!(ssa, Err(RuntimeError::UnconstrainedCallingConstrained { .. })) {
+            panic!("Expected inlining to fail with RuntimeError::UnconstrainedCallingConstrained");
+        }
+    }
 }

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

@@ -323,6 +323,20 @@ impl<'f> Validator<'f> {
         }
     }
 
+    /// Validates that acir functions are not called from unconstrained code.
+    fn check_calls_in_unconstrained(&self, instruction: InstructionId) {
+        if self.function.runtime().is_brillig() {
+            if let Instruction::Call { func, .. } = &self.function.dfg[instruction] {
+                if let Value::Function(func_id) = &self.function.dfg[*func] {
+                    let called_function = &self.ssa.functions[func_id];
+                    if called_function.runtime().is_acir() {
+                        panic!("Call to acir function {} from unconstrained code", func_id);
+                    }
+                }
+            }
+        }
+    }
+
     fn type_check_globals(&self) {
         let globals = (*self.function.dfg.globals).clone();
         for (_, global) in globals.values_iter() {
@@ -341,6 +355,7 @@ impl<'f> Validator<'f> {
             for instruction in self.function.dfg[block].instructions() {
                 self.validate_field_to_integer_cast_invariant(*instruction);
                 self.type_check_instruction(*instruction);
+                self.check_calls_in_unconstrained(*instruction);
             }
         }
     }
@@ -760,4 +775,23 @@ mod tests {
         ";
         let _ = Ssa::from_str(src).unwrap();
     }
+
+    #[test]
+    #[should_panic(expected = "Call to acir function f1 from unconstrained code")]
+    fn disallows_calling_acir_from_brillig() {
+        let src = "
+        brillig(inline) fn main f0 {
+          b0(v0: u32):
+            call f1(v0)
+            return
+        }
+        acir(inline) fn foo f1 {
+          b0(v0: u32):
+            v4 = make_array [Field 1, Field 2, Field 3] : [Field; 3]
+            v5 = array_get v4, index v0 -> Field
+            return
+        }
+        ";
+        let _ = Ssa::from_str(src).unwrap();
+    }
 }