[Mesa-dev] [PATCH 18/18] nir/spirv: Rework function argument setup

[Connor Abbott]

On Fri, Jun 30, 2017 at 4:25 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On June 30, 2017 3:28:51 PM Connor Abbott <cwabbott0 at gmail.com> wrote:
>
>> Now, that we're handling the pointers explicitly, can't we delete the
>> code in vtn_ssa_value()
>> to implicitly load pointers? I'd expect to see that in this patch.
>
>
> Almost.  It's still used to implement some glsl.450 functions which take a
> variable rather than an SSA vale as an input.  I'm happy to fix those up or
> at least to move the hack to glsl_450 but it'll have to be a separate patch.

Ok... at least you should change the comment there that says "This is
needed for function parameters" though. And making those cases aware
of pointers vs. SSA would be nice. It would probably make the code
less confusing too, since it would match the spec.

>
>
>> On Thu, Jun 29, 2017 at 10:33 AM, Jason Ekstrand <jason at jlekstrand.net>
>> wrote:
>>>
>>> Now that we have proper pointer types, we can be more sensible about the
>>> way we set up function arguments and deal with the two cases of pointer
>>> vs. SSA parameters distinctly.
>>> ---
>>>  src/compiler/spirv/vtn_cfg.c | 62
>>> +++++++++++++++++++++++++-------------------
>>>  1 file changed, 35 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c
>>> index 83e77e2..f53572b 100644
>>> --- a/src/compiler/spirv/vtn_cfg.c
>>> +++ b/src/compiler/spirv/vtn_cfg.c
>>> @@ -65,6 +65,7 @@ vtn_cfg_handle_prepass_instruction(struct vtn_builder
>>> *b, SpvOp opcode,
>>>        func->return_type = func_type->return_type->type;
>>>
>>>        b->func->impl = nir_function_impl_create(func);
>>> +      b->nb.cursor = nir_before_cf_list(&b->func->impl->body);
>>>
>>>        b->func_param_idx = 0;
>>>        break;
>>> @@ -77,43 +78,50 @@ vtn_cfg_handle_prepass_instruction(struct vtn_builder
>>> *b, SpvOp opcode,
>>>
>>>     case SpvOpFunctionParameter: {
>>>        struct vtn_type *type = vtn_value(b, w[1],
>>> vtn_value_type_type)->type;
>>> -      if (type->base_type == vtn_base_type_pointer) {
>>> -         type = type->deref;
>>> -         assert(type->base_type != vtn_base_type_pointer);
>>> -      }
>>>
>>>        assert(b->func_param_idx < b->func->impl->num_params);
>>>        nir_variable *param = b->func->impl->params[b->func_param_idx++];
>>>
>>> -      assert(type->type == param->type);
>>> +      if (type->base_type == vtn_base_type_pointer) {
>>> +         struct vtn_variable *vtn_var = rzalloc(b, struct vtn_variable);
>>> +         vtn_var->type = type->deref;
>>> +         vtn_var->var = param;
>>> +
>>> +         assert(vtn_var->type->type == param->type);
>>> +
>>> +         struct vtn_type *without_array = vtn_var->type;
>>> +         while(glsl_type_is_array(without_array->type))
>>> +            without_array = without_array->array_element;
>>> +
>>> +         if (glsl_type_is_image(without_array->type)) {
>>> +            vtn_var->mode = vtn_variable_mode_image;
>>> +            param->interface_type = without_array->type;
>>> +         } else if (glsl_type_is_sampler(without_array->type)) {
>>> +            vtn_var->mode = vtn_variable_mode_sampler;
>>> +            param->interface_type = without_array->type;
>>> +         } else {
>>> +            vtn_var->mode = vtn_variable_mode_param;
>>> +         }
>>>
>>> -      struct vtn_variable *vtn_var = rzalloc(b, struct vtn_variable);
>>> -      vtn_var->type = type;
>>> -      vtn_var->var = param;
>>> +         struct vtn_value *val =
>>> +            vtn_push_value(b, w[2], vtn_value_type_pointer);
>>>
>>> -      struct vtn_type *without_array = type;
>>> -      while(glsl_type_is_array(without_array->type))
>>> -         without_array = without_array->array_element;
>>> +         /* Name the parameter so it shows up nicely in NIR */
>>> +         param->name = ralloc_strdup(param, val->name);
>>>
>>> -      if (glsl_type_is_image(without_array->type)) {
>>> -         vtn_var->mode = vtn_variable_mode_image;
>>> -         param->interface_type = without_array->type;
>>> -      } else if (glsl_type_is_sampler(without_array->type)) {
>>> -         vtn_var->mode = vtn_variable_mode_sampler;
>>> -         param->interface_type = without_array->type;
>>> +         val->pointer = rzalloc(b, struct vtn_pointer);
>>> +         val->pointer->mode = vtn_var->mode;
>>> +         val->pointer->type = type;
>>> +         val->pointer->var = vtn_var;
>>>        } else {
>>> -         vtn_var->mode = vtn_variable_mode_param;
>>> -      }
>>> +         /* We're a regular SSA value. */
>>> +         struct vtn_value *val = vtn_push_value(b, w[2],
>>> vtn_value_type_ssa);
>>>
>>> -      struct vtn_value *val = vtn_push_value(b, w[2],
>>> vtn_value_type_pointer);
>>> +         /* Name the parameter so it shows up nicely in NIR */
>>> +         param->name = ralloc_strdup(param, val->name);
>>>
>>> -      /* Name the parameter so it shows up nicely in NIR */
>>> -      param->name = ralloc_strdup(param, val->name);
>>> -
>>> -      val->pointer = rzalloc(b, struct vtn_pointer);
>>> -      val->pointer->mode = vtn_var->mode;
>>> -      val->pointer->type = type;
>>> -      val->pointer->var = vtn_var;
>>> +         val->ssa = vtn_local_load(b, nir_deref_var_create(b, param));
>>> +      }
>>>        break;
>>>     }
>>>
>>> --
>>> 2.5.0.400.gff86faf
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>