Merge branch 'master' into rk/directed-graph-ssa-check

compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/array_copy.rs

@@ -69,6 +69,8 @@ pub(super) fn compile_array_copy_procedure<F: AcirField + DebugToString>(
                 BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
                 1_usize.into(),
             );
+            // Decrease the original ref count now that this copy is no longer pointing to it
+            ctx.codegen_usize_op(rc.address, rc.address, BrilligBinaryOp::Sub, 1);
         }
     });
 }

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

@@ -441,29 +441,38 @@ impl FunctionBuilder {
     /// Insert instructions to increment the reference count of any array(s) stored
     /// within the given value. If the given value is not an array and does not contain
     /// any arrays, this does nothing.
-    pub(crate) fn increment_array_reference_count(&mut self, value: ValueId) {
-        self.update_array_reference_count(value, true);
+    ///
+    /// Returns whether a reference count instruction was issued.
+    pub(crate) fn increment_array_reference_count(&mut self, value: ValueId) -> bool {
+        self.update_array_reference_count(value, true)
     }
 
     /// Insert instructions to decrement the reference count of any array(s) stored
     /// within the given value. If the given value is not an array and does not contain
     /// any arrays, this does nothing.
-    pub(crate) fn decrement_array_reference_count(&mut self, value: ValueId) {
-        self.update_array_reference_count(value, false);
+    ///
+    /// Returns whether a reference count instruction was issued.
+    pub(crate) fn decrement_array_reference_count(&mut self, value: ValueId) -> bool {
+        self.update_array_reference_count(value, false)
     }
 
     /// Increment or decrement the given value's reference count if it is an array.
     /// If it is not an array, this does nothing. Note that inc_rc and dec_rc instructions
     /// are ignored outside of unconstrained code.
-    fn update_array_reference_count(&mut self, value: ValueId, increment: bool) {
+    ///
+    /// Returns whether a reference count instruction was issued.
+    fn update_array_reference_count(&mut self, value: ValueId, increment: bool) -> bool {
         match self.type_of_value(value) {
-            Type::Numeric(_) => (),
-            Type::Function => (),
+            Type::Numeric(_) => false,
+            Type::Function => false,
             Type::Reference(element) => {
                 if element.contains_an_array() {
                     let reference = value;
                     let value = self.insert_load(reference, element.as_ref().clone());
                     self.update_array_reference_count(value, increment);
+                    true
+                } else {
+                    false
                 }
             }
             Type::Array(..) | Type::Slice(..) => {
@@ -474,6 +483,7 @@ impl FunctionBuilder {
                 } else {
                     self.insert_dec_rc(value);
                 }
+                true
             }
         }
     }

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

@@ -129,7 +129,7 @@ impl<'f> FunctionInserter<'f> {
         // another MakeArray instruction. Note that this assumes the function inserter is inserting
         // in control-flow order. Otherwise we could refer to ValueIds defined later in the program.
         let make_array = if let Instruction::MakeArray { elements, typ } = &instruction {
-            if self.array_is_constant(elements) {
+            if self.array_is_constant(elements) && self.function.runtime().is_acir() {
                 if let Some(fetched_value) = self.get_cached_array(elements, typ) {
                     assert_eq!(results.len(), 1);
                     self.values.insert(results[0], fetched_value);

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

@@ -429,7 +429,7 @@ impl Instruction {
     /// conditional on whether the caller wants the predicate to be taken into account or not.
     pub(crate) fn can_be_deduplicated(
         &self,
-        dfg: &DataFlowGraph,
+        function: &Function,
         deduplicate_with_predicate: bool,
     ) -> bool {
         use Instruction::*;
@@ -443,7 +443,7 @@ impl Instruction {
             | IncrementRc { .. }
             | DecrementRc { .. } => false,
 
-            Call { func, .. } => match dfg[*func] {
+            Call { func, .. } => match function.dfg[*func] {
                 Value::Intrinsic(intrinsic) => {
                     intrinsic.can_be_deduplicated(deduplicate_with_predicate)
                 }
@@ -453,8 +453,11 @@ impl Instruction {
             // We can deduplicate these instructions if we know the predicate is also the same.
             Constrain(..) | RangeCheck { .. } => deduplicate_with_predicate,
 
-            // This should never be side-effectful
-            MakeArray { .. } => true,
+            // Arrays can be mutated in unconstrained code so code that handles this case must
+            // take care to track whether the array was possibly mutated or not before
+            // deduplicating. Since we don't know if the containing pass checks for this, we
+            // can only assume these are safe to deduplicate in constrained code.
+            MakeArray { .. } => function.runtime().is_acir(),
 
             // These can have different behavior depending on the EnableSideEffectsIf context.
             // Replacing them with a similar instruction potentially enables replacing an instruction
@@ -467,7 +470,7 @@ impl Instruction {
             | IfElse { .. }
             | ArrayGet { .. }
             | ArraySet { .. } => {
-                deduplicate_with_predicate || !self.requires_acir_gen_predicate(dfg)
+                deduplicate_with_predicate || !self.requires_acir_gen_predicate(&function.dfg)
             }
         }
     }

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

@@ -842,27 +842,27 @@ mod tests {
 
     #[test]
     fn simplify_derive_generators_has_correct_type() {
-        let src = "
+        let src = r#"
             brillig(inline) fn main f0 {
               b0():
-                v0 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24]
+                v0 = make_array b"DEFAULT_DOMAIN_SEPARATOR"
 
                 // This call was previously incorrectly simplified to something that returned `[Field; 3]`
                 v2 = call derive_pedersen_generators(v0, u32 0) -> [(Field, Field, u1); 1]
 
                 return v2
             }
-            ";
+            "#;
         let ssa = Ssa::from_str(src).unwrap();
 
-        let expected = "
+        let expected = r#"
             brillig(inline) fn main f0 {
               b0():
-                v15 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24]
+                v15 = make_array b"DEFAULT_DOMAIN_SEPARATOR"
                 v19 = make_array [Field 3728882899078719075161482178784387565366481897740339799480980287259621149274, Field -9903063709032878667290627648209915537972247634463802596148419711785767431332, u1 0] : [(Field, Field, u1); 1]
                 return v19
             }
-            ";
+            "#;
         assert_normalized_ssa_equals(ssa, expected);
     }
 }

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

@@ -5,8 +5,11 @@ use std::{
 };
 
 use acvm::acir::AcirField;
+use im::Vector;
 use iter_extended::vecmap;
 
+use crate::ssa::ir::types::{NumericType, Type};
+
 use super::{
     basic_block::BasicBlockId,
     dfg::DataFlowGraph,
@@ -220,6 +223,28 @@ fn display_instruction_inner(
             )
         }
         Instruction::MakeArray { elements, typ } => {
+            // If the array is a byte array, we check if all the bytes are printable ascii characters
+            // and, if so, we print the array as a string literal (easier to understand).
+            // It could happen that the byte array is a random byte sequence that happens to be printable
+            // (it didn't come from a string literal) but this still reduces the noise in the output
+            // and actually represents the same value.
+            let (element_types, is_slice) = match typ {
+                Type::Array(types, _) => (types, false),
+                Type::Slice(types) => (types, true),
+                _ => panic!("Expected array or slice type for MakeArray"),
+            };
+            if element_types.len() == 1
+                && element_types[0] == Type::Numeric(NumericType::Unsigned { bit_size: 8 })
+            {
+                if let Some(string) = try_byte_array_to_string(elements, function) {
+                    if is_slice {
+                        return writeln!(f, "make_array &b{:?}", string);
+                    } else {
+                        return writeln!(f, "make_array b{:?}", string);
+                    }
+                }
+            }
+
             write!(f, "make_array [")?;
 
             for (i, element) in elements.iter().enumerate() {
@@ -234,6 +259,25 @@ fn display_instruction_inner(
     }
 }
 
+fn try_byte_array_to_string(elements: &Vector<ValueId>, function: &Function) -> Option<String> {
+    let mut string = String::new();
+    for element in elements {
+        let element = function.dfg.get_numeric_constant(*element)?;
+        let element = element.try_to_u32()?;
+        if element > 0xFF {
+            return None;
+        }
+        let byte = element as u8;
+        if byte.is_ascii_alphanumeric() || byte.is_ascii_punctuation() || byte.is_ascii_whitespace()
+        {
+            string.push(byte as char);
+        } else {
+            return None;
+        }
+    }
+    Some(string)
+}
+
 fn result_types(function: &Function, results: &[ValueId]) -> String {
     let types = vecmap(results, |result| function.dfg.type_of_value(*result).to_string());
     if types.is_empty() {

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

@@ -190,6 +190,15 @@ impl Type {
         }
     }
 
+    /// Retrieves the array or slice type within this type, or panics if there is none.
+    pub(crate) fn get_contained_array(&self) -> &Type {
+        match self {
+            Type::Numeric(_) | Type::Function => panic!("Expected an array type"),
+            Type::Array(_, _) | Type::Slice(_) => self,
+            Type::Reference(element) => element.get_contained_array(),
+        }
+    }
+
     pub(crate) fn element_types(self) -> Arc<Vec<Type>> {
         match self {
             Type::Array(element_types, _) | Type::Slice(element_types) => element_types,