[Mesa-dev] [PATCH 1/4] nir/instr_set: Add an allow_loads field

[Iago Toral Quiroga]

We need this so we can configure different behaviors for passes that
cannot deal with side-effectful instructions (CSE) and passes that can
(we will add a load-combine pass shortly).

For now, when allow_loads is true, we let the instruction set rewrite
SSBO loads.
---
 src/glsl/nir/nir_instr_set.c | 51 ++++++++++++++++++++++++++++----------------
 src/glsl/nir/nir_instr_set.h | 20 ++++++++++++-----
 src/glsl/nir/nir_opt_cse.c   |  4 ++--
 3 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/src/glsl/nir/nir_instr_set.c b/src/glsl/nir/nir_instr_set.c
index d3f939f..583618f 100644
--- a/src/glsl/nir/nir_instr_set.c
+++ b/src/glsl/nir/nir_instr_set.c
@@ -398,6 +398,13 @@ dest_is_ssa(nir_dest *dest, void *data)
    return dest->is_ssa;
 }
 
+static bool
+is_load(nir_intrinsic_instr *instr)
+{
+   return instr->intrinsic == nir_intrinsic_load_ssbo ||
+          instr->intrinsic == nir_intrinsic_load_ssbo_indirect;
+}
+
 /* This function determines if uses of an instruction can safely be rewritten
  * to use another identical instruction instead. Note that this function must
  * be kept in sync with hash_instr() and nir_instrs_equal() -- only
@@ -406,7 +413,7 @@ dest_is_ssa(nir_dest *dest, void *data)
  */
 
 static bool
-instr_can_rewrite(nir_instr *instr)
+instr_can_rewrite(nir_instr *instr, bool allow_loads)
 {
    /* We only handle SSA. */
    if (!nir_foreach_dest(instr, dest_is_ssa, NULL) ||
@@ -428,11 +435,15 @@ instr_can_rewrite(nir_instr *instr)
       return true;
    }
    case nir_instr_type_intrinsic: {
+      nir_intrinsic_instr *intrinsic = nir_instr_as_intrinsic(instr);
       const nir_intrinsic_info *info =
-         &nir_intrinsic_infos[nir_instr_as_intrinsic(instr)->intrinsic];
-      return (info->flags & NIR_INTRINSIC_CAN_ELIMINATE) &&
-             (info->flags & NIR_INTRINSIC_CAN_REORDER) &&
-             info->num_variables == 0; /* not implemented yet */
+         &nir_intrinsic_infos[intrinsic->intrinsic];
+      bool can_eliminate_and_reorder =
+         (info->flags & NIR_INTRINSIC_CAN_ELIMINATE) &&
+         (info->flags & NIR_INTRINSIC_CAN_REORDER) &&
+         info->num_variables == 0; /* not implemented yet */
+      return can_eliminate_and_reorder ?
+         true: allow_loads && is_load(intrinsic);
    }
    case nir_instr_type_call:
    case nir_instr_type_jump:
@@ -475,25 +486,29 @@ cmp_func(const void *data1, const void *data2)
    return nir_instrs_equal(data1, data2);
 }
 
-struct set *
-nir_instr_set_create(void *mem_ctx)
+struct nir_instr_set *
+nir_instr_set_create(void *mem_ctx, bool allow_loads)
 {
-   return _mesa_set_create(mem_ctx, hash_instr, cmp_func);
+   struct nir_instr_set *instr_set = ralloc(mem_ctx, struct nir_instr_set);
+   instr_set->set = _mesa_set_create(mem_ctx, hash_instr, cmp_func);
+   instr_set->allow_loads = allow_loads;
+   return instr_set;
 }
 
 void
-nir_instr_set_destroy(struct set *instr_set)
+nir_instr_set_destroy(struct nir_instr_set *instr_set)
 {
-   _mesa_set_destroy(instr_set, NULL);
+   _mesa_set_destroy(instr_set->set, NULL);
+   ralloc_free(instr_set);
 }
 
 bool
