Re: [Mesa-dev] [PATCH 10/11] nir: Take call instruction into account in copy_prop_vars

2018-10-10 Thread Jason Ekstrand
On Sat, Sep 15, 2018 at 12:45 AM Caio Marcelo de Oliveira Filho <
caio.olive...@intel.com> wrote:

> Calls are not used yet (functions are inlined), but since new code is
> already taking them into account, do it here too.  The convention here
> and in other places is that no writable memory is assumed to remain
> unchanged, as well as global variables.
>
> Also, explicitly state the modes affected (instead of using the
> reverse logic) in one of the apply_for_barrier_modes calls.
>
> Suggested (indirectly) by Jason.
> ---
>
> Jason suggested this for the other pass, so doing this here too.
>
>  src/compiler/nir/nir_opt_copy_prop_vars.c | 17 +++--
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_copy_prop_vars.c
> b/src/compiler/nir/nir_opt_copy_prop_vars.c
> index 5276aa176d8..f58abfbb69f 100644
> --- a/src/compiler/nir/nir_opt_copy_prop_vars.c
> +++ b/src/compiler/nir/nir_opt_copy_prop_vars.c
> @@ -404,6 +404,14 @@ copy_prop_vars_block(struct copy_prop_var_state
> *state,
>copy_entry_remove(state, iter);
>
> nir_foreach_instr_safe(instr, block) {
> +  if (instr->type == nir_instr_type_call) {
> + apply_barrier_for_modes(copies, nir_var_shader_out |
> + nir_var_global |
> + nir_var_shader_storage |
> + nir_var_shared);
>

As I commented on the dead writes, pass, I think locals should be included
here.  Other than that,

Reviewed-by: Jason Ekstrand 


> + continue;
> +  }
> +
>if (instr->type != nir_instr_type_intrinsic)
>   continue;
>
> @@ -411,12 +419,9 @@ copy_prop_vars_block(struct copy_prop_var_state
> *state,
>switch (intrin->intrinsic) {
>case nir_intrinsic_barrier:
>case nir_intrinsic_memory_barrier:
> - /* If we hit a barrier, we need to trash everything that may
> possibly
> -  * be accessible to another thread.  Locals, globals, and things
> of
> -  * the like are safe, however.
> -  */
> - apply_barrier_for_modes(state, ~(nir_var_local | nir_var_global |
> -  nir_var_shader_in |
> nir_var_uniform));
> + apply_barrier_for_modes(copies, nir_var_shader_out |
> + nir_var_shader_storage |
> + nir_var_shared);
>

Yeah, this is better.


>   break;
>
>case nir_intrinsic_emit_vertex:
> --
> 2.19.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 10/11] nir: Take call instruction into account in copy_prop_vars

2018-09-14 Thread Caio Marcelo de Oliveira Filho
Calls are not used yet (functions are inlined), but since new code is
already taking them into account, do it here too.  The convention here
and in other places is that no writable memory is assumed to remain
unchanged, as well as global variables.

Also, explicitly state the modes affected (instead of using the
reverse logic) in one of the apply_for_barrier_modes calls.

Suggested (indirectly) by Jason.
---

Jason suggested this for the other pass, so doing this here too.

 src/compiler/nir/nir_opt_copy_prop_vars.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/compiler/nir/nir_opt_copy_prop_vars.c 
b/src/compiler/nir/nir_opt_copy_prop_vars.c
index 5276aa176d8..f58abfbb69f 100644
--- a/src/compiler/nir/nir_opt_copy_prop_vars.c
+++ b/src/compiler/nir/nir_opt_copy_prop_vars.c
@@ -404,6 +404,14 @@ copy_prop_vars_block(struct copy_prop_var_state *state,
   copy_entry_remove(state, iter);
 
nir_foreach_instr_safe(instr, block) {
+  if (instr->type == nir_instr_type_call) {
+ apply_barrier_for_modes(copies, nir_var_shader_out |
+ nir_var_global |
+ nir_var_shader_storage |
+ nir_var_shared);
+ continue;
+  }
+
   if (instr->type != nir_instr_type_intrinsic)
  continue;
 
@@ -411,12 +419,9 @@ copy_prop_vars_block(struct copy_prop_var_state *state,
   switch (intrin->intrinsic) {
   case nir_intrinsic_barrier:
   case nir_intrinsic_memory_barrier:
- /* If we hit a barrier, we need to trash everything that may possibly
-  * be accessible to another thread.  Locals, globals, and things of
-  * the like are safe, however.
-  */
- apply_barrier_for_modes(state, ~(nir_var_local | nir_var_global |
-  nir_var_shader_in | 
nir_var_uniform));
+ apply_barrier_for_modes(copies, nir_var_shader_out |
+ nir_var_shader_storage |
+ nir_var_shared);
  break;
 
   case nir_intrinsic_emit_vertex:
-- 
2.19.0

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