chore: Try to optimize compilation memory

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

@@ -335,10 +335,8 @@ impl DataFlowGraph {
                         if self[id] == instruction {
                             // Just (re)insert into the block, no need to redefine.
                             self.blocks[block].insert_instruction(id);
-                            return InsertInstructionResult::Results(
-                                id,
-                                self.instruction_results(id),
-                            );
+                            let results = self.instruction_results(id);
+                            return InsertInstructionResult::Results(id, results);
                         }
                     }
                 }

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

@@ -29,7 +29,7 @@ impl<'f> FunctionInserter<'f> {
     /// ValueId that was passed in.
     pub(crate) fn resolve(&self, value: ValueId) -> ValueId {
         match self.values.get(&value) {
-            Some(value) => self.resolve(*value),
+            Some(new_value) => self.resolve(*new_value),
             None => value,
         }
     }
@@ -41,6 +41,7 @@ impl<'f> FunctionInserter<'f> {
             // existing entries, but we should never have a value in the map referring to itself anyway.
             self.values.remove(&key);
         } else {
+            self.validate_map_value(key, value);
             self.values.entry(key).or_insert(value);
         }
     }
@@ -50,10 +51,19 @@ impl<'f> FunctionInserter<'f> {
         if key == value {
             self.values.remove(&key);
         } else {
+            self.validate_map_value(key, value);
             self.values.insert(key, value);
         }
     }
 
+    /// Sanity check that we are not creating cycles.
+    fn validate_map_value(&self, key: ValueId, value: ValueId) {
+        #[cfg(debug_assertions)]
+        if let Some(value_of_value) = self.values.get(&value) {
+            assert!(*value_of_value != key, "reflexive mapping: {key} <-> {value}");
+        }
+    }
+
     /// Get an instruction and make sure all the values in it are freshly resolved.
     pub(crate) fn map_instruction(&mut self, id: InstructionId) -> (Instruction, CallStackId) {
         let mut instruction = self.function.dfg[id].clone();
@@ -84,39 +94,46 @@ impl<'f> FunctionInserter<'f> {
         self.function.dfg.data_bus = data_bus;
     }
 
-    /// Push a new instruction to the given block and return its new `InstructionId`.
+    /// Push an instruction by ID, after re-mapping the values in it, 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,
+        allow_reinsert: bool,
     ) -> Option<InstructionId> {
         let (instruction, location) = self.map_instruction(id);
 
-        match self.push_instruction_value(instruction, id, block, location) {
+        match self.push_instruction_value(instruction, id, block, location, allow_reinsert) {
             InsertInstructionResult::Results(new_id, _) => Some(new_id),
             _ => None,
         }
     }
 
+    /// Push a instruction that already exists with an ID, given as an instance with potential modification already applied to it.
+    ///
+    /// If `allow_insert` is set, we consider reinserting the instruction as it is, if it hasn't changed and cannot be simplified.
+    /// Consider using this if we are re-inserting when we take instructions from a block and re-insert them into the same block.
     pub(crate) fn push_instruction_value(
         &mut self,
         instruction: Instruction,
         id: InstructionId,
         block: BasicBlockId,
         call_stack: CallStackId,
+        allow_reinsert: bool,
     ) -> InsertInstructionResult<'_> {
         let results = self.function.dfg.instruction_results(id).to_vec();
 
         let ctrl_typevars = instruction
             .requires_ctrl_typevars()
             .then(|| vecmap(&results, |result| self.function.dfg.type_of_value(*result)));
 
-        let new_results = self.function.dfg.insert_instruction_and_results(
+        let new_results = self.function.dfg.insert_instruction_and_results_if_simplified(
             instruction,
             block,
             ctrl_typevars,
             call_stack,
+            allow_reinsert.then_some(id),
         );
 
         Self::insert_new_instruction_results(&mut self.values, &results, &new_results);
@@ -125,14 +142,16 @@ impl<'f> FunctionInserter<'f> {
 
     /// Modify the values HashMap to remember the mapping between an instruction result's previous
     /// ValueId (from the source_function) and its new ValueId in the destination function.
-    pub(crate) fn insert_new_instruction_results(
+    fn insert_new_instruction_results(
         values: &mut HashMap<ValueId, ValueId>,
         old_results: &[ValueId],
         new_results: &InsertInstructionResult,
     ) {
         assert_eq!(old_results.len(), new_results.len());
         for i in 0..old_results.len() {
-            values.insert(old_results[i], new_results[i]);
+            if old_results[i] != new_results[i] {
+                values.insert(old_results[i], new_results[i]);
+            }
         }
     }
 

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

@@ -840,8 +840,13 @@ impl<'f> Context<'f> {
         let instruction = self.handle_instruction_side_effects(instruction, call_stack);
 
         let instruction_is_allocate = matches!(&instruction, Instruction::Allocate);
-        let results =
-            self.inserter.push_instruction_value(instruction, id, self.target_block, call_stack);
+        let results = self.inserter.push_instruction_value(
+            instruction,
+            id,
+            self.target_block,
+            call_stack,
+            true,
+        );
 
         // Remember an allocate was created local to this branch so that we do not try to merge store
         // values across branches for it later.

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

@@ -427,7 +427,7 @@ impl<'f> LoopInvariantContext<'f> {
                     self.can_hoist_invariant(&loop_context, &block_context, instruction_id);
 
                 if hoist_invariant {
-                    self.inserter.push_instruction(instruction_id, pre_header);
+                    self.inserter.push_instruction(instruction_id, pre_header, false);
 
                     // If we are hoisting a MakeArray instruction,
                     // we need to issue an extra inc_rc in case they are mutated afterward.
@@ -452,7 +452,7 @@ impl<'f> LoopInvariantContext<'f> {
                     if !block_context.is_impure {
                         block_context.is_impure = dfg[instruction_id].has_side_effects(dfg);
                     }
-                    self.inserter.push_instruction(instruction_id, *block);
+                    self.inserter.push_instruction(instruction_id, *block, true);
                 }
 
                 // We will have new IDs after pushing instructions.
@@ -748,7 +748,7 @@ impl<'f> LoopInvariantContext<'f> {
 
         for block in block_order {
             for instruction_id in self.inserter.function.dfg[block].take_instructions() {
-                self.inserter.push_instruction(instruction_id, block);
+                self.inserter.push_instruction(instruction_id, block, true);
             }
             self.inserter.map_terminator_in_place(block);
         }

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

@@ -419,7 +419,17 @@ impl<'f> PerFunctionContext<'f> {
         // and we need to mark those references as used to keep their stores alive.
         let (instruction, loc) = self.inserter.map_instruction(instruction_id);
 
-        match self.inserter.push_instruction_value(instruction, instruction_id, block_id, loc) {
+        // We track which instructions can be removed by ID; if we allowed the same ID to appear multiple times
+        // in a block then we could not tell them apart.
+        let allow_reinsert = false;
+
+        match self.inserter.push_instruction_value(
+            instruction,
+            instruction_id,
+            block_id,
+            loc,
+            allow_reinsert,
+        ) {
             InsertInstructionResult::Results(id, _) => {
                 self.analyze_possibly_simplified_instruction(references, id, false);
             }

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

@@ -1136,7 +1136,7 @@ impl<'f> LoopIteration<'f> {
         // instances of the induction variable or any values that were changed as a result
         // of the new induction variable value.
         for instruction in instructions {
-            self.inserter.push_instruction(instruction, self.insert_block);
+            self.inserter.push_instruction(instruction, self.insert_block, false);
         }
         let mut terminator = self.dfg()[self.source_block].unwrap_terminator().clone();