fix: Implement constant folding during the mem2reg pass

crates/nargo_cli/tests/execution_success/references_aliasing/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "references"
+type = "bin"
+authors = [""]
+compiler_version = "0.5.1"
+
+[dependencies]

crates/nargo_cli/tests/execution_success/references_aliasing/src/main.nr

@@ -0,0 +1,29 @@
+fn main() {
+    let mut x = 100;
+    let mut xref = &mut x;
+    increment(xref);
+    assert(*xref == 101);
+
+    regression_2445();
+}
+
+fn increment(mut r: &mut Field) {
+    *r = *r + 1;
+}
+
+// If aliasing within arrays and constant folding within the mem2reg pass aren't
+// handled, we'll fail to optimize out all the references in this function.
+fn regression_2445() {
+    let mut var = 0;
+    let ref = &mut &mut var;
+
+    let mut array = [ref, ref];
+
+    **array[0] = 1;
+    **array[1] = 2;
+
+    assert(var == 2);
+    assert(**ref == 2);
+    assert(**array[0] == 2);
+    assert(**array[1] == 2);
+}

crates/nargo_cli/tests/execution_success/references_aliasing/target/references.bytecode

@@ -0,0 +1 @@
+H4sIAAAAAAAA/+1c8ZONVRh+WCKykYqIiFRE37f37u5dkY2IEEJEtLv2bkJKQkpCSkJKIjLGNE3TmJqpmZqpmZrRH9L/Uu907szZzfjlPO+Z98yc88u+w8yz3/O853ue93zf3fsXgL/x/zXY/ex0P4uwVQ7ysCpFW7Vab2+pl5Wyu2jp6Km1FtXWnrZaWStba629LbVKpV6r1to7ejrai46yWqmXfa0dlT4HNpiAVe/7bzX9izHoJvwHkfkP5mEV/vU2efWQAb3z//82BU4Y8HsG6th8k3+j/nKNJjUp4A4Bb/Nr8R7C71HhQZrXtLEsG99QpGd8Q6FjfLd5dTa+QMyhSkINg23jE97D+D1SNT62po1l2fiGIz3jGw4d47vdq7PxBWIOd4KycUfAtvEJ7xH8HqkaH1vTxrJsfCORnvGNhI7x3eHV2fgCMUc6Qdm4o2Db+IT3KH6PVI2PrWljWTa+ZtCMrxbL+JqhY3x3enU2vkDMZicoG3c0bBuf8B7N71EyhjIGLEOp12MZyhjoGMpdXp0NJRBzjBOUjTsWtg1FeI/l9+iWhhKqA9Ok74bOTcHmzNxH9yTCmdnneyNxLsJWL7PP43jX1SIYbO+Rnoy7CW4o76vQ8bEmKv+yytzf44l9uQrWkBXvcRWRf78h6z6vzkNWIOZ4JygbdwJsD1nCewK/R6qPq9iaNhZ7SGCeLifS9CsrsYyPd839je9+r87GF4g50QnKxp0E28YnvCfxe6T5uKqVqelkIudYhsK8Zv96H/DqbCiBmJOdoGzcKbBtKMJ7Cr9HqpPUFMSZpIqwRX1OP5WAFfsIORU6xvegV2fjC8Sc6gRl406DbeMT3tP4PUrGUKaDZSjxXvxNh46hPOTV2VACMac7Qdm4M2DbUIT3DH6PVJ/1ME36YejcFGzOzH30SCKcmX1+NBLnImz1Mvs8k3ddKi/+pCczwX/xdw06PsZ+8cfc37OIfbkG2pDVG2vIIvLvN2Q95tV5yArEnOUEZePOhu0hS3jP5vdI9XEVW9PGsny6nAOW8cX7nPoc6Bjf416djS8Qc44TlI1bwLbxCe+C3yNFQylbmJqWRM6xDIV5zf71tnh1NpRAzNIJysatwLahCO8Kv0eqkxRb08ayPElVkd4kVYWO8bV6dTa+QMyqE5SN2wbbxie82/g9SsZQ2pGeobRDx1BqXp0NJRCz3QnKxu2AbUMR3h38HiVjKHORnqHMhY6hPOHV2VACMec6Qdm482DbUIT3PH6PkjGU+UjPUOZDx1Ce9OpsKIGY852gbNwFsG0ownsBv0fJGEon0jOUTugYylNenQ0lELPTCcrGXQjbhiI4C/k9SsZQFiE9Q1kEHUN52quzoQRiLnKCsnEXw7ahCO/F/B4lYyhLwDKUeH+NsQQ6hvKMV2dDCcRc4gRl4y6FbUMR3kv5PUrGUJYhPUNZBh1Dedars6EEYi5zgrJxl8O2oQjv5fweqf55F9OkV/A4t9yKcxG2qCa6EumZ6EromOhzXp1NNBBzpROUjbsKtk1UeK/i90jlWsXsV4D/N3XfQCc8mkjX2fiWEGYgryb2hahfMkG0BukF0RroBNHzXp2DKBBzjROUjbsWtoNIeK/l90jlWiUwV4MfRN8ijSBiDjXriH0h6pdMEK1HekG0HjpB9IJX5yAKxFzvBGXjboDtIBLeG/g9UrlWCcx14AfRd0gjiJhDzUZiX4j63TKIQjkzX0K/CNt+Jvf0RoV75Xukca8wfXcTsS9E/VS+yUn29SaFfXM90r4pglZZZXrEZmJfrhPvjViDL5F/v8H3Ja/Og28g5mYnKBt3C2wPvsJ7C79Hqt8/wNa0sSyf+LeCZnzRPuq6FTrG97JXZ+MLxNzqBGXjdsG28QnvLn6PkjGUbqRnKN3QMZQer86GEojZ7QRl426DbUMR3tv4PVL9ZFoXEasXOjcFmzNzH9UjcS7CFjU4+sAKjnjvnvqgExyveHUOjkDMPicoG3c7bAeH8N7O75FqcDBN9FWkERzMfbSDx7nfuyLLz4F3Evus9RzdcvDuAit4453YdkEneF/z6hy8gZi7nKBs3N2wHbzCeze/R8mc2F5HGsHL3EdvROJchC1qcOwBKzjindj2QCc43vTqHByBmHucoGzcvbAdHMJ7L79HqsHBNNG3kEZwMPfRPh5nlU8syYl8J/ifWPoB3P3N5i1PD3Yo8P4ROvd1E/k69xO1JPa61NLP8qB1AKxBK94J/QB0Bq23vToPWoGYB5ygbNyDsD1oCe+D/B4lc0J/Bzo3BZszcx+9G4lzEbaowXEIrOCId0I/BJ3geM+rc3AEYh5ygrJxD8N2cAjvw/weqQYH00TfRxrBwdxHR3icVU7o8gRmP/gn1Z/A3d9s3vK0aJ8C75+hc1+zT+hHiVoSe11q6Wd50DoG1qAV74R+DDqD1gdenQetQMxjTlA27nHYHrSE93F+j5I5oX8InZuCzZm5jz6KxLkIW9TgOAFWcMQ7oZ+ATnB87NU5OAIxTzhB2bgnYTs4hPdJfo9Ug4Npop8gjeBg7qNTPM4qJ3R5AnMU/JPqL+DubzZveVp0RIH3r9C5r9kn9NNELYm9LrX0szxonQFr0Ip3Qj8DnUHrU6/Og1Yg5hknKBv3LGwPWsL7LL9HyZzQP4POTcHmzNxHn0fiXIQtanCcAys44p3Qz0EnOL7w6hwcgZjnnKBs3POwHRzC+zy/R6rBwTTRL5FGcDD30QUeZ5UTujyBOQ3+SfU3cPc3m7c8LTqlwPt36NzX7BP6RaKWxF6XWvpZHqi/gu28lgy4qHCv/AHbHiF5dUGB959IwyMuEbUk9rpk68feN3I/X1LYNzcQZ98UQausdhE5Xyb25QbhumJ/1zWRf78D7ddenQ+0gZiXnaBs3CuwfaAV3lf4PVL9rmumpoMGXKO//gGokXOuaucAAA==

crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -1081,7 +1081,7 @@ impl<'block> BrilligBlock<'block> {
 /// Returns the type of the operation considering the types of the operands
 /// TODO: SSA issues binary operations between fields and integers.
 /// This probably should be explicitly casted in SSA to avoid having to coerce at this level.
-pub(crate) fn type_of_binary_operation(lhs_type: Type, rhs_type: Type) -> Type {
+pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type) -> Type {
     match (lhs_type, rhs_type) {
         (_, Type::Function) | (Type::Function, _) => {
             unreachable!("Functions are invalid in binary operations")
@@ -1098,15 +1098,15 @@ pub(crate) fn type_of_binary_operation(lhs_type: Type, rhs_type: Type) -> Type {
         // If either side is a Field constant then, we coerce into the type
         // of the other operand
         (Type::Numeric(NumericType::NativeField), typ)
-        | (typ, Type::Numeric(NumericType::NativeField)) => typ,
+        | (typ, Type::Numeric(NumericType::NativeField)) => typ.clone(),
         // If both sides are numeric type, then we expect their types to be
         // the same.
         (Type::Numeric(lhs_type), Type::Numeric(rhs_type)) => {
             assert_eq!(
                 lhs_type, rhs_type,
                 "lhs and rhs types in a binary operation are always the same"
             );
-            Type::Numeric(lhs_type)
+            Type::Numeric(*lhs_type)
         }
     }
 }

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

@@ -106,7 +106,7 @@ impl DataFlowGraph {
         let parameters = self.blocks[block].parameters();
 
         let parameters = vecmap(parameters.iter().enumerate(), |(position, param)| {
-            let typ = self.values[*param].get_type();
+            let typ = self.values[*param].get_type().clone();
             self.values.insert(Value::Param { block: new_block, position, typ })
         });
 
@@ -171,7 +171,7 @@ impl DataFlowGraph {
                 let id = self.make_instruction(instruction, ctrl_typevars);
                 self.blocks[block].insert_instruction(id);
                 self.locations.insert(id, call_stack);
-                InsertInstructionResult::Results(self.instruction_results(id))
+                InsertInstructionResult::Results(id, self.instruction_results(id))
             }
         }
     }
@@ -314,7 +314,13 @@ impl DataFlowGraph {
 
     /// Returns the type of a given value
     pub(crate) fn type_of_value(&self, value: ValueId) -> Type {
-        self.values[value].get_type()
+        self.values[value].get_type().clone()
+    }
+
+    /// True if the type of this value is Type::Reference.
+    /// Using this method over type_of_value avoids cloning the value's type.
+    pub(crate) fn value_is_reference(&self, value: ValueId) -> bool {
+        matches!(self.values[value].get_type(), Type::Reference)
     }
 
     /// Appends a result type to the instruction.
@@ -472,7 +478,8 @@ impl std::ops::IndexMut<BasicBlockId> for DataFlowGraph {
 // be a list of results or a single ValueId if the instruction was simplified
 // to an existing value.
 pub(crate) enum InsertInstructionResult<'dfg> {
-    Results(&'dfg [ValueId]),
+    /// Results is the standard case containing the instruction id and the results of that instruction.
+    Results(InstructionId, &'dfg [ValueId]),
     SimplifiedTo(ValueId),
     SimplifiedToMultiple(Vec<ValueId>),
     InstructionRemoved,
@@ -484,7 +491,7 @@ impl<'dfg> InsertInstructionResult<'dfg> {
         match self {
             InsertInstructionResult::SimplifiedTo(value) => *value,
             InsertInstructionResult::SimplifiedToMultiple(values) => values[0],
-            InsertInstructionResult::Results(results) => results[0],
+            InsertInstructionResult::Results(_, results) => results[0],
             InsertInstructionResult::InstructionRemoved => {
                 panic!("Instruction was removed, no results")
             }
@@ -495,7 +502,7 @@ impl<'dfg> InsertInstructionResult<'dfg> {
     /// This is used for instructions returning multiple results like function calls.
     pub(crate) fn results(self) -> Cow<'dfg, [ValueId]> {
         match self {
-            InsertInstructionResult::Results(results) => Cow::Borrowed(results),
+            InsertInstructionResult::Results(_, results) => Cow::Borrowed(results),
             InsertInstructionResult::SimplifiedTo(result) => Cow::Owned(vec![result]),
             InsertInstructionResult::SimplifiedToMultiple(results) => Cow::Owned(results),
             InsertInstructionResult::InstructionRemoved => Cow::Owned(vec![]),
@@ -507,7 +514,7 @@ impl<'dfg> InsertInstructionResult<'dfg> {
         match self {
             InsertInstructionResult::SimplifiedTo(_) => 1,
             InsertInstructionResult::SimplifiedToMultiple(results) => results.len(),
-            InsertInstructionResult::Results(results) => results.len(),
+            InsertInstructionResult::Results(_, results) => results.len(),
             InsertInstructionResult::InstructionRemoved => 0,
         }
     }

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

@@ -30,7 +30,7 @@ impl<'f> FunctionInserter<'f> {
     pub(crate) fn resolve(&mut self, mut value: ValueId) -> ValueId {
         value = self.function.dfg.resolve(value);
         match self.values.get(&value) {
-            Some(value) => *value,
+            Some(value) => self.resolve(*value),
             None => match &self.function.dfg[value] {
                 super::value::Value::Array { array, typ } => {
                     let array = array.clone();
@@ -62,9 +62,27 @@ impl<'f> FunctionInserter<'f> {
         )
     }
 
-    pub(crate) fn push_instruction(&mut self, id: InstructionId, block: BasicBlockId) {
+    /// Maps a terminator in place, replacing any ValueId in the terminator with the
+    /// resolved version of that value id from this FunctionInserter's internal value mapping.
+    pub(crate) fn map_terminator_in_place(&mut self, block: BasicBlockId) {
+        let mut terminator = self.function.dfg[block].take_terminator();
+        terminator.mutate_values(|value| self.resolve(value));
+        self.function.dfg[block].set_terminator(terminator);
+    }
+
+    /// Push a new instruction to the given block and return its new InstructionId.
+    /// If the instruction was simplified out of the program, None is returned.
+    pub(crate) fn push_instruction(
+        &mut self,
+        id: InstructionId,
+        block: BasicBlockId,
+    ) -> Option<InstructionId> {
         let (instruction, location) = self.map_instruction(id);
-        self.push_instruction_value(instruction, id, block, location);
+
+        match self.push_instruction_value(instruction, id, block, location) {
+            InsertInstructionResult::Results(new_id, _) => Some(new_id),
+            _ => None,
+        }
     }
 
     pub(crate) fn push_instruction_value(
@@ -110,7 +128,7 @@ impl<'f> FunctionInserter<'f> {
                     values.insert(*old_result, *new_result);
                 }
             }
-            InsertInstructionResult::Results(new_results) => {
+            InsertInstructionResult::Results(_, new_results) => {
                 for (old_result, new_result) in old_results.iter().zip(*new_results) {
                     values.insert(*old_result, *new_result);
                 }

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

@@ -517,6 +517,26 @@ impl TerminatorInstruction {
         }
     }
 
+    /// Mutate each ValueId to a new ValueId using the given mapping function
+    pub(crate) fn mutate_values(&mut self, mut f: impl FnMut(ValueId) -> ValueId) {
+        use TerminatorInstruction::*;
+        match self {
+            JmpIf { condition, .. } => {
+                *condition = f(*condition);
+            }
+            Jmp { arguments, .. } => {
+                for argument in arguments {
+                    *argument = f(*argument);
+                }
+            }
+            Return { return_values } => {
+                for return_value in return_values {
+                    *return_value = f(*return_value);
+                }
+            }
+        }
+    }
+
     /// Apply a function to each value
     pub(crate) fn for_each_value<T>(&self, mut f: impl FnMut(ValueId) -> T) {
         use TerminatorInstruction::*;
@@ -837,7 +857,7 @@ pub(crate) enum SimplifyResult {
     /// a function such as a tuple
     SimplifiedToMultiple(Vec<ValueId>),
 
-    /// Replace this function with an simpler but equivalent function.
+    /// Replace this function with an simpler but equivalent instruction.
     SimplifiedToInstruction(Instruction),
 
     /// Remove the instruction, it is unnecessary

crates/noirc_evaluator/src/ssa/ir/value.rs

@@ -57,15 +57,15 @@ pub(crate) enum Value {
 
 impl Value {
     /// Retrieves the type of this Value
-    pub(crate) fn get_type(&self) -> Type {
+    pub(crate) fn get_type(&self) -> &Type {
         match self {
-            Value::Instruction { typ, .. } => typ.clone(),
-            Value::Param { typ, .. } => typ.clone(),
-            Value::NumericConstant { typ, .. } => typ.clone(),
-            Value::Array { typ, .. } => typ.clone(),
-            Value::Function { .. } => Type::Function,
-            Value::Intrinsic { .. } => Type::Function,
-            Value::ForeignFunction { .. } => Type::Function,
+            Value::Instruction { typ, .. } => typ,
+            Value::Param { typ, .. } => typ,
+            Value::NumericConstant { typ, .. } => typ,
+            Value::Array { typ, .. } => typ,
+            Value::Function { .. } => &Type::Function,
+            Value::Intrinsic { .. } => &Type::Function,
+            Value::ForeignFunction { .. } => &Type::Function,
         }
     }
 }

crates/noirc_evaluator/src/ssa/opt/constant_folding.rs

@@ -95,7 +95,7 @@ impl Context {
         ) {
             InsertInstructionResult::SimplifiedTo(new_result) => vec![new_result],
             InsertInstructionResult::SimplifiedToMultiple(new_results) => new_results,
-            InsertInstructionResult::Results(new_results) => new_results.to_vec(),
+            InsertInstructionResult::Results(_, new_results) => new_results.to_vec(),
             InsertInstructionResult::InstructionRemoved => vec![],
         };
         assert_eq!(old_results.len(), new_results.len());

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

@@ -392,7 +392,7 @@ impl<'function> PerFunctionContext<'function> {
             self.context.call_stack.pop_back();
         }
 
-        let new_results = InsertInstructionResult::Results(&new_results);
+        let new_results = InsertInstructionResult::Results(call_id, &new_results);
         Self::insert_new_instruction_results(&mut self.values, old_results, new_results);
     }
 
@@ -435,7 +435,7 @@ impl<'function> PerFunctionContext<'function> {
                     values.insert(*old_result, new_result);
                 }
             }
-            InsertInstructionResult::Results(new_results) => {
+            InsertInstructionResult::Results(_, new_results) => {
                 for (old_result, new_result) in old_results.iter().zip(new_results) {
                     values.insert(*old_result, *new_result);
                 }