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

[Jason Ekstrand]

On Sat, Sep 15, 2018 at 12:45 AM Caio Marcelo de Oliveira Filho <
caio.oliveira at 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 <jason at jlekstrand.net>


> +         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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181010/cd0fc929/attachment.html>