<div dir="ltr"><div>Thanks!  I've been wanting to do this ever since I added that function.  I just didn't have a second consumer.</div><div><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Aug 15, 2018 at 4:57 PM Caio Marcelo de Oliveira Filho <<a href="mailto:caio.oliveira@intel.com">caio.oliveira@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Reviewed-by: Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com" target="_blank">tarceri@itsqueeze.com</a>><br>
---<br>
 src/compiler/nir/nir_deref.c              | 109 ++++++++++++++++<br>
 src/compiler/nir/nir_deref.h              |  10 ++<br>
 src/compiler/nir/nir_opt_copy_prop_vars.c | 145 ++--------------------<br>
 3 files changed, 132 insertions(+), 132 deletions(-)<br>
<br>
diff --git a/src/compiler/nir/nir_deref.c b/src/compiler/nir/nir_deref.c<br>
index c03acf83597..d013b423a8b 100644<br>
--- a/src/compiler/nir/nir_deref.c<br>
+++ b/src/compiler/nir/nir_deref.c<br>
@@ -270,3 +270,112 @@ nir_fixup_deref_modes(nir_shader *shader)<br>
       }<br>
    }<br>
 }<br>
+<br>
+/** Returns true if the storage referrenced to by deref completely contains<br>
+ * the storage referenced by sub.<br>
+ */<br>
+nir_deref_compare_result<br>
+nir_compare_deref_paths(nir_deref_path *a_path,<br>
+                        nir_deref_path *b_path)<br>
+{<br>
+   if (a_path->path[0]->var != b_path->path[0]->var)<br>
+      return 0;<br>
+<br>
+   /* Start off assuming they fully compare.  We ignore equality for now.  In<br>
+    * the end, we'll determine that by containment.<br>
+    */<br>
+   nir_deref_compare_result result = nir_derefs_may_alias_bit |<br>
+                                     nir_derefs_a_contains_b_bit |<br>
+                                     nir_derefs_b_contains_a_bit;<br>
+<br>
+   nir_deref_instr **a_p = &a_path->path[1];<br>
+   nir_deref_instr **b_p = &b_path->path[1];<br>
+   while (*a_p != NULL && *b_p != NULL) {<br>
+      nir_deref_instr *a_tail = *(a_p++);<br>
+      nir_deref_instr *b_tail = *(b_p++);<br>
+<br>
+      switch (a_tail->deref_type) {<br>
+      case nir_deref_type_array:<br>
+      case nir_deref_type_array_wildcard: {<br>
+         assert(b_tail->deref_type == nir_deref_type_array ||<br>
+                b_tail->deref_type == nir_deref_type_array_wildcard);<br>
+<br>
+         if (a_tail->deref_type == nir_deref_type_array_wildcard) {<br>
+            if (b_tail->deref_type != nir_deref_type_array_wildcard)<br>
+               result &= ~nir_derefs_b_contains_a_bit;<br>
+         } else if (b_tail->deref_type == nir_deref_type_array_wildcard) {<br>
+            if (a_tail->deref_type != nir_deref_type_array_wildcard)<br>
+               result &= ~nir_derefs_a_contains_b_bit;<br>
+         } else {<br>
+            assert(a_tail->deref_type == nir_deref_type_array &&<br>
+                   b_tail->deref_type == nir_deref_type_array);<br>
+            assert(a_tail->arr.index.is_ssa && b_tail->arr.index.is_ssa);<br>
+<br>
+            nir_const_value *a_index_const =<br>
+               nir_src_as_const_value(a_tail->arr.index);<br>
+            nir_const_value *b_index_const =<br>
+               nir_src_as_const_value(b_tail->arr.index);<br>
+            if (a_index_const && b_index_const) {<br>
+               /* If they're both direct and have different offsets, they<br>
+                * don't even alias much less anything else.<br>
+                */<br>
+               if (a_index_const->u32[0] != b_index_const->u32[0])<br>
+                  return 0;<br>
+            } else if (a_tail->arr.index.ssa == b_tail->arr.index.ssa) {<br>
+               /* They're the same indirect, continue on */<br>
+            } else {<br>
+               /* They're not the same index so we can't prove anything about<br>
+                * containment.<br>
+                */<br>
+               result &= ~(nir_derefs_a_contains_b_bit | nir_derefs_b_contains_a_bit);<br>
+            }<br>
+         }<br>
+         break;<br>
+      }<br>
+<br>
+      case nir_deref_type_struct: {<br>
+         /* If they're different struct members, they don't even alias */<br>
+         if (a_tail->strct.index != b_tail->strct.index)<br>
+            return 0;<br>
+         break;<br>
+      }<br>
+<br>
+      default:<br>
+         unreachable("Invalid deref type");<br>
+      }<br>
+   }<br>
+<br>
+   /* If a is longer than b, then it can't contain b */<br>
+   if (*a_p != NULL)<br>
+      result &= ~nir_derefs_a_contains_b_bit;<br>
+   if (*b_p != NULL)<br>
+      result &= ~nir_derefs_b_contains_a_bit;<br>
+<br>
+   /* If a contains b and b contains a they must be equal. */<br>
+   if ((result & nir_derefs_a_contains_b_bit) && (result & nir_derefs_b_contains_a_bit))<br>
+      result |= nir_derefs_equal_bit;<br>
+<br>
+   return result;<br>
+}<br>
+<br>
+nir_deref_compare_result<br>
+nir_compare_derefs(nir_deref_instr *a, nir_deref_instr *b)<br>
+{<br>
+   if (a == b) {<br>
+      return nir_derefs_equal_bit | nir_derefs_may_alias_bit |<br>
+             nir_derefs_a_contains_b_bit | nir_derefs_b_contains_a_bit;<br>
+   }<br>
+<br>
+   nir_deref_path a_path, b_path;<br>
+   nir_deref_path_init(&a_path, a, NULL);<br>
+   nir_deref_path_init(&b_path, b, NULL);<br>
+   assert(a_path.path[0]->deref_type == nir_deref_type_var);<br>
+   assert(b_path.path[0]->deref_type == nir_deref_type_var);<br>
+<br>
+   nir_deref_compare_result result = nir_compare_deref_paths(&a_path, &b_path);<br>
+<br>
+   nir_deref_path_finish(&a_path);<br>
+   nir_deref_path_finish(&b_path);<br>
+<br>
+   return result;<br>
+}<br>
diff --git a/src/compiler/nir/nir_deref.h b/src/compiler/nir/nir_deref.h<br>
index 6f4141aaf82..c61c3f9366f 100644<br>
--- a/src/compiler/nir/nir_deref.h<br>
+++ b/src/compiler/nir/nir_deref.h<br>
@@ -54,6 +54,16 @@ unsigned nir_deref_instr_get_const_offset(nir_deref_instr *deref,<br>
 nir_ssa_def *nir_build_deref_offset(nir_builder *b, nir_deref_instr *deref,<br>
                                     glsl_type_size_align_func size_align);<br>
<br>
+typedef enum {<br>
+   nir_derefs_equal_bit        = (1 << 0),<br>
+   nir_derefs_may_alias_bit    = (1 << 1),<br>
+   nir_derefs_a_contains_b_bit = (1 << 2),<br>
+   nir_derefs_b_contains_a_bit = (1 << 3),<br>
+} nir_deref_compare_result;<br>
+<br>
+nir_deref_compare_result nir_compare_deref_paths(nir_deref_path *a_path, nir_deref_path *b_path);<br>
+nir_deref_compare_result nir_compare_derefs(nir_deref_instr *a, nir_deref_instr *b);<br>
+<br>
 #ifdef __cplusplus<br>
 } /* extern "C" */<br>
 #endif<br>
