[Mesa-dev] [PATCH 12/17] nir/lower-io: add support for lowering inputs

[Jason Ekstrand]

On Mon, May 9, 2016 at 12:34 PM, Rob Clark <robdclark at gmail.com> wrote:

> From: Rob Clark <robclark at freedesktop.org>
>
> Signed-off-by: Rob Clark <robclark at freedesktop.org>
> ---
>  src/compiler/nir/nir.h                         |  3 +-
>  src/compiler/nir/nir_lower_io_to_temporaries.c | 56
> +++++++++++++++++++++-----
>  src/mesa/drivers/dri/i965/brw_nir.c            |  4 +-
>  3 files changed, 52 insertions(+), 11 deletions(-)
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 5410f0b..c96eaf9 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2279,7 +2279,8 @@ bool nir_lower_indirect_derefs(nir_shader *shader,
> nir_variable_mode modes);
>
>  bool nir_lower_locals_to_regs(nir_shader *shader);
>
> -void nir_lower_io_to_temporaries(nir_shader *shader, nir_function
> *entrypoint);
> +void nir_lower_io_to_temporaries(nir_shader *shader, nir_function
> *entrypoint,
> +                                 bool outputs, bool inputs);
>
>  void nir_shader_gather_info(nir_shader *shader, nir_function_impl
> *entrypoint);
>
> diff --git a/src/compiler/nir/nir_lower_io_to_temporaries.c
> b/src/compiler/nir/nir_lower_io_to_temporaries.c
> index 9df2ba0..34e7477 100644
> --- a/src/compiler/nir/nir_lower_io_to_temporaries.c
> +++ b/src/compiler/nir/nir_lower_io_to_temporaries.c
> @@ -22,9 +22,12 @@
>   */
>
>  /*
> - * Implements a pass that lowers output variables to a temporary plus an
> - * output variable with a single copy at each exit point of the shader.
> - * This way the output variable is only ever written.
> + * Implements a pass that lowers output and/or input variables to a
> + * temporary plus an output variable with a single copy at each exit
> + * point of the shader and/or an input variable with a single copy
> + * at the entrance point of the shader.  This way the output variable
> + * is only ever written once and/or input is only read once, and there
> + * are no indirect outut/input accesses.
>   */
>
>  #include "nir.h"
> @@ -33,6 +36,7 @@ struct lower_io_state {
>     nir_shader *shader;
>     nir_function *entrypoint;
>     struct exec_list old_outputs;
> +   struct exec_list old_inputs;
>  };
>
>  static void
> @@ -49,7 +53,6 @@ emit_copies(nir_cursor cursor, nir_shader *shader,
> struct exec_list *new_vars,
>           nir_intrinsic_instr_create(shader, nir_intrinsic_copy_var);
>        copy->variables[0] = nir_deref_var_create(copy, newv);
>        copy->variables[1] = nir_deref_var_create(copy, temp);
> -
>

I don't think this was intended.


>        nir_instr_insert(cursor, &copy->instr);
>     }
>  }
> @@ -90,6 +93,20 @@ emit_output_copies_impl(struct lower_io_state *state,
> nir_function_impl *impl)
>     }
>  }
>
> +static void
> +emit_input_copies(nir_cursor cursor, struct lower_io_state *state)
> +{
> +   emit_copies(cursor, state->shader, &state->old_inputs,
> &state->shader->inputs);
> +}
> +
> +static void
> +emit_input_copies_impl(struct lower_io_state *state, nir_function_impl
> *impl)
> +{
> +   if (impl->function == state->entrypoint) {
> +      emit_input_copies(nir_before_block(nir_start_block(impl)), state);
> +   }
> +}
>

I was questioning the need for the wrapper before, but this is a bit silly
:-)

I think I'd like to see the extra layer of wrappers go if you're not too
attached to them.  and I had one other trivial change above.  Other than
that the lower_io_to_temporaries patches are

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


