[Mesa-dev] [PATCH 4/5] nir: Allocate dereferences out of their parent instruction or deref.

Kenneth Graunke kenneth at whitecape.org
Tue Apr 7 02:32:22 PDT 2015


Jason pointed out that variable dereferences in NIR are really part of
their parent instruction, and should have the same lifetime.

Unlike in GLSL IR, they're not used very often - just for intrinsic
variables, call parameters & return, and indirect samplers for
texturing.  Also, nir_deref_var is the top-level concept, and
nir_deref_array/nir_deref_record are child nodes.

This patch attempts to allocate nir_deref_vars out of their parent
instruction, and any sub-dereferences out of their parent deref.
It enforces these restrictions in the validator as well.

This means that freeing an instruction should free its associated
dereference chain as well.  The memory sweeper pass can also happily
ignore them.

Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
 src/glsl/nir/glsl_to_nir.cpp        | 47 ++++++++++++++++++++-----------------
 src/glsl/nir/nir.c                  |  6 ++---
 src/glsl/nir/nir_lower_var_copies.c |  8 +++----
 src/glsl/nir/nir_split_var_copies.c |  4 ++--
 src/glsl/nir/nir_validate.c         | 13 ++++++----
 src/mesa/program/prog_to_nir.c      |  9 ++++---
 6 files changed, 45 insertions(+), 42 deletions(-)

This is still a lot of churn, but surprisingly about even on LOC.
With the validator code in place, I suspect we can get this right
going forward without too much trouble.

diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
index 80c5b3a..f61a47a 100644
--- a/src/glsl/nir/glsl_to_nir.cpp
+++ b/src/glsl/nir/glsl_to_nir.cpp
@@ -88,6 +88,8 @@ private:
    exec_list *cf_node_list;
    nir_instr *result; /* result of the expression tree last visited */
 
+   nir_deref_var *make_deref(void *mem_ctx, ir_instruction *ir);
+
    /* the head of the dereference chain we're creating */
    nir_deref_var *deref_head;
    /* the tail of the dereference chain we're creating */
@@ -156,6 +158,14 @@ nir_visitor::~nir_visitor()
    _mesa_hash_table_destroy(this->overload_table, NULL);
 }
 
