nir/vars_to_ssa: Always do OOB load/store removal.

We elminated OOB loads while renaming vars to SSA.  However, if the OOB
load only appeared after some other passes had constant folded, there may
be no renaming work to do, at which point we'd leave the OOB load deref
around without renaming it or deleting it.  For vc4, this was quite a
surprise and caused a regression when we stop eliminating some OOB
accesses at the GLSL level.

Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18466>

src/compiler/nir/nir_lower_vars_to_ssa.c

@@ -402,34 +402,65 @@ register_complex_use(nir_deref_instr *deref,
    node->has_complex_use = true;
 }
 
-static void
+static bool
 register_load_instr(nir_intrinsic_instr *load_instr,
                     struct lower_variables_state *state)
 {
    nir_deref_instr *deref = nir_src_as_deref(load_instr->src[0]);
    struct deref_node *node = get_deref_node(deref, state);
-   if (node == NULL || node == UNDEF_NODE)
-      return;
+   if (node == NULL)
+      return false;
+
+   /* Replace out-of-bounds load derefs with an undef, so that they don't get
+    * left around when a driver has lowered all indirects and thus doesn't
+    * expect any array derefs at all after vars_to_ssa.
+    */
+   if (node == UNDEF_NODE) {
+      nir_ssa_undef_instr *undef =
+         nir_ssa_undef_instr_create(state->shader,
+                                    load_instr->num_components,
+                                    load_instr->dest.ssa.bit_size);
+
+      nir_instr_insert_before(&load_instr->instr, &undef->instr);
+      nir_instr_remove(&load_instr->instr);
+
+      nir_ssa_def_rewrite_uses(&load_instr->dest.ssa, &undef->def);
+      return true;
+   }
 
    if (node->loads == NULL)
       node->loads = _mesa_pointer_set_create(state->dead_ctx);
 
    _mesa_set_add(node->loads, load_instr);
+
+   return false;
 }
 
-static void
+static bool
 register_store_instr(nir_intrinsic_instr *store_instr,
                      struct lower_variables_state *state)
 {
    nir_deref_instr *deref = nir_src_as_deref(store_instr->src[0]);
    struct deref_node *node = get_deref_node(deref, state);
-   if (node == NULL || node == UNDEF_NODE)
-      return;
+
+   /* Drop out-of-bounds store derefs, so that they don't get left around when a
+    * driver has lowered all indirects and thus doesn't expect any array derefs
+    * at all after vars_to_ssa.
+    */
+   if (node == UNDEF_NODE) {
+      nir_instr_remove(&store_instr->instr);
+      return true;
+   }
+
+   if (node == NULL)
+      return false;
 
    if (node->stores == NULL)
       node->stores = _mesa_pointer_set_create(state->dead_ctx);
 
    _mesa_set_add(node->stores, store_instr);
+
+   return false;
 }
 
 static void
@@ -449,10 +480,12 @@ register_copy_instr(nir_intrinsic_instr *copy_instr,
    }
 }
 
-static void
+static bool
 register_variable_uses(nir_function_impl *impl,
                        struct lower_variables_state *state)
 {
+   bool progress = false;
+
    nir_foreach_block(block, impl) {
       nir_foreach_instr_safe(instr, block) {
          switch (instr->type) {
@@ -471,11 +504,11 @@ register_variable_uses(nir_function_impl *impl,
 
             switch (intrin->intrinsic) {
             case nir_intrinsic_load_deref:
-               register_load_instr(intrin, state);
+               progress = register_load_instr(intrin, state) || progress;
                break;
 
             case nir_intrinsic_store_deref:
-               register_store_instr(intrin, state);
+               progress = register_store_instr(intrin, state) || progress;
                break;
 
             case nir_intrinsic_copy_deref:
@@ -493,6 +526,7 @@ register_variable_uses(nir_function_impl *impl,
          }
       }
    }
+   return progress;
 }
 
 /* Walks over all of the copy instructions to or from the given deref_node
@@ -562,24 +596,8 @@ rename_variables(struct lower_variables_state *state)
             if (node == NULL)
                continue;
 
-            if (node == UNDEF_NODE) {
-               /* If we hit this path then we are referencing an invalid
-                * value.  Most likely, we unrolled something and are
-                * reading past the end of some array.  In any case, this
-                * should result in an undefined value.
-                */
-               nir_ssa_undef_instr *undef =
-                  nir_ssa_undef_instr_create(state->shader,
-                                             intrin->num_components,
-                                             intrin->dest.ssa.bit_size);
-
-               nir_instr_insert_before(&intrin->instr, &undef->instr);
-               nir_instr_remove(&intrin->instr);
-
-               nir_ssa_def_rewrite_uses(&intrin->dest.ssa,
-                                        &undef->def);
-               continue;
-            }
+            /* Should have been removed before rename_variables(). */
+            assert(node != UNDEF_NODE);
 
             if (!node->lower_to_ssa)
                continue;
@@ -615,16 +633,12 @@ rename_variables(struct lower_variables_state *state)
             if (node == NULL)
                continue;
 
+            /* Should have been removed before rename_variables(). */
+            assert(node != UNDEF_NODE);
+
             assert(intrin->src[1].is_ssa);
             nir_ssa_def *value = intrin->src[1].ssa;
 
-            if (node == UNDEF_NODE) {
-               /* Probably an out-of-bounds array store.  That should be a
-                * no-op. */
-               nir_instr_remove(&intrin->instr);
-               continue;
-            }
-
             if (!node->lower_to_ssa)
                continue;
 
@@ -720,9 +734,7 @@ nir_lower_vars_to_ssa_impl(nir_function_impl *impl)
    /* Build the initial deref structures and direct_deref_nodes table */
    state.add_to_direct_deref_nodes = true;
 
-   register_variable_uses(impl, &state);
-
-   bool progress = false;
+   bool progress = register_variable_uses(impl, &state);
 
    nir_metadata_require(impl, nir_metadata_block_index);