On Oct 12, 2015 1:26 AM, "Iago Toral" <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote: > > On Fri, 2015-10-09 at 07:09 -0700, Jason Ekstrand wrote: > > --- > > src/glsl/nir/glsl_to_nir.cpp | 40 ++++--------------------- > > src/glsl/nir/nir.c | 66 ++++++++++++++++++++++++++++++++++++++++++ > > src/glsl/nir/nir.h | 20 +++++++++++++ > > src/mesa/program/prog_to_nir.c | 19 +++++------- > > 4 files changed, 99 insertions(+), 46 deletions(-) > > > > diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp > > index 874e361..5250080 100644 > > --- a/src/glsl/nir/glsl_to_nir.cpp > > +++ b/src/glsl/nir/glsl_to_nir.cpp > > @@ -389,35 +389,10 @@ nir_visitor::visit(ir_variable *ir) > > > > var->interface_type = ir->get_interface_type(); > > > > - switch (var->data.mode) { > > - case nir_var_local: > > - exec_list_push_tail(&impl->locals, &var->node); > > - break; > > - > > - case nir_var_global: > > - exec_list_push_tail(&shader->globals, &var->node); > > - break; > > - > > - case nir_var_shader_in: > > - exec_list_push_tail(&shader->inputs, &var->node); > > - break; > > - > > - case nir_var_shader_out: > > - exec_list_push_tail(&shader->outputs, &var->node); > > - break; > > - > > - case nir_var_uniform: > > - case nir_var_shader_storage: > > - exec_list_push_tail(&shader->uniforms, &var->node); > > - break; > > - > > - case nir_var_system_value: > > - exec_list_push_tail(&shader->system_values, &var->node); > > - break; > > - > > - default: > > - unreachable("not reached"); > > - } > > + if (var->data.mode == nir_var_local) > > + nir_function_impl_add_variable(impl, var); > > + else > > + nir_shader_add_variable(shader, var); > > > > _mesa_hash_table_insert(var_table, ir, var); > > this->var = var; > > @@ -2023,13 +1998,10 @@ nir_visitor::visit(ir_constant *ir) > > * constant initializer and return a dereference. > > */ > > > > - nir_variable *var = ralloc(this->shader, nir_variable); > > - var->name = ralloc_strdup(var, "const_temp"); > > - var->type = ir->type; > > - var->data.mode = nir_var_local; > > + nir_variable *var = > > + nir_local_variable_create(this->impl, ir->type, "const_temp"); > > var->data.read_only = true; > > var->constant_initializer = constant_copy(ir, var); > > - exec_list_push_tail(&this->impl->locals, &var->node); > > > > this->deref_head = nir_deref_var_create(this->shader, var); > > this->deref_tail = &this->deref_head->deref; > > diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c > > index e12da80..3645726 100644 > > --- a/src/glsl/nir/nir.c > > +++ b/src/glsl/nir/nir.c > > @@ -103,6 +103,72 @@ nir_reg_remove(nir_register *reg) > > exec_node_remove(&reg->node); > > } > > > > +void > > +nir_shader_add_variable(nir_shader *shader, nir_variable *var) > > +{ > > + switch (var->data.mode) { > > + case nir_var_local: > > + assert(!"nir_shader_add_variable cannot be used for local variables"); > > + break; > > + > > + case nir_var_global: > > + exec_list_push_tail(&shader->globals, &var->node); > > + break; > > + > > + case nir_var_shader_in: > > + exec_list_push_tail(&shader->inputs, &var->node); > > + break; > > + > > + case nir_var_shader_out: > > + exec_list_push_tail(&shader->outputs, &var->node); > > + break; > > + > > + case nir_var_uniform: > > + case nir_var_shader_storage: > > + exec_list_push_tail(&shader->uniforms, &var->node); > > + break; > > + > > + case nir_var_system_value: > > + exec_list_push_tail(&shader->system_values, &var->node); > > + break; > > + } > > +} > > + > > +nir_variable * > > +nir_variable_create(nir_shader *shader, nir_variable_mode mode, > > + const struct glsl_type *type, const char *name) > > +{ > > + nir_variable *var = rzalloc(shader, nir_variable); > > + var->name = ralloc_strdup(var, name); > > + var->type = type; > > + var->data.mode = mode; > > + > > + if ((mode == nir_var_shader_in && shader->stage != MESA_SHADER_VERTEX) || > > + (mode == nir_var_shader_out && shader->stage != MESA_SHADER_FRAGMENT)) > > + var->data.interpolation = INTERP_QUALIFIER_SMOOTH; > > + > > + if (mode == nir_var_shader_in || mode == nir_var_uniform) > > + var->data.read_only = true; > > + > > + nir_shader_add_variable(shader, var); > > + > > + return var; > > +} > > + > > +nir_variable * > > +nir_local_variable_create(nir_function_impl *impl, > > + const struct glsl_type *type, const char *name) > > +{ > > + nir_variable *var = rzalloc(impl->overload->function->shader, nir_variable); > > + var->name = ralloc_strdup(var, name); > > + var->type = type; > > + var->data.mode = nir_var_local; > > + > > + nir_function_impl_add_variable(impl, var); > > + > > + return var; > > +} > > + > > nir_function * > > nir_function_create(nir_shader *shader, const char *name) > > { > > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h > > index bde9f49..997e2bd 100644 > > --- a/src/glsl/nir/nir.h > > +++ b/src/glsl/nir/nir.h > > @@ -1559,6 +1559,26 @@ nir_register *nir_local_reg_create(nir_function_impl *impl); > > > > void nir_reg_remove(nir_register *reg); > > > > +/** Adds a variable to the appropreate list in nir_shader */ > > +void nir_shader_add_variable(nir_shader *shader, nir_variable *var); > > + > > +static inline void > > +nir_function_impl_add_variable(nir_function_impl *impl, nir_variable *var) > > +{ > > + assert(var->data.mode == nir_var_local); > > + exec_list_push_tail(&impl->locals, &var->node); > > +} > > + > > +/** creates a variable, sets a few defaults, and adds it to the list */ > > +nir_variable *nir_variable_create(nir_shader *shader, > > + nir_variable_mode mode, > > + const struct glsl_type *type, > > + const char *name); > > +/** creates a local variable and adds it to the list */ > > +nir_variable *nir_local_variable_create(nir_function_impl *impl, > > + const struct glsl_type *type, > > + const char *name); > > + > > /** creates a function and adds it to the shader's list of functions */ > > nir_function *nir_function_create(nir_shader *shader, const char *name); > > > > diff --git a/src/mesa/program/prog_to_nir.c b/src/mesa/program/prog_to_nir.c > > index d9b1854..ea47f79 100644 > > --- a/src/mesa/program/prog_to_nir.c > > +++ b/src/mesa/program/prog_to_nir.c > > @@ -958,11 +958,10 @@ setup_registers_and_variables(struct ptn_compile *c) > > for (int i = 0; i < num_inputs; i++) { > > if (!(c->prog->InputsRead & BITFIELD64_BIT(i))) > > continue; > > - nir_variable *var = rzalloc(shader, nir_variable); > > - var->type = glsl_vec4_type(); > > - var->data.read_only = true; > > - var->data.mode = nir_var_shader_in; > > - var->name = ralloc_asprintf(var, "in_%d", i); > > + > > + nir_variable *var = > > + nir_variable_create(shader, nir_var_shader_in, glsl_vec4_type(), > > + ralloc_asprintf(var, "in_%d", i)); > > Isn't this leaking the name? We are going to strdup it again inside > nir_variable_create. Sort-of. First, passing var in as the parent before creating var is illegal. I've already fixed that locally by parenting it to the shader. Since it's parented to the shader it's not truly leaked but will get deleted with the shader. It can also get cleaned up even earlier if nir_sweep() is called on the shader. For what its worth, just ralloc_strdup'ing and dealing with it later is sort-of the standard in NIR for dealing with strings. It's the easiest way to dealing with strings of you don't know where they came from. > Other than that, > Reviewed-by: Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> > > > var->data.location = i; > > var->data.index = 0; > > > > @@ -992,12 +991,9 @@ setup_registers_and_variables(struct ptn_compile *c) > > nir_ssa_def *f001 = nir_vec4(b, &load_x->dest.ssa, nir_imm_float(b, 0.0), > > nir_imm_float(b, 0.0), nir_imm_float(b, 1.0)); > > > > - nir_variable *fullvar = rzalloc(shader, nir_variable); > > - fullvar->type = glsl_vec4_type(); > > - fullvar->data.mode = nir_var_local; > > - fullvar->name = "fogcoord_tmp"; > > - exec_list_push_tail(&b->impl->locals, &fullvar->node); > > - > > + nir_variable *fullvar = > > + nir_local_variable_create(b->impl, glsl_vec4_type(), > > + "fogcoord_tmp"); > > nir_intrinsic_instr *store = > > nir_intrinsic_instr_create(shader, nir_intrinsic_store_var); > > store->num_components = 4; > > @@ -1015,7 +1011,6 @@ setup_registers_and_variables(struct ptn_compile *c) > > } > > } > > > > - exec_list_push_tail(&shader->inputs, &var->node); > > c->input_vars[i] = var; > > } > > > >