[Mesa-dev] [PATCH 5/8] nir/spirv: Add a helper for pushing SSA values

[Iago Toral]

On Thu, 2017-07-13 at 12:41 -0700, Jason Ekstrand wrote:
> ---
>  src/compiler/spirv/spirv_to_nir.c  |  5 +++--
>  src/compiler/spirv/vtn_cfg.c       | 11 +++++------
>  src/compiler/spirv/vtn_private.h   |  9 +++++++++
>  src/compiler/spirv/vtn_variables.c |  5 +++--
>  4 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> index 7038bd9..6e35f83 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -1379,6 +1379,7 @@ static void
>  vtn_handle_function_call(struct vtn_builder *b, SpvOp opcode,
>                           const uint32_t *w, unsigned count)
>  {
> +   struct vtn_type *res_type = vtn_value(b, w[1], 

The call to vtn_push_ssa() below takes this as parameter, but the
implenentation of that function doesn't seem to use it for anything, so
maybe we can drop it. In that case, we might want to add a MAYBE_UNSUED
here since we only use this for an assert below besides that function
call.

> vtn_value_type_type)->type;
>     struct nir_function *callee =
>        vtn_value(b, w[3], vtn_value_type_function)->func->impl-
> >function;
>  
> @@ -1402,6 +1403,7 @@ vtn_handle_function_call(struct vtn_builder *b,
> SpvOp opcode,
>     }
>  
>     nir_variable *out_tmp = NULL;
> +   assert(res_type->type == callee->return_type);
>     if (!glsl_type_is_void(callee->return_type)) {
>        out_tmp = nir_local_variable_create(b->impl, callee-
> >return_type,
>                                            "out_tmp");
> @@ -1413,8 +1415,7 @@ vtn_handle_function_call(struct vtn_builder *b,
> SpvOp opcode,
>     if (glsl_type_is_void(callee->return_type)) {
>        vtn_push_value(b, w[2], vtn_value_type_undef);
>     } else {
> -      struct vtn_value *retval = vtn_push_value(b, w[2],
> vtn_value_type_ssa);
> -      retval->ssa = vtn_local_load(b, call->return_deref);
> +      vtn_push_ssa(b, w[2], res_type, vtn_local_load(b, call-
> >return_deref));
>     }
>  }
>  
> diff --git a/src/compiler/spirv/vtn_cfg.c
> b/src/compiler/spirv/vtn_cfg.c
> index df54b3c..c81a62d 100644
> --- a/src/compiler/spirv/vtn_cfg.c
> +++ b/src/compiler/spirv/vtn_cfg.c
> @@ -112,12 +112,12 @@ vtn_cfg_handle_prepass_instruction(struct
> vtn_builder *b, SpvOp opcode,
>           val->pointer = vtn_pointer_for_variable(b, vtn_var, type);
>        } else {
>           /* We're a regular SSA value. */
> -         struct vtn_value *val = vtn_push_value(b, w[2],
> vtn_value_type_ssa);
> +         struct vtn_ssa_value *param_ssa =
> +            vtn_local_load(b, nir_deref_var_create(b, param));
> +         struct vtn_value *val = vtn_push_ssa(b, w[2], type,
> param_ssa);
>  
>           /* Name the parameter so it shows up nicely in NIR */
>           param->name = ralloc_strdup(param, val->name);
> -
> -         val->ssa = vtn_local_load(b, nir_deref_var_create(b,
> param));
>        }
>        break;
>     }
> @@ -504,14 +504,13 @@ vtn_handle_phis_first_pass(struct vtn_builder
> *b, SpvOp opcode,
>      * algorithm all over again.  It's easier if we just let
>      * lower_vars_to_ssa do that for us instead of repeating it here.
>      */
> -   struct vtn_value *val = vtn_push_value(b, w[2],
> vtn_value_type_ssa);
> -
>     struct vtn_type *type = vtn_value(b, w[1], vtn_value_type_type)-
> >type;
>     nir_variable *phi_var =
>        nir_local_variable_create(b->nb.impl, type->type, "phi");
>     _mesa_hash_table_insert(b->phi_table, w, phi_var);
>  
> -   val->ssa = vtn_local_load(b, nir_deref_var_create(b, phi_var));
> +   vtn_push_ssa(b, w[2], type,
> +                vtn_local_load(b, nir_deref_var_create(b,
> phi_var)));
>  
>     return true;
>  }
> diff --git a/src/compiler/spirv/vtn_private.h
> b/src/compiler/spirv/vtn_private.h
> index 2f96c09..8d745bb 100644
> --- a/src/compiler/spirv/vtn_private.h
> +++ b/src/compiler/spirv/vtn_private.h
> @@ -514,6 +514,15 @@ vtn_push_value(struct vtn_builder *b, uint32_t
> value_id,
>  }
>  
>  static inline struct vtn_value *
> +vtn_push_ssa(struct vtn_builder *b, uint32_t value_id,
> +             struct vtn_type *type, struct vtn_ssa_value *ssa)
> +{

Drop the 'type' parameter? It doesn't seem to be used here.

> +   struct vtn_value *val = vtn_push_value(b, value_id,
> vtn_value_type_ssa);
> +   val->ssa = ssa;
> +   return val;
> +}
> +
> +static inline struct vtn_value *
>  vtn_untyped_value(struct vtn_builder *b, uint32_t value_id)
>  {
>     assert(value_id < b->value_id_bound);
> diff --git a/src/compiler/spirv/vtn_variables.c
> b/src/compiler/spirv/vtn_variables.c
> index a9ba392..a9e2dbf 100644
> --- a/src/compiler/spirv/vtn_variables.c
> +++ b/src/compiler/spirv/vtn_variables.c
> @@ -1774,6 +1774,8 @@ vtn_handle_variables(struct vtn_builder *b,
> SpvOp opcode,
>     }
>  
>     case SpvOpLoad: {
> +      struct vtn_type *res_type =
> +         vtn_value(b, w[1], vtn_value_type_type)->type;

I guess this is not needed if we drop the 'type' parameter from
vtn_push_ssa().

>        struct vtn_pointer *src =
>           vtn_value(b, w[3], vtn_value_type_pointer)->pointer;
>  
> @@ -1783,8 +1785,7 @@ vtn_handle_variables(struct vtn_builder *b,
> SpvOp opcode,
>           return;
>        }
>  
> -      struct vtn_value *val = vtn_push_value(b, w[2],
> vtn_value_type_ssa);
> -      val->ssa = vtn_variable_load(b, src);
> +      vtn_push_ssa(b, w[2], res_type, vtn_variable_load(b, src));
>        break;
>     }
>