Re: [Mesa-dev] [PATCH 2/2] nir: Add a global dead write var removal pass

2018-08-15 Thread Caio Marcelo de Oliveira Filho 
Disregard this patch. I'm sending a replacement for it.

For the record:

> The pass works by walking through the control flow nodes, and traverse
> the instructions keeping track of the write instructions whose
> destination were not overwritten by other instructions (called "live
> writes"). Reading from the destinations cause the writes to be marked
> as "used". If statements and loops are handled specially to take into
> account the different codepaths. The writes that are not "used" are
> removed.

(...)

> +   case nir_cf_node_loop: {
> +  nir_loop *loop = nir_cf_node_as_loop(cf_node);
> +
> +  /* For tracking used variables in a loop, there are three cases: (a) 
> the
> +   * body is not entered; (b) the body is executed once; (c) the body is
> +   * executed more than once.
> +   *
> +   * The case (c) is exemplified below:
> +   *
> +   * c = x
> +   * while condition
> +   * use(c)
> +   * c = y
> +   * c = z
> +   * use(c)
> +   *
> +   * All writes to c must be considered used.  This is achieved by
> +   * performing a second pass in the loop body, with the live write table
> +   * produced by a first pass on it.
> +   */
> +
> +  struct hash_table *loop_live_writes = 
> _mesa_hash_table_clone(live_writes, mem_ctx);
> +  for (int i = 0; i < 2; i++) {
> + foreach_list_typed_safe(nir_cf_node, cf_node, node, >body)
> +mark_used_writes_in_node(mem_ctx, loop_live_writes, cf_node);
> +  }

Doing two iterations here has a bad consequence: the number of times
we execute blocks will blows up, since if there's a loop inside the
loop, we'll execute each block in the inner loop 4 times, and so on.

I've tried other approaches to do this "following the CFG", but for
the case at hand (eliminate dead writes) all them ended up pointing
towards doing the analysis considering the block graph (preds/succs).
So that's what I've done in the new patch.


Thanks,
Caio



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] nir: Add a global dead write var removal pass

2018-07-27 Thread Caio Marcelo de Oliveira Filho 
> ---
>  src/compiler/Makefile.sources  |   1 +
>  src/compiler/nir/meson.build   |   1 +
>  src/compiler/nir/nir.h |   2 +
>  src/compiler/nir/nir_opt_copy_prop_vars.c  |  67 +---
>  src/compiler/nir/nir_opt_dead_write_vars.c | 379 +
>  src/intel/compiler/brw_nir.c   |   1 +

In v2 I'll split the patch into:

- add new pass
- N patches to use new pass in drivers
- remove code from copy_prop_vars

The code itself should be good to be reviewed.


Thanks,
Caio

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] nir: Add a global dead write var removal pass

2018-07-27 Thread Caio Marcelo de Oliveira Filho 
Replaces the existing dead write var removal that is done as part of
nir_opt_copy_prop_vars pass. The previous pass was per-block, so
it would not remove the dead write in a program like

int total = gl_VertexIndex * 10;
float r = gl_VertexIndex;

for (int i = 0; i < total; i++) {
arr[i] = i; // DEAD.
if ((i % 2) == 0) {
arr[i] = i * 3.0;
} else {
arr[i] = i * 5.0;
}
r = arr[i];
}

out_color = vec4(r,r,r,r);

The new pass will identify the first write to arr[i] as dead and
remove it.

