Mesa (staging/21.0): spirv: Don't remove variables used by resource indexing intrinsics

[GitLab Mirror]

Module: Mesa
Branch: staging/21.0
Commit: 96de6d77548c3f56fc8cb4d0af0c6d7bfa5f1e6f
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=96de6d77548c3f56fc8cb4d0af0c6d7bfa5f1e6f

Author: Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com>
Date:   Thu Jan 21 10:14:36 2021 -0800

spirv: Don't remove variables used by resource indexing intrinsics

In Vulkan, for some variable modes, the generated NIR will have derefs
pointing to resource index intrinsics instead of the variable.  This
was letting nir_remove_dead_variables pass remove those variables,
which would lose information relevant for later passes after
spirv2nir.

Add a set to keep track of such variables and prevent them to be
removed when producing the NIR output.

Issue reported by Rhys.

Fixes: c4c9c780b13 ("spirv: Remove more dead variables")
Reviewed-by: Rhys Perry <pendingchaos02 at gmail.com>
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8706>
(cherry picked from commit 10b3eecd361af465e0e207fb18553ae37b924c45)

---

 .pick_status.json                  |  2 +-
 src/compiler/spirv/spirv_to_nir.c  | 17 ++++++++++++++++-
 src/compiler/spirv/vtn_private.h   |  7 +++++++
 src/compiler/spirv/vtn_variables.c |  5 +++++
 4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index 9d43eb7f9a7..c3fa8f97954 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -229,7 +229,7 @@
         "description": "spirv: Don't remove variables used by resource indexing intrinsics",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "master_sha": null,
         "because_sha": "c4c9c780b131939fa10ede84e079a90fc090e17a"
     },
diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c
index d5fe6483eb8..6864bb61cb2 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -5728,6 +5728,9 @@ vtn_create_builder(const uint32_t *words, size_t word_count,
    b->value_id_bound = value_id_bound;
    b->values = rzalloc_array(b, struct vtn_value, value_id_bound);
 
+   if (b->options->environment == NIR_SPIRV_VULKAN)
+      b->vars_used_indirectly = _mesa_pointer_set_create(b);
+
    return b;
  fail:
    ralloc_free(b);
@@ -5805,6 +5808,13 @@ vtn_emit_kernel_entry_point_wrapper(struct vtn_builder *b,
    return main_entry_point;
 }
 
+static bool
+can_remove(nir_variable *var, void *data)
+{
+   const struct set *vars_used_indirectly = data;
+   return !_mesa_set_search(vars_used_indirectly, var);
+}
+
 nir_shader *
 spirv_to_nir(const uint32_t *words, size_t word_count,
              struct nir_spirv_specialization *spec, unsigned num_spec,
@@ -5953,7 +5963,12 @@ spirv_to_nir(const uint32_t *words, size_t word_count,
     */
    nir_lower_variable_initializers(b->shader, nir_var_shader_out |
                                               nir_var_system_value);
-   nir_remove_dead_variables(b->shader, ~nir_var_function_temp, NULL);
+   const nir_remove_dead_variables_options dead_opts = {
+      .can_remove_var = can_remove,
+      .can_remove_var_data = b->vars_used_indirectly,
+   };
+   nir_remove_dead_variables(b->shader, ~nir_var_function_temp,
+                             b->vars_used_indirectly ? &dead_opts : NULL);
 
    /* We sometimes generate bogus derefs that, while never used, give the
     * validator a bit of heartburn.  Run dead code to get rid of them.
diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h
index 11b95b60f4e..c12b27e8eed 100644
--- a/src/compiler/spirv/vtn_private.h
+++ b/src/compiler/spirv/vtn_private.h
@@ -674,6 +674,13 @@ struct vtn_builder {
     */
    struct hash_table *phi_table;
 
+   /* In Vulkan, when lowering some modes variable access, the derefs of the
+    * variables are replaced with a resource index intrinsics, leaving the
+    * variable hanging.  This set keeps track of them so they can be filtered
+    * (and not removed) in nir_remove_dead_variables.
+    */
+   struct set *vars_used_indirectly;
+
    unsigned num_specializations;
    struct nir_spirv_specialization *specializations;
 
diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c
index 6cccb540090..1481e6b4e57 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -237,6 +237,11 @@ vtn_variable_resource_index(struct vtn_builder *b, struct vtn_variable *var,
       desc_array_index = nir_imm_int(&b->nb, 0);
    }
 
+   if (b->vars_used_indirectly) {
+      vtn_assert(var->var);
+      _mesa_set_add(b->vars_used_indirectly, var->var);
+   }
+
    nir_intrinsic_instr *instr =
       nir_intrinsic_instr_create(b->nb.shader,
                                  nir_intrinsic_vulkan_resource_index);