fix(ssa): Swap Brillig index shift and DIE in minimal pipeline

compiler/noirc_evaluator/src/ssa.rs

@@ -207,7 +207,10 @@ pub fn primary_passes(options: &SsaEvaluatorOptions) -> Vec<SsaPass> {
         // We can safely place the pass before DIE as that pass only removes instructions.
         // We also need DIE's tracking of used globals in case the array get transformations
         // end up using an existing constant from the globals space.
+        // This pass might result in otherwise unused global constant becoming used,
+        // because the creation of shifted index constants can reuse their IDs.
         SsaPass::new(Ssa::brillig_array_get_and_set, "Brillig Array Get and Set Optimizations"),
+        // Perform another DIE pass to update the used globals after offsetting Brillig indexes.
         SsaPass::new(Ssa::dead_instruction_elimination, "Dead Instruction Elimination"),
         // A function can be potentially unreachable post-DIE if all calls to that function were removed.
         SsaPass::new(Ssa::remove_unreachable_functions, "Removing Unreachable Functions"),
@@ -228,7 +231,7 @@ pub fn secondary_passes(brillig: &Brillig) -> Vec<SsaPass> {
         // It could happen that we inlined all calls to a given brillig function.
         // In that case it's unused so we can remove it. This is what we check next.
         SsaPass::new(Ssa::remove_unreachable_functions, "Removing Unreachable Functions"),
-        SsaPass::new(Ssa::dead_instruction_elimination_acir, "Dead Instruction Elimination"),
+        SsaPass::new(Ssa::dead_instruction_elimination_acir, "Dead Instruction Elimination - ACIR"),
     ]
 }
 
@@ -248,10 +251,12 @@ pub fn minimal_passes() -> Vec<SsaPass<'static>> {
         // which was called in the AST not being called in the SSA. Such functions would cause
         // panics later, when we are looking for global allocations.
         SsaPass::new(Ssa::remove_unreachable_functions, "Removing Unreachable Functions"),
-        // We need a DIE pass to populate `used_globals`, otherwise it will panic later.
-        SsaPass::new(Ssa::dead_instruction_elimination, "Dead Instruction Elimination"),
         // We need to add an offset to constant array indices in Brillig.
+        // This can change which globals are used, because constant creation might result
+        // in the (re)use of otherwise unused global values.
         SsaPass::new(Ssa::brillig_array_get_and_set, "Brillig Array Get and Set Optimizations"),
+        // We need a DIE pass to populate `used_globals`, otherwise it will panic later.
+        SsaPass::new(Ssa::dead_instruction_elimination, "Dead Instruction Elimination"),
     ]
 }
 

tooling/nargo_cli/build.rs

@@ -119,14 +119,14 @@
 ];
 
 /// `nargo interpret` ignored tests, either because they don't currently work or
 /// becuase they are too slow to run.
 const IGNORED_INTERPRET_EXECUTION_TESTS: [&str; 1] = [
     // slow
     "regression_4709",
 ];
 
 /// `nargo execute --minimal-ssa` ignored tests
-const IGNORED_MINIMAL_EXECUTION_TESTS: [&str; 13] = [
+const IGNORED_MINIMAL_EXECUTION_TESTS: [&str; 12] = [
     // internal error: entered unreachable code: unsupported function call type Intrinsic(AssertConstant)
     // These tests contain calls to `assert_constant`, which are evaluated and removed in the full SSA
     // pipeline, but in the minimal they are untouched, and trying to remove them causes a failure because
@@ -141,8 +141,6 @@
     "pedersen_commitment",
     "simple_shield",
     "strings",
-    // ICE: Global value not found in cache v0
-    "integer_array_indexing",
     // The minimum SSA pipeline only works with Brillig: \'zeroed_lambda\' needs to be unconstrained
     "lambda_from_dynamic_if",
     // This relies on maximum inliner setting
@@ -162,7 +160,7 @@
     // There's no "src/main.nr" here so it's trickier to make this work
     "overlapping_dep_and_mod",
     // bug
     "poseidonsponge_x5_254",
     // bug
     "regression_5045",
     // bug
@@ -813,7 +811,7 @@
     writeln!(test_file, "}}").unwrap();
 }
 
 /// Here we check, for every program in `test_programs/exeuction_success`, that:
 /// 1. `nargo expand` works on it
 /// 2. That the output of the original program is the same as the output of the expanded program
 ///    (that is, we run `nargo execute` on the original program and the expanded program and compare the output)