Re: [Mesa-dev] [PATCH v4 106/129] nir: Rework opt_copy_prop_vars to use deref instructions
On June 22, 2018 11:52:54 Kenneth Graunke wrote:
On Friday, June 22, 2018 8:28:47 AM PDT Jason Ekstrand wrote:
On Thu, Jun 21, 2018 at 1:03 AM, Kenneth Graunke
wrote:
On Thursday, May 31, 2018 10:06:47 PM PDT Jason Ekstrand wrote:
[snip]
@@ -529,57 +509,55 @@ static bool
load_from_deref_entry_value(struct copy_prop_var_state *state,
struct copy_entry *entry,
nir_builder *b, nir_intrinsic_instr *intrin,
-nir_deref_var *src, struct value *value)
+nir_deref_instr *src, struct value *value)
{
*value = entry->src;
- /* Walk the deref to get the two tails and also figure out if we
need to
-* specialize any wildcards.
-*/
- bool need_to_specialize_wildcards = false;
- nir_deref *entry_tail = >dst->deref;
- nir_deref *src_tail = >deref;
- while (entry_tail->child && src_tail->child) {
- assert(src_tail->child->deref_type ==
entry_tail->child->deref_type);
- if (src_tail->child->deref_type == nir_deref_type_array) {
- nir_deref_array *entry_arr = nir_deref_as_array(entry_tail-
child);
- nir_deref_array *src_arr = nir_deref_as_array(src_tail->
child);
-
I think there might be a bug here...note this condition...
- if (src_arr->deref_array_type != nir_deref_array_type_wildcard
&&
- entry_arr->deref_array_type ==
nir_deref_array_type_wildcard)
Old: Source NOT wildcard, dest is wildcard.
-need_to_specialize_wildcards = true;
- }
+ b->cursor = nir_instr_remove(>instr);
- entry_tail = entry_tail->child;
- src_tail = src_tail->child;
+ nir_deref_path entry_dst_path, src_path;
+ nir_deref_path_init(_dst_path, entry->dst, state->mem_ctx);
+ nir_deref_path_init(_path, src, state->mem_ctx);
+
+ bool need_to_specialize_wildcards = false;
+ nir_deref_instr **entry_p = _dst_path.path[1];
+ nir_deref_instr **src_p = _path.path[1];
+ while (*entry_p && *src_p) {
+ nir_deref_instr *entry_tail = *entry_p++;
+ nir_deref_instr *src_tail = *src_p++;
+
+ if (src_tail->deref_type == nir_deref_type_array &&
+ entry_tail->deref_type == nir_deref_type_array_wildcard)
New: Source IS wildcard, dest is wildcard. I think you want != on the
source condition to match the old behavior.
I don't think there's a bug here. With the old mechanism, we had
deref_type_array and then within that deref_array_type_direct, _indirect,
and _wildcard. In the new scheme, we have deref_type_array and
deref_type_array_wildcard. So deref_type == nir_deref_type_array in the
new translates to deref_type == nir_deref_type_array && deref_array_type !=
nir_deref_array_type_wildcard. Does that make sense?
I missed that it also changed from != wildcard to == array. :(
No worries. Thanks for double checking!
--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 106/129] nir: Rework opt_copy_prop_vars to use deref instructions
On Friday, June 22, 2018 8:28:47 AM PDT Jason Ekstrand wrote:
> On Thu, Jun 21, 2018 at 1:03 AM, Kenneth Graunke
> wrote:
>
> > On Thursday, May 31, 2018 10:06:47 PM PDT Jason Ekstrand wrote:
> > [snip]
> > > @@ -529,57 +509,55 @@ static bool
> > > load_from_deref_entry_value(struct copy_prop_var_state *state,
> > > struct copy_entry *entry,
> > > nir_builder *b, nir_intrinsic_instr *intrin,
> > > -nir_deref_var *src, struct value *value)
> > > +nir_deref_instr *src, struct value *value)
> > > {
> > > *value = entry->src;
> > >
> > > - /* Walk the deref to get the two tails and also figure out if we
> > need to
> > > -* specialize any wildcards.
> > > -*/
> > > - bool need_to_specialize_wildcards = false;
> > > - nir_deref *entry_tail = >dst->deref;
> > > - nir_deref *src_tail = >deref;
> > > - while (entry_tail->child && src_tail->child) {
> > > - assert(src_tail->child->deref_type ==
> > entry_tail->child->deref_type);
> > > - if (src_tail->child->deref_type == nir_deref_type_array) {
> > > - nir_deref_array *entry_arr = nir_deref_as_array(entry_tail-
> > >child);
> > > - nir_deref_array *src_arr = nir_deref_as_array(src_tail->
> > child);
> > > -
> >
> > I think there might be a bug here...note this condition...
> >
> > > - if (src_arr->deref_array_type != nir_deref_array_type_wildcard
> > &&
> > > - entry_arr->deref_array_type ==
> > nir_deref_array_type_wildcard)
> >
> > Old: Source NOT wildcard, dest is wildcard.
> >
> > > -need_to_specialize_wildcards = true;
> > > - }
> > > + b->cursor = nir_instr_remove(>instr);
> > >
> > > - entry_tail = entry_tail->child;
> > > - src_tail = src_tail->child;
> > > + nir_deref_path entry_dst_path, src_path;
> > > + nir_deref_path_init(_dst_path, entry->dst, state->mem_ctx);
> > > + nir_deref_path_init(_path, src, state->mem_ctx);
> > > +
> > > + bool need_to_specialize_wildcards = false;
> > > + nir_deref_instr **entry_p = _dst_path.path[1];
> > > + nir_deref_instr **src_p = _path.path[1];
> > > + while (*entry_p && *src_p) {
> > > + nir_deref_instr *entry_tail = *entry_p++;
> > > + nir_deref_instr *src_tail = *src_p++;
> > > +
> > > + if (src_tail->deref_type == nir_deref_type_array &&
> > > + entry_tail->deref_type == nir_deref_type_array_wildcard)
> >
> > New: Source IS wildcard, dest is wildcard. I think you want != on the
> > source condition to match the old behavior.
> >
>
> I don't think there's a bug here. With the old mechanism, we had
> deref_type_array and then within that deref_array_type_direct, _indirect,
> and _wildcard. In the new scheme, we have deref_type_array and
> deref_type_array_wildcard. So deref_type == nir_deref_type_array in the
> new translates to deref_type == nir_deref_type_array && deref_array_type !=
> nir_deref_array_type_wildcard. Does that make sense?
>
I missed that it also changed from != wildcard to == array. :(
Looks okay after all.
--Ken
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 106/129] nir: Rework opt_copy_prop_vars to use deref instructions
On Thu, Jun 21, 2018 at 1:03 AM, Kenneth Graunke
wrote:
> On Thursday, May 31, 2018 10:06:47 PM PDT Jason Ekstrand wrote:
> [snip]
> > @@ -529,57 +509,55 @@ static bool
> > load_from_deref_entry_value(struct copy_prop_var_state *state,
> > struct copy_entry *entry,
> > nir_builder *b, nir_intrinsic_instr *intrin,
> > -nir_deref_var *src, struct value *value)
> > +nir_deref_instr *src, struct value *value)
> > {
> > *value = entry->src;
> >
> > - /* Walk the deref to get the two tails and also figure out if we
> need to
> > -* specialize any wildcards.
> > -*/
> > - bool need_to_specialize_wildcards = false;
> > - nir_deref *entry_tail = >dst->deref;
> > - nir_deref *src_tail = >deref;
> > - while (entry_tail->child && src_tail->child) {
> > - assert(src_tail->child->deref_type ==
> entry_tail->child->deref_type);
> > - if (src_tail->child->deref_type == nir_deref_type_array) {
> > - nir_deref_array *entry_arr = nir_deref_as_array(entry_tail-
> >child);
> > - nir_deref_array *src_arr = nir_deref_as_array(src_tail->
> child);
> > -
>
> I think there might be a bug here...note this condition...
>
> > - if (src_arr->deref_array_type != nir_deref_array_type_wildcard
> &&
> > - entry_arr->deref_array_type ==
> nir_deref_array_type_wildcard)
>
> Old: Source NOT wildcard, dest is wildcard.
>
> > -need_to_specialize_wildcards = true;
> > - }
> > + b->cursor = nir_instr_remove(>instr);
> >
> > - entry_tail = entry_tail->child;
> > - src_tail = src_tail->child;
> > + nir_deref_path entry_dst_path, src_path;
> > + nir_deref_path_init(_dst_path, entry->dst, state->mem_ctx);
> > + nir_deref_path_init(_path, src, state->mem_ctx);
> > +
> > + bool need_to_specialize_wildcards = false;
> > + nir_deref_instr **entry_p = _dst_path.path[1];
> > + nir_deref_instr **src_p = _path.path[1];
> > + while (*entry_p && *src_p) {
> > + nir_deref_instr *entry_tail = *entry_p++;
> > + nir_deref_instr *src_tail = *src_p++;
> > +
> > + if (src_tail->deref_type == nir_deref_type_array &&
> > + entry_tail->deref_type == nir_deref_type_array_wildcard)
>
> New: Source IS wildcard, dest is wildcard. I think you want != on the
> source condition to match the old behavior.
>
I don't think there's a bug here. With the old mechanism, we had
deref_type_array and then within that deref_array_type_direct, _indirect,
and _wildcard. In the new scheme, we have deref_type_array and
deref_type_array_wildcard. So deref_type == nir_deref_type_array in the
new translates to deref_type == nir_deref_type_array && deref_array_type !=
nir_deref_array_type_wildcard. Does that make sense?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 106/129] nir: Rework opt_copy_prop_vars to use deref instructions
On Thursday, May 31, 2018 10:06:47 PM PDT Jason Ekstrand wrote:
[snip]
> @@ -529,57 +509,55 @@ static bool
> load_from_deref_entry_value(struct copy_prop_var_state *state,
> struct copy_entry *entry,
> nir_builder *b, nir_intrinsic_instr *intrin,
> -nir_deref_var *src, struct value *value)
> +nir_deref_instr *src, struct value *value)
> {
> *value = entry->src;
>
> - /* Walk the deref to get the two tails and also figure out if we need to
> -* specialize any wildcards.
> -*/
> - bool need_to_specialize_wildcards = false;
> - nir_deref *entry_tail = >dst->deref;
> - nir_deref *src_tail = >deref;
> - while (entry_tail->child && src_tail->child) {
> - assert(src_tail->child->deref_type == entry_tail->child->deref_type);
> - if (src_tail->child->deref_type == nir_deref_type_array) {
> - nir_deref_array *entry_arr = nir_deref_as_array(entry_tail->child);
> - nir_deref_array *src_arr = nir_deref_as_array(src_tail->child);
> -
I think there might be a bug here...note this condition...
> - if (src_arr->deref_array_type != nir_deref_array_type_wildcard &&
> - entry_arr->deref_array_type == nir_deref_array_type_wildcard)
Old: Source NOT wildcard, dest is wildcard.
> -need_to_specialize_wildcards = true;
> - }
> + b->cursor = nir_instr_remove(>instr);
>
> - entry_tail = entry_tail->child;
> - src_tail = src_tail->child;
> + nir_deref_path entry_dst_path, src_path;
> + nir_deref_path_init(_dst_path, entry->dst, state->mem_ctx);
> + nir_deref_path_init(_path, src, state->mem_ctx);
> +
> + bool need_to_specialize_wildcards = false;
> + nir_deref_instr **entry_p = _dst_path.path[1];
> + nir_deref_instr **src_p = _path.path[1];
> + while (*entry_p && *src_p) {
> + nir_deref_instr *entry_tail = *entry_p++;
> + nir_deref_instr *src_tail = *src_p++;
> +
> + if (src_tail->deref_type == nir_deref_type_array &&
> + entry_tail->deref_type == nir_deref_type_array_wildcard)
New: Source IS wildcard, dest is wildcard. I think you want != on the
source condition to match the old behavior.
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v4 106/129] nir: Rework opt_copy_prop_vars to use deref instructions
---
src/compiler/nir/nir_opt_copy_prop_vars.c | 316 ++
1 file changed, 146 insertions(+), 170 deletions(-)
diff --git a/src/compiler/nir/nir_opt_copy_prop_vars.c
b/src/compiler/nir/nir_opt_copy_prop_vars.c
index bf3b793..f96bcb9 100644
--- a/src/compiler/nir/nir_opt_copy_prop_vars.c
+++ b/src/compiler/nir/nir_opt_copy_prop_vars.c
@@ -23,6 +23,7 @@
#include "nir.h"
#include "nir_builder.h"
+#include "nir_deref.h"
#include "util/bitscan.h"
@@ -40,7 +41,7 @@
* 2) Dead code elimination of store_var and copy_var intrinsics based on
* killed destination values.
*
- * 3) Removal of redundant load_var intrinsics. We can't trust regular CSE
+ * 3) Removal of redundant load_deref intrinsics. We can't trust regular CSE
* to do this because it isn't aware of variable writes that may alias the
* value and make the former load invalid.
*
@@ -56,7 +57,7 @@ struct value {
bool is_ssa;
union {
nir_ssa_def *ssa[4];
- nir_deref_var *deref;
+ nir_deref_instr *deref;
};
};
@@ -68,7 +69,7 @@ struct copy_entry {
unsigned comps_may_be_read;
struct value src;
- nir_deref_var *dst;
+ nir_deref_instr *dst;
};
struct copy_prop_var_state {
@@ -88,7 +89,7 @@ struct copy_prop_var_state {
static struct copy_entry *
copy_entry_create(struct copy_prop_var_state *state,
- nir_deref_var *dst_deref)
+ nir_deref_instr *dst_deref)
{
struct copy_entry *entry;
if (!list_empty(>copy_free_list)) {
@@ -127,9 +128,10 @@ enum deref_compare_result {
* ever needs it.
*/
static enum deref_compare_result
-compare_derefs(nir_deref_var *a, nir_deref_var *b)
+compare_deref_paths(nir_deref_path *a_path,
+nir_deref_path *b_path)
{
- if (a->var != b->var)
+ if (a_path->path[0]->var != b_path->path[0]->var)
return 0;
/* Start off assuming they fully compare. We ignore equality for now. In
@@ -139,62 +141,54 @@ compare_derefs(nir_deref_var *a, nir_deref_var *b)
derefs_a_contains_b_bit |
derefs_b_contains_a_bit;
- nir_deref *a_tail = >deref;
- nir_deref *b_tail = >deref;
- while (a_tail->child && b_tail->child) {
- a_tail = a_tail->child;
- b_tail = b_tail->child;
+ nir_deref_instr **a_p = _path->path[1];
+ nir_deref_instr **b_p = _path->path[1];
+ while (*a_p != NULL && *b_p != NULL) {
+ nir_deref_instr *a_tail = *(a_p++);
+ nir_deref_instr *b_tail = *(b_p++);
- assert(a_tail->deref_type == b_tail->deref_type);
switch (a_tail->deref_type) {
- case nir_deref_type_array: {
- nir_deref_array *a_arr = nir_deref_as_array(a_tail);
- nir_deref_array *b_arr = nir_deref_as_array(b_tail);
+ case nir_deref_type_array:
+ case nir_deref_type_array_wildcard: {
+ assert(b_tail->deref_type == nir_deref_type_array ||
+b_tail->deref_type == nir_deref_type_array_wildcard);
- if (a_arr->deref_array_type == nir_deref_array_type_wildcard) {
-if (b_arr->deref_array_type != nir_deref_array_type_wildcard)
+ if (a_tail->deref_type == nir_deref_type_array_wildcard) {
+if (b_tail->deref_type != nir_deref_type_array_wildcard)
result &= ~derefs_b_contains_a_bit;
- } else if (b_arr->deref_array_type == nir_deref_array_type_wildcard) {
-if (a_arr->deref_array_type != nir_deref_array_type_wildcard)
+ } else if (b_tail->deref_type == nir_deref_type_array_wildcard) {
+if (a_tail->deref_type != nir_deref_type_array_wildcard)
result &= ~derefs_a_contains_b_bit;
- } else if (a_arr->deref_array_type == nir_deref_array_type_direct &&
-b_arr->deref_array_type == nir_deref_array_type_direct) {
-/* If they're both direct and have different offsets, they
- * don't even alias much less anything else.
- */
-if (a_arr->base_offset != b_arr->base_offset)
- return 0;
- } else if (a_arr->deref_array_type == nir_deref_array_type_indirect &&
-b_arr->deref_array_type == nir_deref_array_type_indirect) {
-assert(a_arr->indirect.is_ssa && b_arr->indirect.is_ssa);
-if (a_arr->indirect.ssa == b_arr->indirect.ssa) {
- /* If they're different constant offsets from the same indirect
-* then they don't alias at all.
+ } else {
+assert(a_tail->deref_type == nir_deref_type_array &&
+ b_tail->deref_type == nir_deref_type_array);
+assert(a_tail->arr.index.is_ssa && b_tail->arr.index.is_ssa);
+
+nir_const_value *a_index_const =
+ nir_src_as_const_value(a_tail->arr.index);
+nir_const_value *b_index_const =
+ nir_src_as_const_value(b_tail->arr.index);
+
