[Mesa-dev] [PATCH 1/2] nir: Add a safety check that we don't remove dead I/O vars after lowering.

[Timothy Arceri]

On 18/10/17 07:52, Eric Anholt wrote:
> The pass only looks at var load/store intrinsics, not input load/store
> intrinsics, so assert that we don't see the other type.
> ---
> 
> I tripped over this limitation when trying to use the NIR linking
> helpers in vc4.
> 
>   src/compiler/nir/nir_remove_dead_variables.c | 20 ++++++++++++++++----
>   1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/src/compiler/nir/nir_remove_dead_variables.c b/src/compiler/nir/nir_remove_dead_variables.c
> index a1fe0de9c61f..7a676f2309c4 100644
> --- a/src/compiler/nir/nir_remove_dead_variables.c
> +++ b/src/compiler/nir/nir_remove_dead_variables.c
> @@ -28,7 +28,8 @@
>   #include "nir.h"
>   
>   static void
> -add_var_use_intrinsic(nir_intrinsic_instr *instr, struct set *live)
> +add_var_use_intrinsic(nir_intrinsic_instr *instr, struct set *live,
> +                      nir_variable_mode modes)
>   {
>      unsigned num_vars = nir_intrinsic_infos[instr->intrinsic].num_variables;
>   
> @@ -47,6 +48,16 @@ add_var_use_intrinsic(nir_intrinsic_instr *instr, struct set *live)
>         break;
>      }
>   
> +      /* This pass can't be used on I/O variables after they've been
> +       * lowered.
> +       */

Shouldn't this be aligned with the case rather since its not inside the 
case? Either way both patches:

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

> +   case nir_intrinsic_load_input:
> +      assert(!(modes & nir_var_shader_in));
> +      break;
> +   case nir_intrinsic_store_output:
> +      assert(!(modes & nir_var_shader_out));
> +      break;
> +
>      default:
>         for (unsigned i = 0; i < num_vars; i++) {
>            _mesa_set_add(live, instr->variables[i]->var);
> @@ -84,7 +95,7 @@ add_var_use_tex(nir_tex_instr *instr, struct set *live)
>   }
>   
>   static void
> -add_var_use_shader(nir_shader *shader, struct set *live)
> +add_var_use_shader(nir_shader *shader, struct set *live, nir_variable_mode modes)
>   {
>      nir_foreach_function(function, shader) {
>         if (function->impl) {
> @@ -92,7 +103,8 @@ add_var_use_shader(nir_shader *shader, struct set *live)
>               nir_foreach_instr(instr, block) {
>                  switch(instr->type) {
>                  case nir_instr_type_intrinsic:
> -                  add_var_use_intrinsic(nir_instr_as_intrinsic(instr), live);
> +                  add_var_use_intrinsic(nir_instr_as_intrinsic(instr), live,
> +                                        modes);
>                     break;
>   
>                  case nir_instr_type_call:
> @@ -162,7 +174,7 @@ nir_remove_dead_variables(nir_shader *shader, nir_variable_mode modes)
>      struct set *live =
>         _mesa_set_create(NULL, _mesa_hash_pointer, _mesa_key_pointer_equal);
>   
> -   add_var_use_shader(shader, live);
> +   add_var_use_shader(shader, live, modes);
>   
>      if (modes & nir_var_uniform)
>         progress = remove_dead_vars(&shader->uniforms, live) || progress;
>