+nir_deref_var *
+nir_visitor::make_deref(void *mem_ctx, ir_instruction *ir)
+{
+   ir->accept(this);
+   ralloc_steal(mem_ctx, this->deref_head);
+   return this->deref_head;
+}
+
 static nir_constant *
 constant_copy(ir_constant *ir, void *mem_ctx)
 {
@@ -582,13 +592,11 @@ void
 nir_visitor::visit(ir_return *ir)
 {
    if (ir->value != NULL) {
-      ir->value->accept(this);
       nir_intrinsic_instr *copy =
          nir_intrinsic_instr_create(this->shader, nir_intrinsic_copy_var);
 
-      copy->variables[0] = nir_deref_var_create(this->shader,
-                                                this->impl->return_var);
-      copy->variables[1] = this->deref_head;
+      copy->variables[0] = nir_deref_var_create(copy, this->impl->return_var);
+      copy->variables[1] = make_deref(copy, ir->value);
    }
 
    nir_jump_instr *instr = nir_jump_instr_create(this->shader, nir_jump_return);
@@ -613,8 +621,7 @@ nir_visitor::visit(ir_call *ir)
       nir_intrinsic_instr *instr = nir_intrinsic_instr_create(shader, op);
       ir_dereference *param =
          (ir_dereference *) ir->actual_parameters.get_head();
-      param->accept(this);
-      instr->variables[0] = this->deref_head;
+      instr->variables[0] = make_deref(instr, param);
       nir_ssa_dest_init(&instr->instr, &instr->dest, 1, NULL);
 
       nir_instr_insert_after_cf_list(this->cf_node_list, &instr->instr);
@@ -623,8 +630,7 @@ nir_visitor::visit(ir_call *ir)
          nir_intrinsic_instr_create(shader, nir_intrinsic_store_var);
       store_instr->num_components = 1;
 
-      ir->return_deref->accept(this);
-      store_instr->variables[0] = this->deref_head;
+      store_instr->variables[0] = make_deref(store_instr, ir->return_deref);
       store_instr->src[0].is_ssa = true;
       store_instr->src[0].ssa = &instr->dest.ssa;
 
@@ -642,13 +648,11 @@ nir_visitor::visit(ir_call *ir)
 
    unsigned i = 0;
    foreach_in_list(ir_dereference, param, &ir->actual_parameters) {
-      param->accept(this);
-      instr->params[i] = this->deref_head;
+      instr->params[i] = make_deref(instr, param);
       i++;
    }
 
-   ir->return_deref->accept(this);
-   instr->return_deref = this->deref_head;
+   instr->return_deref = make_deref(instr, ir->return_deref);
    nir_instr_insert_after_cf_list(this->cf_node_list, &instr->instr);
 }
 
@@ -663,12 +667,8 @@ nir_visitor::visit(ir_assignment *ir)
       nir_intrinsic_instr *copy =
          nir_intrinsic_instr_create(this->shader, nir_intrinsic_copy_var);
 
-      ir->lhs->accept(this);
-      copy->variables[0] = this->deref_head;
-
-      ir->rhs->accept(this);
-      copy->variables[1] = this->deref_head;
-
+      copy->variables[0] = make_deref(copy, ir->lhs);
+      copy->variables[1] = make_deref(copy, ir->rhs);
 
       if (ir->condition) {
          nir_if *if_stmt = nir_if_create(this->shader);
@@ -700,6 +700,7 @@ nir_visitor::visit(ir_assignment *ir)
       load->num_components = ir->lhs->type->vector_elements;
       nir_ssa_dest_init(&load->instr, &load->dest, num_components, NULL);
       load->variables[0] = lhs_deref;
+      ralloc_steal(load, load->variables[0]);
       nir_instr_insert_after_cf_list(this->cf_node_list, &load->instr);
 
       nir_op vec_op;
@@ -741,7 +742,7 @@ nir_visitor::visit(ir_assignment *ir)
    nir_intrinsic_instr *store =
       nir_intrinsic_instr_create(this->shader, nir_intrinsic_store_var);
    store->num_components = ir->lhs->type->vector_elements;
-   nir_deref *store_deref = nir_copy_deref(this->shader, &lhs_deref->deref);
+   nir_deref *store_deref = nir_copy_deref(store, &lhs_deref->deref);
    store->variables[0] = nir_deref_as_var(store_deref);
    store->src[0] = src;
 
@@ -816,6 +817,7 @@ nir_visitor::evaluate_rvalue(ir_rvalue* ir)
          nir_intrinsic_instr_create(this->shader, nir_intrinsic_load_var);
       load_instr->num_components = ir->type->vector_elements;
       load_instr->variables[0] = this->deref_head;
+      ralloc_steal(load_instr, load_instr->variables[0]);
       add_instr(&load_instr->instr, ir->type->vector_elements);
    }
 
@@ -959,6 +961,7 @@ nir_visitor::visit(ir_expression *ir)
       nir_intrinsic_instr *intrin = nir_intrinsic_instr_create(shader, op);
       intrin->num_components = deref->type->vector_elements;
       intrin->variables[0] = this->deref_head;
+      ralloc_steal(intrin, intrin->variables[0]);
 
       if (intrin->intrinsic == nir_intrinsic_interp_var_at_offset ||
           intrin->intrinsic == nir_intrinsic_interp_var_at_sample)
@@ -1630,8 +1633,7 @@ nir_visitor::visit(ir_texture *ir)
       unreachable("not reached");
    }
 
-   ir->sampler->accept(this);
-   instr->sampler = this->deref_head;
+   instr->sampler = make_deref(instr, ir->sampler);
 
    unsigned src_number = 0;
 
@@ -1756,7 +1758,7 @@ nir_visitor::visit(ir_dereference_record *ir)
    int field_index = this->deref_tail->type->field_index(ir->field);
    assert(field_index >= 0);
 
-   nir_deref_struct *deref = nir_deref_struct_create(this->shader, field_index);
+   nir_deref_struct *deref = nir_deref_struct_create(this->deref_tail, field_index);
    deref->deref.type = ir->type;
    this->deref_tail->child = &deref->deref;
    this->deref_tail = &deref->deref;
@@ -1780,5 +1782,6 @@ nir_visitor::visit(ir_dereference_array *ir)
    ir->array->accept(this);
 
    this->deref_tail->child = &deref->deref;
+   ralloc_steal(this->deref_tail, deref);
    this->deref_tail = &deref->deref;
 }
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index 85ff0f4..1c6b603 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -543,7 +543,7 @@ copy_deref_var(void *mem_ctx, nir_deref_var *deref)
    nir_deref_var *ret = nir_deref_var_create(mem_ctx, deref->var);
    ret->deref.type = deref->deref.type;
    if (deref->deref.child)
