feat(perf): Remove known store values that equal the store address in…

… mem2reg (noir-lang/noir#5935)

feat: remove blocks which consist of only a jump to another block (noir-lang/noir#5889)
fix: use element_size() instead of computing it with division (noir-lang/noir#5939)
feat: Add `StructDefinition::set_fields` (noir-lang/noir#5931)
feat: Only check array bounds in brillig if index is unsafe (noir-lang/noir#5938)
chore: error on false constraint (noir-lang/noir#5890)
feat: warn on unused functions (noir-lang/noir#5892)
feat: add `fmtstr::contents` (noir-lang/noir#5928)
fix: collect functions generated by attributes (noir-lang/noir#5930)
fix: Support debug comptime flag for attributes (noir-lang/noir#5929)
feat: Allow inserting new structs and into programs from attributes (noir-lang/noir#5927)
feat: module attributes (noir-lang/noir#5888)
feat: unquote some value as tokens, not as unquote markers (noir-lang/noir#5924)
feat: check argument count and types on attribute function callback (noir-lang/noir#5921)
feat: LSP will now suggest private items if they are visible (noir-lang/noir#5923)

.noir-sync-commit

@@ -1 +1 @@
-9d2629dd1bb28a8c2ecb4c33d26119da75d626c2
+b84009ca428a5790acf53a6c027146b706170574

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs

@@ -377,13 +377,12 @@ fn handle_array_get_group(
     next_out_of_bounds_index: &mut Option<usize>,
     possible_index_out_of_bounds_indexes: &mut Vec<usize>,
 ) {
-    let Some(array_length) = function.dfg.try_get_array_length(*array) else {
+    if function.dfg.try_get_array_length(*array).is_none() {
         // Nothing to do for slices
         return;
     };
 
-    let flattened_size = function.dfg.type_of_value(*array).flattened_size();
-    let element_size = flattened_size / array_length;
+    let element_size = function.dfg.type_of_value(*array).element_size();
     if element_size <= 1 {
         // Not a composite type
         return;

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs

@@ -117,6 +117,10 @@ struct PerFunctionContext<'f> {
     /// Track a value's last load across all blocks.
     /// If a value is not used in anymore loads we can remove the last store to that value.
     last_loads: HashMap<ValueId, (InstructionId, BasicBlockId)>,
+
+    /// Track whether the last instruction is an inc_rc/dec_rc instruction.
+    /// If it is we should not remove any known store values.
+    inside_rc_reload: bool,
 }
 
 impl<'f> PerFunctionContext<'f> {
@@ -131,6 +135,7 @@ impl<'f> PerFunctionContext<'f> {
             blocks: BTreeMap::new(),
             instructions_to_remove: BTreeSet::new(),
             last_loads: HashMap::default(),
+            inside_rc_reload: false,
         }
     }
 
@@ -281,6 +286,10 @@ impl<'f> PerFunctionContext<'f> {
                 } else {
                     references.mark_value_used(address, self.inserter.function);
 
+                    references.expressions.insert(result, Expression::Other(result));
+                    references.aliases.insert(Expression::Other(result), AliasSet::known(result));
+                    references.set_known_value(result, address);
+
                     self.last_loads.insert(address, (instruction, block_id));
                 }
             }
@@ -296,6 +305,14 @@ impl<'f> PerFunctionContext<'f> {
                     self.instructions_to_remove.insert(*last_store);
                 }
 
+                let known_value = references.get_known_value(value);
+                if let Some(known_value) = known_value {
+                    let known_value_is_address = known_value == address;
+                    if known_value_is_address && !self.inside_rc_reload {
+                        self.instructions_to_remove.insert(instruction);
+                    }
+                }
+
                 references.set_known_value(address, value);
                 references.last_stores.insert(address, instruction);
             }
@@ -350,6 +367,18 @@ impl<'f> PerFunctionContext<'f> {
             Instruction::Call { arguments, .. } => self.mark_all_unknown(arguments, references),
             _ => (),
         }
+
+        self.track_rc_reload_state(instruction);
+    }
+
+    fn track_rc_reload_state(&mut self, instruction: InstructionId) {
+        match &self.inserter.function.dfg[instruction] {
+            // We just had an increment or decrement to an array's reference counter
+            Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. } => {
+                self.inside_rc_reload = true;
+            }
+            _ => self.inside_rc_reload = false,
+        }
     }
 
     fn check_array_aliasing(&self, references: &mut Block, array: ValueId) {

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs

@@ -15,7 +15,9 @@ use acvm::acir::AcirField;
 
 use crate::ssa::{
     ir::{
-        basic_block::BasicBlockId, cfg::ControlFlowGraph, function::Function,
+        basic_block::BasicBlockId,
+        cfg::ControlFlowGraph,
+        function::{Function, RuntimeType},
         instruction::TerminatorInstruction,
     },
     ssa_gen::Ssa,
@@ -30,7 +32,7 @@ impl Ssa {
     /// 5. Replacing any jmpifs with constant conditions with jmps. If this causes the block to have
     ///    only 1 successor then (2) also will be applied.
     ///
-    /// Currently, 1 and 4 are unimplemented.
+    /// Currently, 1 is unimplemented.
     #[tracing::instrument(level = "trace", skip(self))]
     pub(crate) fn simplify_cfg(mut self) -> Self {
         for function in self.functions.values_mut() {
@@ -72,6 +74,10 @@ fn simplify_function(function: &mut Function) {
             // optimizations performed after this point on the same block should check if
             // the inlining here was successful before continuing.
             try_inline_into_predecessor(function, &mut cfg, block, predecessor);
+        } else {
+            drop(predecessors);
+
+            check_for_double_jmp(function, block, &mut cfg);
         }
     }
 }
@@ -102,6 +108,71 @@ fn check_for_constant_jmpif(
     }
 }
 
+/// Optimize a jmp to a block which immediately jmps elsewhere to just jmp to the second block.
+fn check_for_double_jmp(function: &mut Function, block: BasicBlockId, cfg: &mut ControlFlowGraph) {
+    if matches!(function.runtime(), RuntimeType::Acir(_)) {
+        // We can't remove double jumps in ACIR functions as this interferes with the `flatten_cfg` pass.
+        return;
+    }
+
+    if !function.dfg[block].instructions().is_empty()
+        || !function.dfg[block].parameters().is_empty()
+    {
+        return;
+    }
+
+    let Some(TerminatorInstruction::Jmp { destination: final_destination, arguments, .. }) =
+        function.dfg[block].terminator()
+    else {
+        return;
+    };
+
+    if !arguments.is_empty() {
+        return;
+    }
+
+    let final_destination = *final_destination;
+
+    let predecessors: Vec<_> = cfg.predecessors(block).collect();
+    for predecessor_block in predecessors {
+        let terminator_instruction = function.dfg[predecessor_block].take_terminator();
+        let redirected_terminator_instruction = match terminator_instruction {
+            TerminatorInstruction::JmpIf {
+                condition,
+                then_destination,
+                else_destination,
+                call_stack,
+            } => {
+                let then_destination =
+                    if then_destination == block { final_destination } else { then_destination };
+                let else_destination =
+                    if else_destination == block { final_destination } else { else_destination };
+                TerminatorInstruction::JmpIf {
+                    condition,
+                    then_destination,
+                    else_destination,
+                    call_stack,
+                }
+            }
+            TerminatorInstruction::Jmp { destination, arguments, call_stack } => {
+                assert_eq!(
+                    destination, block,
+                    "ICE: predecessor block doesn't jump to current block"
+                );
+                assert!(arguments.is_empty(), "ICE: predecessor jmp has arguments");
+                TerminatorInstruction::Jmp { destination: final_destination, arguments, call_stack }
+            }
+            TerminatorInstruction::Return { .. } => {
+                unreachable!("ICE: predecessor block should not have return terminator instruction")
+            }
+        };
+
+        function.dfg[predecessor_block].set_terminator(redirected_terminator_instruction);
+        cfg.recompute_block(function, predecessor_block);
+    }
+    cfg.recompute_block(function, block);
+}
+
 /// If the given block has block parameters, replace them with the jump arguments from the predecessor.
 ///
 /// Currently, if this function is needed, `try_inline_into_predecessor` will also always apply,

noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "empty_composite_array_get"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.32.0"
+
+[dependencies]

noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/Prover.toml

@@ -0,0 +1 @@
+empty_input = []

noir/noir-repo/test_programs/execution_failure/empty_composite_array_get/src/main.nr

@@ -0,0 +1,5 @@
+fn main(empty_input: [(Field, Field); 0]) {
+    let empty_array: [(Field, Field); 0] = [];
+    let _ = empty_input[0];
+    let _ = empty_array[0];
+}