<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jun 30, 2017 at 4:31 PM, Connor Abbott <<a href="mailto:cwabbott0@gmail.com" target="_blank">cwabbott0@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Jun 30, 2017 at 4:25 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > On June 30, 2017 3:28:51 PM Connor Abbott <<a href="mailto:cwabbott0@gmail.com">cwabbott0@gmail.com</a>> 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. <div class="HOEnZb"><div class="h5"></div></div></blockquote><div> </div><div>I've sent a 17.5/18 which fixes the problem so it can be deleted in this one. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > > >> On Thu, Jun 29, 2017 at 10:33 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> >> 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 >>> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> >>> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > > > </div></div></blockquote></div> </div></div>