-nir_instr_set_add_or_rewrite(struct set *instr_set, nir_instr *instr)
+nir_instr_set_add_or_rewrite(struct nir_instr_set *instr_set, nir_instr *instr)
 {
-   if (!instr_can_rewrite(instr))
+   if (!instr_can_rewrite(instr, instr_set->allow_loads))
       return false;
 
-   struct set_entry *entry = _mesa_set_search(instr_set, instr);
+   struct set_entry *entry = _mesa_set_search(instr_set->set, instr);
    if (entry) {
       nir_ssa_def *def = nir_instr_get_dest_ssa_def(instr);
       nir_ssa_def *new_def =
@@ -502,18 +517,18 @@ nir_instr_set_add_or_rewrite(struct set *instr_set, nir_instr *instr)
       return true;
    }
 
-   _mesa_set_add(instr_set, instr);
+   _mesa_set_add(instr_set->set, instr);
    return false;
 }
 
 void
-nir_instr_set_remove(struct set *instr_set, nir_instr *instr)
+nir_instr_set_remove(struct nir_instr_set *instr_set, nir_instr *instr)
 {
-   if (!instr_can_rewrite(instr))
+   if (!instr_can_rewrite(instr, instr_set->allow_loads))
       return;
 
-   struct set_entry *entry = _mesa_set_search(instr_set, instr);
+   struct set_entry *entry = _mesa_set_search(instr_set->set, instr);
    if (entry)
-      _mesa_set_remove(instr_set, entry);
+      _mesa_set_remove(instr_set->set, entry);
 }
 
diff --git a/src/glsl/nir/nir_instr_set.h b/src/glsl/nir/nir_instr_set.h
index 939e8dd..d00f9a8 100644
--- a/src/glsl/nir/nir_instr_set.h
+++ b/src/glsl/nir/nir_instr_set.h
@@ -38,11 +38,20 @@
 
 /*@{*/
 
-/** Creates an instruction set, using a given ralloc mem_ctx */
-struct set *nir_instr_set_create(void *mem_ctx);
+struct nir_instr_set {
+   struct set *set;
+   bool allow_loads;
+};
+
+/**
+ * Creates an instruction set, using a given ralloc mem_ctx. If allow_loads
+ * is true, then side-effectful instructions like SSBO loads that can't be
+ * freely moved around can still be rewritten.
+ */
+struct nir_instr_set *nir_instr_set_create(void *mem_ctx, bool allow_loads);
 
 /** Destroys an instruction set. */
-void nir_instr_set_destroy(struct set *instr_set);
+void nir_instr_set_destroy(struct nir_instr_set *instr_set);
 
 /**
  * Adds an instruction to an instruction set if it doesn't exist, or if it
@@ -50,13 +59,14 @@ void nir_instr_set_destroy(struct set *instr_set);
  * already-inserted instruction. Returns 'true' if the uses of the instruction
  * were rewritten.
  */
-bool nir_instr_set_add_or_rewrite(struct set *instr_set, nir_instr *instr);
+bool nir_instr_set_add_or_rewrite(struct nir_instr_set *instr_set,
+                                  nir_instr *instr);
 
 /**
  * Removes an instruction from an instruction set, so that other instructions
  * won't be merged with it.
  */
-void nir_instr_set_remove(struct set *instr_set, nir_instr *instr);
+void nir_instr_set_remove(struct nir_instr_set *instr_set, nir_instr *instr);
 
 /*@}*/
 
diff --git a/src/glsl/nir/nir_opt_cse.c b/src/glsl/nir/nir_opt_cse.c
index 93a6635..eb7296a 100644
--- a/src/glsl/nir/nir_opt_cse.c
+++ b/src/glsl/nir/nir_opt_cse.c
@@ -39,7 +39,7 @@
  */
 
 static bool
-cse_block(nir_block *block, struct set *instr_set)
+cse_block(nir_block *block, struct nir_instr_set *instr_set)
 {
    bool progress = false;
 
@@ -64,7 +64,7 @@ cse_block(nir_block *block, struct set *instr_set)
 static bool
 nir_opt_cse_impl(nir_function_impl *impl)
 {
-   struct set *instr_set = nir_instr_set_create(NULL);
+   struct nir_instr_set *instr_set = nir_instr_set_create(NULL, false);
 
    nir_metadata_require(impl, nir_metadata_dominance);
 
-- 
1.9.1