diff --git a/src/compiler/nir/nir_opt_copy_prop_vars.c b/src/compiler/nir/nir_opt_copy_prop_vars.c<br>
index d8c4cab34d7..9fecaf0eeec 100644<br>
--- a/src/compiler/nir/nir_opt_copy_prop_vars.c<br>
+++ b/src/compiler/nir/nir_opt_copy_prop_vars.c<br>
@@ -114,125 +114,6 @@ copy_entry_remove(struct copy_prop_var_state *state, struct copy_entry *entry)<br>
    list_add(&entry->link, &state->copy_free_list);<br>
 }<br>
<br>
-enum deref_compare_result {<br>
-   derefs_equal_bit = (1 << 0),<br>
-   derefs_may_alias_bit = (1 << 1),<br>
-   derefs_a_contains_b_bit = (1 << 2),<br>
-   derefs_b_contains_a_bit = (1 << 3),<br>
-};<br>
-<br>
-/** Returns true if the storage referrenced to by deref completely contains<br>
- * the storage referenced by sub.<br>
- *<br>
- * NOTE: This is fairly general and could be moved to core NIR if someone else<br>
- * ever needs it.<br>
- */<br>
-static enum deref_compare_result<br>
-compare_deref_paths(nir_deref_path *a_path,<br>
-                    nir_deref_path *b_path)<br>
-{<br>
-   if (a_path->path[0]->var != b_path->path[0]->var)<br>
-      return 0;<br>
-<br>
-   /* Start off assuming they fully compare.  We ignore equality for now.  In<br>
-    * the end, we'll determine that by containment.<br>
-    */<br>
-   enum deref_compare_result result = derefs_may_alias_bit |<br>
-                                      derefs_a_contains_b_bit |<br>
-                                      derefs_b_contains_a_bit;<br>
-<br>
-   nir_deref_instr **a_p = &a_path->path[1];<br>
-   nir_deref_instr **b_p = &b_path->path[1];<br>
-   while (*a_p != NULL && *b_p != NULL) {<br>
-      nir_deref_instr *a_tail = *(a_p++);<br>
-      nir_deref_instr *b_tail = *(b_p++);<br>
-<br>
-      switch (a_tail->deref_type) {<br>
-      case nir_deref_type_array:<br>
-      case nir_deref_type_array_wildcard: {<br>
-         assert(b_tail->deref_type == nir_deref_type_array ||<br>
-                b_tail->deref_type == nir_deref_type_array_wildcard);<br>
-<br>
-         if (a_tail->deref_type == nir_deref_type_array_wildcard) {<br>
-            if (b_tail->deref_type != nir_deref_type_array_wildcard)<br>
-               result &= ~derefs_b_contains_a_bit;<br>
-         } else if (b_tail->deref_type == nir_deref_type_array_wildcard) {<br>
-            if (a_tail->deref_type != nir_deref_type_array_wildcard)<br>
-               result &= ~derefs_a_contains_b_bit;<br>
-         } else {<br>
-            assert(a_tail->deref_type == nir_deref_type_array &&<br>
-                   b_tail->deref_type == nir_deref_type_array);<br>
-            assert(a_tail->arr.index.is_ssa && b_tail->arr.index.is_ssa);<br>
-<br>
-            nir_const_value *a_index_const =<br>
-               nir_src_as_const_value(a_tail->arr.index);<br>
-            nir_const_value *b_index_const =<br>
-               nir_src_as_const_value(b_tail->arr.index);<br>
-            if (a_index_const && b_index_const) {<br>
-               /* If they're both direct and have different offsets, they<br>
-                * don't even alias much less anything else.<br>
-                */<br>
-               if (a_index_const->u32[0] != b_index_const->u32[0])<br>
-                  return 0;<br>
-            } else if (a_tail->arr.index.ssa == b_tail->arr.index.ssa) {<br>
-               /* They're the same indirect, continue on */<br>
-            } else {<br>
-               /* They're not the same index so we can't prove anything about<br>
-                * containment.<br>
-                */<br>
-               result &= ~(derefs_a_contains_b_bit | derefs_b_contains_a_bit);<br>
-            }<br>
-         }<br>
-         break;<br>
-      }<br>
-<br>
-      case nir_deref_type_struct: {<br>
-         /* If they're different struct members, they don't even alias */<br>
-         if (a_tail->strct.index != b_tail->strct.index)<br>
-            return 0;<br>
-         break;<br>
-      }<br>
-<br>
-      default:<br>
-         unreachable("Invalid deref type");<br>
-      }<br>
-   }<br>
-<br>
-   /* If a is longer than b, then it can't contain b */<br>
-   if (*a_p != NULL)<br>
-      result &= ~derefs_a_contains_b_bit;<br>
-   if (*b_p != NULL)<br>
-      result &= ~derefs_b_contains_a_bit;<br>
-<br>
-   /* If a contains b and b contains a they must be equal. */<br>
-   if ((result & derefs_a_contains_b_bit) && (result & derefs_b_contains_a_bit))<br>
-      result |= derefs_equal_bit;<br>
-<br>
-   return result;<br>
-}<br>
-<br>
-static enum deref_compare_result<br>
-compare_derefs(nir_deref_instr *a, nir_deref_instr *b)<br>
-{<br>
-   if (a == b) {<br>
-      return derefs_equal_bit | derefs_may_alias_bit |<br>
-             derefs_a_contains_b_bit | derefs_b_contains_a_bit;<br>
-   }<br>
-<br>
-   nir_deref_path a_path, b_path;<br>
-   nir_deref_path_init(&a_path, a, NULL);<br>
-   nir_deref_path_init(&b_path, b, NULL);<br>
-   assert(a_path.path[0]->deref_type == nir_deref_type_var);<br>
-   assert(b_path.path[0]->deref_type == nir_deref_type_var);<br>
-<br>
-   enum deref_compare_result result = compare_deref_paths(&a_path, &b_path);<br>
-<br>
-   nir_deref_path_finish(&a_path);<br>
-   nir_deref_path_finish(&b_path);<br>
-<br>
-   return result;<br>
-}<br>
-<br>
 static void<br>
 remove_dead_writes(struct copy_prop_var_state *state,<br>
                    struct copy_entry *entry, unsigned write_mask)<br>