-      ret->deref.child = nir_copy_deref(mem_ctx, deref->deref.child);
+      ret->deref.child = nir_copy_deref(ret, deref->deref.child);
    return ret;
 }
 
@@ -558,7 +558,7 @@ copy_deref_array(void *mem_ctx, nir_deref_array *deref)
    }
    ret->deref.type = deref->deref.type;
    if (deref->deref.child)
-      ret->deref.child = nir_copy_deref(mem_ctx, deref->deref.child);
+      ret->deref.child = nir_copy_deref(ret, deref->deref.child);
    return ret;
 }
 
@@ -568,7 +568,7 @@ copy_deref_struct(void *mem_ctx, nir_deref_struct *deref)
    nir_deref_struct *ret = nir_deref_struct_create(mem_ctx, deref->index);
    ret->deref.type = deref->deref.type;
    if (deref->deref.child)
-      ret->deref.child = nir_copy_deref(mem_ctx, deref->deref.child);
+      ret->deref.child = nir_copy_deref(ret, deref->deref.child);
    return ret;
 }
 
diff --git a/src/glsl/nir/nir_lower_var_copies.c b/src/glsl/nir/nir_lower_var_copies.c
index 85ebb28..58389a7 100644
--- a/src/glsl/nir/nir_lower_var_copies.c
+++ b/src/glsl/nir/nir_lower_var_copies.c
@@ -148,13 +148,10 @@ emit_copy_load_store(nir_intrinsic_instr *copy_instr,
 
       unsigned num_components = glsl_get_vector_elements(src_tail->type);
 
-      nir_deref *src_deref = nir_copy_deref(mem_ctx, &src_head->deref);
-      nir_deref *dest_deref = nir_copy_deref(mem_ctx, &dest_head->deref);
-
       nir_intrinsic_instr *load =
          nir_intrinsic_instr_create(mem_ctx, nir_intrinsic_load_var);
       load->num_components = num_components;
-      load->variables[0] = nir_deref_as_var(src_deref);
+      load->variables[0] = nir_deref_as_var(nir_copy_deref(load, &src_head->deref));
       nir_ssa_dest_init(&load->instr, &load->dest, num_components, NULL);
 
       nir_instr_insert_before(&copy_instr->instr, &load->instr);
@@ -162,7 +159,8 @@ emit_copy_load_store(nir_intrinsic_instr *copy_instr,
       nir_intrinsic_instr *store =
          nir_intrinsic_instr_create(mem_ctx, nir_intrinsic_store_var);
       store->num_components = num_components;
-      store->variables[0] = nir_deref_as_var(dest_deref);
+      store->variables[0] = nir_deref_as_var(nir_copy_deref(store, &dest_head->deref));
+
       store->src[0].is_ssa = true;
       store->src[0].ssa = &load->dest.ssa;
 
diff --git a/src/glsl/nir/nir_split_var_copies.c b/src/glsl/nir/nir_split_var_copies.c
index 4d663b5..fc72c07 100644
--- a/src/glsl/nir/nir_split_var_copies.c
+++ b/src/glsl/nir/nir_split_var_copies.c
@@ -188,8 +188,8 @@ split_var_copy_instr(nir_intrinsic_instr *old_copy,
           * belongs to the copy instruction and b) the deref chains may
           * have some of the same links due to the way we constructed them
           */
-         nir_deref *src = nir_copy_deref(state->mem_ctx, src_head);
-         nir_deref *dest = nir_copy_deref(state->mem_ctx, dest_head);
+         nir_deref *src = nir_copy_deref(new_copy, src_head);
+         nir_deref *dest = nir_copy_deref(new_copy, dest_head);
 
          new_copy->variables[0] = nir_deref_as_var(dest);
          new_copy->variables[1] = nir_deref_as_var(src);
diff --git a/src/glsl/nir/nir_validate.c b/src/glsl/nir/nir_validate.c
index e8c9d7b..a7aa798 100644
--- a/src/glsl/nir/nir_validate.c
+++ b/src/glsl/nir/nir_validate.c
@@ -295,6 +295,8 @@ validate_alu_instr(nir_alu_instr *instr, validate_state *state)
 static void
 validate_deref_chain(nir_deref *deref, validate_state *state)
 {
+   assert(deref->child == NULL || ralloc_parent(deref->child) == deref);
+
    nir_deref *parent = NULL;
    while (deref != NULL) {
       switch (deref->deref_type) {
@@ -336,9 +338,10 @@ validate_var_use(nir_variable *var, validate_state *state)
 }
 
 static void
-validate_deref_var(nir_deref_var *deref, validate_state *state)
+validate_deref_var(void *parent_mem_ctx, nir_deref_var *deref, validate_state *state)
 {
    assert(deref != NULL);
+   assert(ralloc_parent(deref) == parent_mem_ctx);
    assert(deref->deref.type == deref->var->type);
 
    validate_var_use(deref->var, state);
@@ -386,7 +389,7 @@ validate_intrinsic_instr(nir_intrinsic_instr *instr, validate_state *state)
 
    unsigned num_vars = nir_intrinsic_infos[instr->intrinsic].num_variables;
    for (unsigned i = 0; i < num_vars; i++) {
-      validate_deref_var(instr->variables[i], state);
+      validate_deref_var(instr, instr->variables[i], state);
    }
 
    switch (instr->intrinsic) {
@@ -423,7 +426,7 @@ validate_tex_instr(nir_tex_instr *instr, validate_state *state)
    }
 
    if (instr->sampler != NULL)
-      validate_deref_var(instr->sampler, state);
+      validate_deref_var(instr, instr->sampler, state);
 }
 
 static void
@@ -438,10 +441,10 @@ validate_call_instr(nir_call_instr *instr, validate_state *state)
 
    for (unsigned i = 0; i < instr->num_params; i++) {
       assert(instr->callee->params[i].type == instr->params[i]->deref.type);
-      validate_deref_var(instr->params[i], state);
+      validate_deref_var(instr, instr->params[i], state);
    }
 
-   validate_deref_var(instr->return_deref, state);
+   validate_deref_var(instr, instr->return_deref, state);
 }
 
 static void
diff --git a/src/mesa/program/prog_to_nir.c b/src/mesa/program/prog_to_nir.c
index 5f00a8b..b298d07 100644
--- a/src/mesa/program/prog_to_nir.c
+++ b/src/mesa/program/prog_to_nir.c
@@ -153,8 +153,7 @@ ptn_get_src(struct ptn_compile *c, const struct prog_src_register *prog_src)
       nir_intrinsic_instr *load =
          nir_intrinsic_instr_create(b->shader, nir_intrinsic_load_var);
       load->num_components = 4;
-      load->variables[0] =
-         nir_deref_var_create(b->shader, c->input_vars[prog_src->Index]);
+      load->variables[0] = nir_deref_var_create(load, c->input_vars[prog_src->Index]);
 
       nir_ssa_dest_init(&load->instr, &load->dest, 4, NULL);
       nir_instr_insert_after_cf_list(b->cf_node_list, &load->instr);
@@ -918,7 +917,7 @@ ptn_add_output_stores(struct ptn_compile *c)
          nir_intrinsic_instr_create(b->shader, nir_intrinsic_store_var);
       store->num_components = 4;
       store->variables[0] =
-         nir_deref_var_create(b->shader, c->output_vars[var->data.location]);
+         nir_deref_var_create(store, c->output_vars[var->data.location]);
       store->src[0].reg.reg = c->output_regs[var->data.location];
       nir_instr_insert_after_cf_list(c->build.cf_node_list, &store->instr);
    }
@@ -962,7 +961,7 @@ setup_registers_and_variables(struct ptn_compile *c)
             nir_intrinsic_instr *load_x =
                nir_intrinsic_instr_create(shader, nir_intrinsic_load_var);
             load_x->num_components = 1;
-            load_x->variables[0] = nir_deref_var_create(shader, var);
+            load_x->variables[0] = nir_deref_var_create(load_x, var);
             nir_ssa_dest_init(&load_x->instr, &load_x->dest, 1, NULL);
             nir_instr_insert_after_cf_list(b->cf_node_list, &load_x->instr);
 
@@ -978,7 +977,7 @@ setup_registers_and_variables(struct ptn_compile *c)
             nir_intrinsic_instr *store =
                nir_intrinsic_instr_create(shader, nir_intrinsic_store_var);
             store->num_components = 4;
-            store->variables[0] = nir_deref_var_create(shader, fullvar);
+            store->variables[0] = nir_deref_var_create(store, fullvar);
             store->src[0] = nir_src_for_ssa(f001);
             nir_instr_insert_after_cf_list(b->cf_node_list, &store->instr);
 
-- 
2.3.5



More information about the mesa-dev mailing list