Merge branch 'master' into ab/parser

compiler/noirc_driver/src/abi_gen.rs

@@ -7,6 +7,7 @@ use noirc_abi::{
     Abi, AbiErrorType, AbiParameter, AbiReturnType, AbiType, AbiValue, AbiVisibility, Sign,
 };
 use noirc_frontend::ast::{Signedness, Visibility};
+use noirc_frontend::TypeBinding;
 use noirc_frontend::{
     hir::Context,
     hir_def::{
@@ -17,7 +18,6 @@ use noirc_frontend::{
     },
     node_interner::{FuncId, NodeInterner},
 };
-use noirc_frontend::{TypeBinding, TypeVariableKind};
 
 /// Arranges a function signature and a generated circuit's return witnesses into a
 /// `noirc_abi::Abi`.
@@ -72,13 +72,18 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
 
             AbiType::Integer { sign, width: (*bit_width).into() }
         }
-        Type::TypeVariable(binding, TypeVariableKind::IntegerOrField)
-        | Type::TypeVariable(binding, TypeVariableKind::Integer) => match &*binding.borrow() {
-            TypeBinding::Bound(typ) => abi_type_from_hir_type(context, typ),
-            TypeBinding::Unbound(_) => {
-                abi_type_from_hir_type(context, &Type::default_int_or_field_type())
+        Type::TypeVariable(binding) => {
+            if binding.is_integer() || binding.is_integer_or_field() {
+                match &*binding.borrow() {
+                    TypeBinding::Bound(typ) => abi_type_from_hir_type(context, typ),
+                    TypeBinding::Unbound(_id, _kind) => {
+                        abi_type_from_hir_type(context, &Type::default_int_or_field_type())
+                    }
+                }
+            } else {
+                unreachable!("{typ} cannot be used in the abi")
             }
-        },
+        }
         Type::Bool => AbiType::Boolean,
         Type::String(size) => {
             let size = size
@@ -106,7 +111,6 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
         | Type::Constant(..)
         | Type::InfixExpr(..)
         | Type::TraitAsType(..)
-        | Type::TypeVariable(_, _)
         | Type::NamedGeneric(..)
         | Type::Forall(..)
         | Type::Quoted(_)

compiler/noirc_evaluator/src/ssa.rs

@@ -118,6 +118,7 @@ pub(crate) fn optimize_into_acir(
     .run_pass(Ssa::remove_enable_side_effects, "After EnableSideEffectsIf removal:")
     .run_pass(Ssa::fold_constants_using_constraints, "After Constraint Folding:")
     .run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:")
+    .run_pass(Ssa::simplify_cfg, "After Simplifying:")
     .run_pass(Ssa::array_set_optimization, "After Array Set Optimizations:")
     .finish();
 

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

@@ -59,7 +59,7 @@ impl ControlFlowGraph {
 
     /// Clears out a given block's successors. This also removes the given block from
     /// being a predecessor of any of its previous successors.
-    fn invalidate_block_successors(&mut self, basic_block_id: BasicBlockId) {
+    pub(crate) fn invalidate_block_successors(&mut self, basic_block_id: BasicBlockId) {
         let node = self
             .data
             .get_mut(&basic_block_id)

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

@@ -2,7 +2,7 @@ use crate::ssa::{
     ir::{
         basic_block::BasicBlockId,
         dfg::DataFlowGraph,
-        function::Function,
+        function::{Function, RuntimeType},
         instruction::{Instruction, InstructionId, TerminatorInstruction},
         types::Type::{Array, Slice},
         value::ValueId,
@@ -34,14 +34,17 @@ impl Function {
         let mut array_to_last_use = HashMap::default();
         let mut instructions_to_update = HashSet::default();
         let mut arrays_from_load = HashSet::default();
+        let mut inner_nested_arrays = HashMap::default();
 
         for block in reachable_blocks.iter() {
             analyze_last_uses(
                 &self.dfg,
                 *block,
+                matches!(self.runtime(), RuntimeType::Brillig),
                 &mut array_to_last_use,
                 &mut instructions_to_update,
                 &mut arrays_from_load,
+                &mut inner_nested_arrays,
             );
         }
         for block in reachable_blocks {
@@ -55,9 +58,11 @@ impl Function {
 fn analyze_last_uses(
     dfg: &DataFlowGraph,
     block_id: BasicBlockId,
+    is_brillig_func: bool,
     array_to_last_use: &mut HashMap<ValueId, InstructionId>,
     instructions_that_can_be_made_mutable: &mut HashSet<InstructionId>,
     arrays_from_load: &mut HashSet<ValueId>,
+    inner_nested_arrays: &mut HashMap<ValueId, InstructionId>,
 ) {
     let block = &dfg[block_id];
 
@@ -70,12 +75,22 @@ fn analyze_last_uses(
                     instructions_that_can_be_made_mutable.remove(&existing);
                 }
             }
-            Instruction::ArraySet { array, .. } => {
+            Instruction::ArraySet { array, value, .. } => {
                 let array = dfg.resolve(*array);
 
                 if let Some(existing) = array_to_last_use.insert(array, *instruction_id) {
                     instructions_that_can_be_made_mutable.remove(&existing);
                 }
+                if is_brillig_func {
+                    let value = dfg.resolve(*value);
+
+                    if let Some(existing) = inner_nested_arrays.get(&value) {
+                        instructions_that_can_be_made_mutable.remove(existing);
+                    }
+                    let result = dfg.instruction_results(*instruction_id)[0];
+                    inner_nested_arrays.insert(result, *instruction_id);
+                }
+
                 // If the array we are setting does not come from a load we can safely mark it mutable.
                 // If the array comes from a load we may potentially being mutating an array at a reference
                 // that is loaded from by other values.
@@ -169,29 +184,31 @@ mod tests {
         // from and cloned in a loop. If the array is inadvertently marked mutable, and is cloned in a previous iteration
         // of the loop, its clone will also be altered.
         //
-        // acir(inline) fn main f0 {
+        // brillig fn main f0 {
         //     b0():
-        //       v2 = allocate
-        //       store [Field 0, Field 0, Field 0, Field 0, Field 0] at v2
         //       v3 = allocate
-        //       store [Field 0, Field 0, Field 0, Field 0, Field 0] at v3
+        //       store [[Field 0, Field 0, Field 0, Field 0, Field 0], [Field 0, Field 0, Field 0, Field 0, Field 0]] at v3
+        //       v4 = allocate
+        //       store [[Field 0, Field 0, Field 0, Field 0, Field 0], [Field 0, Field 0, Field 0, Field 0, Field 0]] at v4
         //       jmp b1(u32 0)
-        //     b1(v5: u32):
-        //       v7 = lt v5, u32 5
-        //       jmpif v7 then: b3, else: b2
+        //     b1(v6: u32):
+        //       v8 = lt v6, u32 5
+        //       jmpif v8 then: b3, else: b2
         //     b3():
-        //       v8 = eq v5, u32 5
-        //       jmpif v8 then: b4, else: b5
+        //       v9 = eq v6, u32 5
+        //       jmpif v9 then: b4, else: b5
         //     b4():
-        //       v9 = load v2
-        //       store v9 at v3
+        //       v10 = load v3
+        //       store v10 at v4
         //       jmp b5()
         //     b5():
-        //       v10 = load v2
-        //       v12 = array_set v10, index v5, value Field 20
-        //       store v12 at v2
-        //       v14 = add v5, u32 1
-        //       jmp b1(v14)
+        //       v11 = load v3
+        //       v13 = array_get v11, index Field 0
+        //       v14 = array_set v13, index v6, value Field 20
+        //       v15 = array_set v11, index v6, value v14
+        //       store v15 at v3
+        //       v17 = add v6, u32 1
+        //       jmp b1(v17)
         //     b2():
         //       return
         //   }
@@ -203,13 +220,16 @@ mod tests {
         let zero = builder.field_constant(0u128);
         let array_constant =
             builder.array_constant(vector![zero, zero, zero, zero, zero], array_type.clone());
+        let nested_array_type = Type::Array(Arc::new(vec![array_type.clone()]), 2);
+        let nested_array_constant = builder
+            .array_constant(vector![array_constant, array_constant], nested_array_type.clone());
 
-        let v2 = builder.insert_allocate(array_type.clone());
+        let v3 = builder.insert_allocate(array_type.clone());
 
-        builder.insert_store(v2, array_constant);
+        builder.insert_store(v3, nested_array_constant);
 
-        let v3 = builder.insert_allocate(array_type.clone());
-        builder.insert_store(v3, array_constant);
+        let v4 = builder.insert_allocate(array_type.clone());
+        builder.insert_store(v4, nested_array_constant);
 
         let b1 = builder.insert_block();
         let zero_u32 = builder.numeric_constant(0u128, Type::unsigned(32));
@@ -219,35 +239,38 @@ mod tests {
         builder.switch_to_block(b1);
         let v5 = builder.add_block_parameter(b1, Type::unsigned(32));
         let five = builder.numeric_constant(5u128, Type::unsigned(32));
-        let v7 = builder.insert_binary(v5, BinaryOp::Lt, five);
+        let v8 = builder.insert_binary(v5, BinaryOp::Lt, five);
 
         let b2 = builder.insert_block();
         let b3 = builder.insert_block();
         let b4 = builder.insert_block();
         let b5 = builder.insert_block();
-        builder.terminate_with_jmpif(v7, b3, b2);
+        builder.terminate_with_jmpif(v8, b3, b2);
 
         // Loop body
         // b3 is the if statement conditional
         builder.switch_to_block(b3);
         let two = builder.numeric_constant(5u128, Type::unsigned(32));
-        let v8 = builder.insert_binary(v5, BinaryOp::Eq, two);
-        builder.terminate_with_jmpif(v8, b4, b5);
+        let v9 = builder.insert_binary(v5, BinaryOp::Eq, two);
+        builder.terminate_with_jmpif(v9, b4, b5);
 
         // b4 is the rest of the loop after the if statement
         builder.switch_to_block(b4);
-        let v9 = builder.insert_load(v2, array_type.clone());
-        builder.insert_store(v3, v9);
+        let v10 = builder.insert_load(v3, nested_array_type.clone());
+        builder.insert_store(v4, v10);
         builder.terminate_with_jmp(b5, vec![]);
 
         builder.switch_to_block(b5);
-        let v10 = builder.insert_load(v2, array_type.clone());
+        let v11 = builder.insert_load(v3, nested_array_type.clone());
         let twenty = builder.field_constant(20u128);
-        let v12 = builder.insert_array_set(v10, v5, twenty);
-        builder.insert_store(v2, v12);
+        let v13 = builder.insert_array_get(v11, zero, array_type.clone());
+        let v14 = builder.insert_array_set(v13, v5, twenty);
+        let v15 = builder.insert_array_set(v11, v5, v14);
+
+        builder.insert_store(v3, v15);
         let one = builder.numeric_constant(1u128, Type::unsigned(32));
-        let v14 = builder.insert_binary(v5, BinaryOp::Add, one);
-        builder.terminate_with_jmp(b1, vec![v14]);
+        let v17 = builder.insert_binary(v5, BinaryOp::Add, one);
+        builder.terminate_with_jmp(b1, vec![v17]);
 
         builder.switch_to_block(b2);
         builder.terminate_with_return(vec![]);
@@ -265,7 +288,7 @@ mod tests {
             .filter(|instruction| matches!(&main.dfg[**instruction], Instruction::ArraySet { .. }))
             .collect::<Vec<_>>();
 
-        assert_eq!(array_set_instructions.len(), 1);
+        assert_eq!(array_set_instructions.len(), 2);
         if let Instruction::ArraySet { mutable, .. } = &main.dfg[*array_set_instructions[0]] {
             // The single array set should not be marked mutable
             assert!(!mutable);

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

@@ -16,7 +16,7 @@ use crate::ssa::{
     ir::{
         basic_block::BasicBlockId,
         dfg::DataFlowGraph,
-        function::Function,
+        function::{Function, RuntimeType},
         instruction::{BinaryOp, Instruction, Intrinsic},
         types::Type,
         value::Value,
@@ -37,6 +37,11 @@ impl Ssa {
 
 impl Function {
     pub(crate) fn remove_enable_side_effects(&mut self) {
+        if matches!(self.runtime(), RuntimeType::Brillig) {
+            // Brillig functions do not make use of the `EnableSideEffects` instruction so are unaffected by this pass.
+            return;
+        }
+
         let mut context = Context::default();
         context.block_queue.push(self.entry_block());
 

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

@@ -60,7 +60,6 @@ impl Function {
             check_for_constant_jmpif(self, block, &mut cfg);
 
             let mut predecessors = cfg.predecessors(block);
-
             if predecessors.len() == 1 {
                 let predecessor =
                     predecessors.next().expect("Already checked length of predecessors");
@@ -99,14 +98,23 @@ fn check_for_constant_jmpif(
     }) = function.dfg[block].terminator()
     {
         if let Some(constant) = function.dfg.get_numeric_constant(*condition) {
-            let destination =
-                if constant.is_zero() { *else_destination } else { *then_destination };
+            let (destination, unchosen_destination) = if constant.is_zero() {
+                (*else_destination, *then_destination)
+            } else {
+                (*then_destination, *else_destination)
+            };
 
             let arguments = Vec::new();
             let call_stack = call_stack.clone();
             let jmp = TerminatorInstruction::Jmp { destination, arguments, call_stack };
             function.dfg[block].set_terminator(jmp);
             cfg.recompute_block(function, block);
+
+            // If `block` was the only predecessor to `unchosen_destination` then it's no long reachable through the CFG,
+            // we can then invalidate it successors as it's an invalid predecessor.
+            if cfg.predecessors(unchosen_destination).len() == 0 {
+                cfg.invalidate_block_successors(unchosen_destination);
+            }
         }
     }
 }

compiler/noirc_frontend/src/ast/expression.rs

@@ -1,11 +1,12 @@
 use std::borrow::Cow;
 use std::fmt::Display;
 
+use thiserror::Error;
+
 use crate::ast::{
     Ident, ItemVisibility, Path, Pattern, Recoverable, Statement, StatementKind,
     UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility,
 };
-use crate::hir::def_collector::errors::DefCollectorErrorKind;
 use crate::node_interner::{
     ExprId, InternedExpressionKind, InternedStatementKind, QuotedTypeId, StructId,
 };
@@ -76,6 +77,13 @@ pub enum UnresolvedGeneric {
     Resolved(QuotedTypeId, Span),
 }
 
+#[derive(Error, PartialEq, Eq, Debug, Clone)]
+#[error("The only supported types of numeric generics are integers, fields, and booleans")]
+pub struct UnsupportedNumericGenericType {
+    pub ident: Ident,
+    pub typ: UnresolvedTypeData,
+}
+
 impl UnresolvedGeneric {
     pub fn span(&self) -> Span {
         match self {
@@ -85,7 +93,7 @@ impl UnresolvedGeneric {
         }
     }
 
-    pub fn kind(&self) -> Result<Kind, DefCollectorErrorKind> {
+    pub fn kind(&self) -> Result<Kind, UnsupportedNumericGenericType> {
         match self {
             UnresolvedGeneric::Variable(_) => Ok(Kind::Normal),
             UnresolvedGeneric::Numeric { typ, .. } => {
@@ -101,14 +109,14 @@ impl UnresolvedGeneric {
     fn resolve_numeric_kind_type(
         &self,
         typ: &UnresolvedType,
-    ) -> Result<Type, DefCollectorErrorKind> {
+    ) -> Result<Type, UnsupportedNumericGenericType> {
         use crate::ast::UnresolvedTypeData::{FieldElement, Integer};
 
         match typ.typ {
             FieldElement => Ok(Type::FieldElement),
             Integer(sign, bits) => Ok(Type::Integer(sign, bits)),
             // Only fields and integers are supported for numeric kinds
-            _ => Err(DefCollectorErrorKind::UnsupportedNumericGenericType {
+            _ => Err(UnsupportedNumericGenericType {
                 ident: self.ident().clone(),
                 typ: typ.typ.clone(),
             }),