On Fri, Jun 30, 2017 at 4:31 PM, Connor Abbott <cwabbo...@gmail.com> wrote:
> On Fri, Jun 30, 2017 at 4:25 PM, Jason Ekstrand <ja...@jlekstrand.net>
> wrote:
> > On June 30, 2017 3:28:51 PM Connor Abbott <cwabbo...@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.
>
I've sent a 17.5/18 which fixes the problem so it can be deleted in this
one.
> >
> >
> >> On Thu, Jun 29, 2017 at 10:33 AM, Jason Ekstrand <ja...@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@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
> >
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