> +
>  static nir_variable *
>  create_shadow_temp(struct lower_io_state *state, nir_variable *var)
>  {
> @@ -105,8 +122,8 @@ create_shadow_temp(struct lower_io_state *state,
> nir_variable *var)
>     /* Reparent the constant initializer (if any) */
>     ralloc_steal(nvar, nvar->constant_initializer);
>
> -   /* Give the output a new name with @out-temp appended */
> -   const char *mode = "out";
> +   /* Give the original a new name with @<mode>-temp appended */
> +   const char *mode = (temp->data.mode == nir_var_shader_in) ? "in" :
> "out";
>     temp->name = ralloc_asprintf(var, "%s@%s-temp", mode, nvar->name);
>     temp->data.mode = nir_var_global;
>     temp->constant_initializer = NULL;
> @@ -115,7 +132,8 @@ create_shadow_temp(struct lower_io_state *state,
> nir_variable *var)
>  }
>
>  void
> -nir_lower_io_to_temporaries(nir_shader *shader, nir_function *entrypoint)
> +nir_lower_io_to_temporaries(nir_shader *shader, nir_function *entrypoint,
> +                            bool outputs, bool inputs)
>  {
>     struct lower_io_state state;
>
> @@ -124,7 +142,16 @@ nir_lower_io_to_temporaries(nir_shader *shader,
> nir_function *entrypoint)
>
>     state.shader = shader;
>     state.entrypoint = entrypoint;
> -   exec_list_move_nodes_to(&shader->outputs, &state.old_outputs);
> +
> +   if (inputs)
> +      exec_list_move_nodes_to(&shader->inputs, &state.old_inputs);
> +   else
> +      exec_list_make_empty(&state.old_inputs);
> +
> +   if (outputs)
> +      exec_list_move_nodes_to(&shader->outputs, &state.old_outputs);
> +   else
> +      exec_list_make_empty(&state.old_outputs);
>
>     /* Walk over all of the outputs turn each output into a temporary and
>      * make a new variable for the actual output.
> @@ -134,15 +161,26 @@ nir_lower_io_to_temporaries(nir_shader *shader,
> nir_function *entrypoint)
>        exec_list_push_tail(&shader->outputs, &output->node);
>     }
>
> +   /* and same for inputs: */
> +   nir_foreach_variable(var, &state.old_inputs) {
> +      nir_variable *input = create_shadow_temp(&state, var);
> +      exec_list_push_tail(&shader->inputs, &input->node);
> +   }
> +
>     nir_foreach_function(function, shader) {
>        if (function->impl == NULL)
>           continue;
>
> -      emit_output_copies_impl(&state, function->impl);
> +      if (inputs)
> +         emit_input_copies_impl(&state, function->impl);
> +
> +      if (outputs)
> +         emit_output_copies_impl(&state, function->impl);
>
>        nir_metadata_preserve(function->impl, nir_metadata_block_index |
>                                              nir_metadata_dominance);
>     }
>
> +   exec_list_append(&shader->globals, &state.old_inputs);
>     exec_list_append(&shader->globals, &state.old_outputs);
>  }
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c
> b/src/mesa/drivers/dri/i965/brw_nir.c
> index 2ed46ca..c5cc07e 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir.c
> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> @@ -553,7 +553,9 @@ brw_create_nir(struct brw_context *brw,
>     /* First, lower the GLSL IR or Mesa IR to NIR */
>     if (shader_prog) {
>        nir = glsl_to_nir(shader_prog, stage, options);
> -      OPT_V(nir_lower_io_to_temporaries, nir_shader_get_entrypoint(nir));
> +      OPT_V(nir_lower_io_to_temporaries,
> +            nir_shader_get_entrypoint(nir),
> +            true, false);
>     } else {
>        nir = prog_to_nir(prog, options);
>        OPT_V(nir_convert_to_ssa); /* turn registers into SSA */
> --
> 2.5.5
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160510/4456dd1a/attachment.html>