<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jul 14, 2017 at 2:04 AM, Iago Toral <span dir="ltr"><<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, 2017-07-13 at 12:41 -0700, Jason Ekstrand wrote:<br>
> ---<br>
>  src/compiler/spirv/spirv_to_<wbr>nir.c  |  5 +++--<br>
>  src/compiler/spirv/vtn_cfg.c <wbr>      | 11 +++++------<br>
>  src/compiler/spirv/vtn_<wbr>private.h   |  9 +++++++++<br>
>  src/compiler/spirv/vtn_<wbr>variables.c |  5 +++--<br>
>  4 files changed, 20 insertions(+), 10 deletions(-)<br>
><br>
> diff --git a/src/compiler/spirv/spirv_to_<wbr>nir.c<br>
> b/src/compiler/spirv/spirv_to_<wbr>nir.c<br>
> index 7038bd9..6e35f83 100644<br>
> --- a/src/compiler/spirv/spirv_to_<wbr>nir.c<br>
> +++ b/src/compiler/spirv/spirv_to_<wbr>nir.c<br>
> @@ -1379,6 +1379,7 @@ static void<br>
>  vtn_handle_function_call(<wbr>struct vtn_builder *b, SpvOp opcode,<br>
>                           <wbr>const uint32_t *w, unsigned count)<br>
>  {<br>
> +   struct vtn_type *res_type = vtn_value(b, w[1],<br>
<br>
</span>The call to vtn_push_ssa() below takes this as parameter, but the<br>
implenentation of that function doesn't seem to use it for anything, so<br>
maybe we can drop it. In that case, we might want to add a MAYBE_UNSUED<br>
here since we only use this for an assert below besides that function<br>
call.<br><div><div class="h5"></div></div></blockquote><div><br></div><div>It doesn't yet but will in a patch or two.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
> vtn_value_type_type)->type;<br>
>     struct nir_function *callee =<br>
>        vtn_value(b, w[3], vtn_value_type_function)-><wbr>func->impl-<br>
> >function;<br>
>  <br>
> @@ -1402,6 +1403,7 @@ vtn_handle_function_call(<wbr>struct vtn_builder *b,<br>
> SpvOp opcode,<br>
>     }<br>
>  <br>
>     nir_variable *out_tmp = NULL;<br>
> +   assert(res_type->type == callee->return_type);<br>
>     if (!glsl_type_is_void(callee-><wbr>return_type)) {<br>
>        out_tmp = nir_local_variable_create(b-><wbr>impl, callee-<br>
> >return_type,<br>
>                               <wbr>             "out_tmp");<br>
> @@ -1413,8 +1415,7 @@ vtn_handle_function_call(<wbr>struct vtn_builder *b,<br>
> SpvOp opcode,<br>
>     if (glsl_type_is_void(callee-><wbr>return_type)) {<br>
>        vtn_push_value(b, w[2], vtn_value_type_undef);<br>
>     } else {<br>
> -      struct vtn_value *retval = vtn_push_value(b, w[2],<br>
> vtn_value_type_ssa);<br>
> -      retval->ssa = vtn_local_load(b, call->return_deref);<br>
> +      vtn_push_ssa(b, w[2], res_type, vtn_local_load(b, call-<br>
> >return_deref));<br>
>     }<br>
>  }<br>
>  <br>
> diff --git a/src/compiler/spirv/vtn_cfg.c<br>
> b/src/compiler/spirv/vtn_cfg.c<br>
> index df54b3c..c81a62d 100644<br>
> --- a/src/compiler/spirv/vtn_cfg.c<br>
> +++ b/src/compiler/spirv/vtn_cfg.c<br>
> @@ -112,12 +112,12 @@ vtn_cfg_handle_prepass_<wbr>instruction(struct<br>
> vtn_builder *b, SpvOp opcode,<br>
>           val->pointer = vtn_pointer_for_variable(b, vtn_var, type);<br>
>        } else {<br>
>           /* We're a regular SSA value. */<br>
> -         struct vtn_value *val = vtn_push_value(b, w[2],<br>
> vtn_value_type_ssa);<br>
> +         struct vtn_ssa_value *param_ssa =<br>
> +            vtn_local_load(b, nir_deref_var_create(b, param));<br>
> +         struct vtn_value *val = vtn_push_ssa(b, w[2], type,<br>
> param_ssa);<br>
>  <br>
>           /* Name the parameter so it shows up nicely in NIR */<br>
>           param->name = ralloc_strdup(param, val->name);<br>
> -<br>
> -         val->ssa = vtn_local_load(b, nir_deref_var_create(b,<br>
> param));<br>
>        }<br>
>        break;<br>
>     }<br>
> @@ -504,14 +504,13 @@ vtn_handle_phis_first_pass(<wbr>struct vtn_builder<br>
> *b, SpvOp opcode,<br>
>      * algorithm all over again.  It's easier if we just let<br>
>      * lower_vars_to_ssa do that for us instead of repeating it here.<br>
>      */<br>
> -   struct vtn_value *val = vtn_push_value(b, w[2],<br>
> vtn_value_type_ssa);<br>
> -<br>
>     struct vtn_type *type = vtn_value(b, w[1], vtn_value_type_type)-<br>
> >type;<br>
>     nir_variable *phi_var =<br>
>        nir_local_variable_<wbr>create(b->nb.impl, type->type, "phi");<br>
>     _mesa_hash_table_insert(b-<wbr>>phi_table, w, phi_var);<br>
>  <br>
> -   val->ssa = vtn_local_load(b, nir_deref_var_create(b, phi_var));<br>
> +   vtn_push_ssa(b, w[2], type,<br>
> +                vtn_local_<wbr>load(b, nir_deref_var_create(b,<br>
> phi_var)));<br>
>  <br>
>     return true;<br>
>  }<br>
> diff --git a/src/compiler/spirv/vtn_<wbr>private.h<br>
> b/src/compiler/spirv/vtn_<wbr>private.h<br>
> index 2f96c09..8d745bb 100644<br>
> --- a/src/compiler/spirv/vtn_<wbr>private.h<br>
> +++ b/src/compiler/spirv/vtn_<wbr>private.h<br>
> @@ -514,6 +514,15 @@ vtn_push_value(struct vtn_builder *b, uint32_t<br>
> value_id,<br>
>  }<br>
>  <br>
>  static inline struct vtn_value *<br>
> +vtn_push_ssa(struct vtn_builder *b, uint32_t value_id,<br>
> +             struct vtn_type *type, struct vtn_ssa_value *ssa)<br>
> +{<br>
<br>
</div></div>Drop the 'type' parameter? It doesn't seem to be used here.<span class=""><br></span></blockquote><div><br></div><div>It will be soon.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> +   struct vtn_value *val = vtn_push_value(b, value_id,<br>
> vtn_value_type_ssa);<br>
> +   val->ssa = ssa;<br>
> +   return val;<br>
> +}<br>
> +<br>
> +static inline struct vtn_value *<br>
>  vtn_untyped_value(struct vtn_builder *b, uint32_t value_id)<br>
>  {<br>
>     assert(value_id < b->value_id_bound);<br>
> diff --git a/src/compiler/spirv/vtn_<wbr>variables.c<br>
> b/src/compiler/spirv/vtn_<wbr>variables.c<br>
> index a9ba392..a9e2dbf 100644<br>
> --- a/src/compiler/spirv/vtn_<wbr>variables.c<br>
> +++ b/src/compiler/spirv/vtn_<wbr>variables.c<br>
> @@ -1774,6 +1774,8 @@ vtn_handle_variables(struct vtn_builder *b,<br>
> SpvOp opcode,<br>
>     }<br>
>  <br>
>     case SpvOpLoad: {<br>
> +      struct vtn_type *res_type =<br>
> +         vtn_value(b, w[1], vtn_value_type_type)->type;<br>
<br>
</span>I guess this is not needed if we drop the 'type' parameter from<br>
vtn_push_ssa().<br>
<div class="HOEnZb"><div class="h5"><br>
>        struct vtn_pointer *src =<br>
>           vtn_value(b, w[3], vtn_value_type_pointer)-><wbr>pointer;<br>
>  <br>
> @@ -1783,8 +1785,7 @@ vtn_handle_variables(struct vtn_builder *b,<br>
> SpvOp opcode,<br>
>           return;<br>
>        }<br>
>  <br>
> -      struct vtn_value *val = vtn_push_value(b, w[2],<br>
> vtn_value_type_ssa);<br>
> -      val->ssa = vtn_variable_load(b, src);<br>
> +      vtn_push_ssa(b, w[2], res_type, vtn_variable_load(b, src));<br>
>        break;<br>
>     }<br>
>  <br>
</div></div></blockquote></div><br></div></div>