The pass works by walking through the control flow nodes, and traverse
the instructions keeping track of the write instructions whose
destination were not overwritten by other instructions (called "live
writes"). Reading from the destinations cause the writes to be marked
as "used". If statements and loops are handled specially to take into
account the different codepaths. The writes that are not "used" are
removed.

The reason to keep this as a separate pass is to unclutter the copy
propagation code -- as later it will be changed to also do more than
the per-block propagation.
---
 src/compiler/Makefile.sources  |   1 +
 src/compiler/nir/meson.build   |   1 +
 src/compiler/nir/nir.h |   2 +
 src/compiler/nir/nir_opt_copy_prop_vars.c  |  67 +---
 src/compiler/nir/nir_opt_dead_write_vars.c | 379 +
 src/intel/compiler/brw_nir.c   |   1 +
 6 files changed, 387 insertions(+), 64 deletions(-)
 create mode 100644 src/compiler/nir/nir_opt_dead_write_vars.c

diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index 908508adffb..046a2f99bd6 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -273,6 +273,7 @@ NIR_FILES = \
nir/nir_opt_cse.c \
nir/nir_opt_dce.c \
nir/nir_opt_dead_cf.c \
+   nir/nir_opt_dead_write_vars.c \
nir/nir_opt_gcm.c \
nir/nir_opt_global_to_local.c \
nir/nir_opt_if.c \
diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
index a1bb19356ce..93fd542ce7e 100644
--- a/src/compiler/nir/meson.build
+++ b/src/compiler/nir/meson.build
@@ -158,6 +158,7 @@ files_libnir = files(
   'nir_opt_cse.c',
   'nir_opt_dce.c',
   'nir_opt_dead_cf.c',
+  'nir_opt_dead_write_vars.c',
   'nir_opt_gcm.c',
   'nir_opt_global_to_local.c',
   'nir_opt_if.c',
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 2ca92e8f34e..04a3b98853b 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2919,6 +2919,8 @@ bool nir_opt_dce(nir_shader *shader);
 
 bool nir_opt_dead_cf(nir_shader *shader);
 
+bool nir_opt_dead_write_vars(nir_shader *shader);
+
 bool nir_opt_gcm(nir_shader *shader, bool value_number);
 
 bool nir_opt_if(nir_shader *shader);
diff --git a/src/compiler/nir/nir_opt_copy_prop_vars.c 
b/src/compiler/nir/nir_opt_copy_prop_vars.c
index 9fecaf0eeec..b6a5b9c2bb4 100644
--- a/src/compiler/nir/nir_opt_copy_prop_vars.c
+++ b/src/compiler/nir/nir_opt_copy_prop_vars.c
@@ -38,10 +38,7 @@
  *  1) Copy-propagation on variables that have indirect access.  This includes
  * propagating from indirect stores into indirect loads.
  *
- *  2) Dead code elimination of store_var and copy_var intrinsics based on
- * killed destination values.
- *
- *  3) Removal of redundant load_deref intrinsics.  We can't trust regular CSE
+ *  2) 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.
  *
@@ -51,6 +48,8 @@
  * rapidly get out of hand.  Fortunately, for anything that is only ever
  * accessed directly, we get SSA based copy-propagation which is extremely
  * powerful so this isn't that great a loss.
+ *
+ * Removal of dead writes to variables is handled by another pass.
  */
 
 struct value {
@@ -66,7 +65,6 @@ struct copy_entry {
 
nir_instr *store_instr[4];
 
-   unsigned comps_may_be_read;
struct value src;
 
nir_deref_instr *dst;
@@ -114,44 +112,6 @@ copy_entry_remove(struct copy_prop_var_state *state, 
struct copy_entry *entry)
list_add(>link, >copy_free_list);
 }
 
-static void
-remove_dead_writes(struct copy_prop_var_state *state,
-   struct copy_entry *entry, unsigned write_mask)
-{
-   /* We're overwriting another entry.  Some of it's components may not
-* have been read yet and, if that's the case, we may be able to delete
-* some instructions but we have to be careful.
-*/
-   unsigned dead_comps = write_mask & ~entry->comps_may_be_read;
-
-   for (unsigned mask = dead_comps; mask;) {
-  unsigned i = u_bit_scan();
-
-  nir_instr *instr = entry->store_instr[i];
-
-  /* We may have already deleted it on a previous iteration */
-  if (!instr)
- continue;
-
-  /* See if this instr is used anywhere