[Mesa-dev] [PATCH 3/3] nir: Add helpers for creating variables and adding them to lists
Iago Toral
itoral at igalia.com
Mon Oct 12 23:02:58 PDT 2015
On Mon, 2015-10-12 at 07:47 -0700, Jason Ekstrand wrote:
>
> On Oct 12, 2015 1:26 AM, "Iago Toral" <itoral at igalia.com> 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(®->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.
I see, thanks for the clarification!
Iago
> > Other than that,
> > Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
> >
> > > 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;
> > > }
> > >
> >
> >
>
>
More information about the mesa-dev
mailing list