@@ -274,10 +155,10 @@ remove_dead_writes(struct copy_prop_var_state *state,<br>
 static struct copy_entry *<br>
 lookup_entry_for_deref(struct copy_prop_var_state *state,<br>
                        nir_deref_instr *deref,<br>
-                       enum deref_compare_result allowed_comparisons)<br>
+                       nir_deref_compare_result allowed_comparisons)<br>
 {<br>
    list_for_each_entry(struct copy_entry, iter, &state->copies, link) {<br>
-      if (compare_derefs(iter->dst, deref) & allowed_comparisons)<br>
+      if (nir_compare_derefs(iter->dst, deref) & allowed_comparisons)<br>
          return iter;<br>
    }<br>
<br>
@@ -289,7 +170,7 @@ mark_aliased_entries_as_read(struct copy_prop_var_state *state,<br>
                              nir_deref_instr *deref, unsigned components)<br>
 {<br>
    list_for_each_entry(struct copy_entry, iter, &state->copies, link) {<br>
-      if (compare_derefs(iter->dst, deref) & derefs_may_alias_bit)<br>
+      if (nir_compare_derefs(iter->dst, deref) & nir_derefs_may_alias_bit)<br>
          iter->comps_may_be_read |= components;<br>
    }<br>
 }<br>
@@ -303,23 +184,23 @@ get_entry_and_kill_aliases(struct copy_prop_var_state *state,<br>
    list_for_each_entry_safe(struct copy_entry, iter, &state->copies, link) {<br>
       if (!iter->src.is_ssa) {<br>
          /* If this write aliases the source of some entry, get rid of it */<br>
-         if (compare_derefs(iter->src.deref, deref) & derefs_may_alias_bit) {<br>
+         if (nir_compare_derefs(iter->src.deref, deref) & nir_derefs_may_alias_bit) {<br>
             copy_entry_remove(state, iter);<br>
             continue;<br>
          }<br>
       }<br>
<br>
-      enum deref_compare_result comp = compare_derefs(iter->dst, deref);<br>
+      nir_deref_compare_result comp = nir_compare_derefs(iter->dst, deref);<br>
       /* This is a store operation.  If we completely overwrite some value, we<br>
        * want to delete any dead writes that may be present.<br>
        */<br>
-      if (comp & derefs_b_contains_a_bit)<br>
+      if (comp & nir_derefs_b_contains_a_bit)<br>
          remove_dead_writes(state, iter, write_mask);<br>
<br>
-      if (comp & derefs_equal_bit) {<br>
+      if (comp & nir_derefs_equal_bit) {<br>
          assert(entry == NULL);<br>
          entry = iter;<br>
-      } else if (comp & derefs_may_alias_bit) {<br>
+      } else if (comp & nir_derefs_may_alias_bit) {<br>
          copy_entry_remove(state, iter);<br>
       }<br>
    }<br>
@@ -613,7 +494,7 @@ copy_prop_vars_block(struct copy_prop_var_state *state,<br>
          mark_aliased_entries_as_read(state, src, comps_read);<br>
<br>
          struct copy_entry *src_entry =<br>
-            lookup_entry_for_deref(state, src, derefs_a_contains_b_bit);<br>
+            lookup_entry_for_deref(state, src, nir_derefs_a_contains_b_bit);<br>
          struct value value;<br>
          if (try_load_from_entry(state, src_entry, b, intrin, src, &value)) {<br>
             if (value.is_ssa) {<br>
@@ -658,7 +539,7 @@ copy_prop_vars_block(struct copy_prop_var_state *state,<br>
           * contains what we're looking for.<br>
           */<br>
          struct copy_entry *store_entry =<br>
-            lookup_entry_for_deref(state, src, derefs_equal_bit);<br>
+            lookup_entry_for_deref(state, src, nir_derefs_equal_bit);<br>
          if (!store_entry)<br>
             store_entry = copy_entry_create(state, src);<br>
<br>
@@ -692,7 +573,7 @@ copy_prop_vars_block(struct copy_prop_var_state *state,<br>
          nir_deref_instr *dst = nir_src_as_deref(intrin->src[0]);<br>
          nir_deref_instr *src = nir_src_as_deref(intrin->src[1]);<br>
<br>
-         if (compare_derefs(src, dst) & derefs_equal_bit) {<br>
+         if (nir_compare_derefs(src, dst) & nir_derefs_equal_bit) {<br>
             /* This is a no-op self-copy.  Get rid of it */<br>
             nir_instr_remove(instr);<br>
             continue;<br>
@@ -701,7 +582,7 @@ copy_prop_vars_block(struct copy_prop_var_state *state,<br>
          mark_aliased_entries_as_read(state, src, 0xf);<br>
<br>
          struct copy_entry *src_entry =<br>
-            lookup_entry_for_deref(state, src, derefs_a_contains_b_bit);<br>
+            lookup_entry_for_deref(state, src, nir_derefs_a_contains_b_bit);<br>
          struct value value;<br>
          if (try_load_from_entry(state, src_entry, b, intrin, src, &value)) {<br>
             if (value.is_ssa) {<br>
@@ -709,7 +590,7 @@ copy_prop_vars_block(struct copy_prop_var_state *state,<br>
                intrin = nir_instr_as_intrinsic(nir_builder_last_instr(b));<br>
             } else {<br>
                /* If this would be a no-op self-copy, don't bother. */<br>
-               if (compare_derefs(value.deref, dst) & derefs_equal_bit)<br>
+               if (nir_compare_derefs(value.deref, dst) & nir_derefs_equal_bit)<br>
                   continue;<br>
<br>
                /* Just turn it into a copy of a different deref */<br>
-- <br>
2.18.0<br>
<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div>