[Mesa-dev] [PATCH 2/5] nir: Make an easier helper for setting up SSA defs.

Jason Ekstrand jason at jlekstrand.net
Wed Jan 21 20:18:12 PST 2015


Yes, please.  I've made that mistake enough myself.

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

On Wed, Jan 21, 2015 at 5:25 PM, Eric Anholt <eric at anholt.net> wrote:

> Almost all instructions we nir_ssa_def_init() for are nir_dests, and you
> have to keep from forgetting to set is_ssa when you do.  Just provide the
> simpler helper, instead.
> ---
>  src/glsl/nir/glsl_to_nir.cpp            | 14 ++++----------
>  src/glsl/nir/nir.c                      |  8 ++++++++
>  src/glsl/nir/nir.h                      |  2 ++
>  src/glsl/nir/nir_from_ssa.c             | 10 ++++------
>  src/glsl/nir/nir_lower_atomics.c        | 11 ++++-------
>  src/glsl/nir/nir_lower_io.c             | 11 ++++-------
>  src/glsl/nir/nir_lower_locals_to_regs.c | 11 ++++-------
>  src/glsl/nir/nir_lower_system_values.c  |  5 ++---
>  src/glsl/nir/nir_lower_var_copies.c     |  3 +--
>  src/glsl/nir/nir_lower_vars_to_ssa.c    | 15 ++++++---------
>  src/glsl/nir/nir_opt_peephole_select.c  |  5 ++---
>  src/glsl/nir/nir_search.c               |  8 +++-----
>  src/glsl/nir/nir_to_ssa.c               |  9 ++-------
>  13 files changed, 46 insertions(+), 66 deletions(-)
>
> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
> index 6ac830e..80bc4ba 100644
> --- a/src/glsl/nir/glsl_to_nir.cpp
> +++ b/src/glsl/nir/glsl_to_nir.cpp
> @@ -618,8 +618,7 @@ nir_visitor::visit(ir_call *ir)
>           (ir_dereference *) ir->actual_parameters.get_head();
>        param->accept(this);
>        instr->variables[0] = this->deref_head;
> -      instr->dest.is_ssa = true;
> -      nir_ssa_def_init(&instr->instr, &instr->dest.ssa, 1, NULL);
> +      nir_ssa_dest_init(&instr->instr, &instr->dest, 1, NULL);
>
>        nir_instr_insert_after_cf_list(this->cf_node_list, &instr->instr);
>
> @@ -702,9 +701,7 @@ nir_visitor::visit(ir_assignment *ir)
>        nir_intrinsic_instr *load =
>           nir_intrinsic_instr_create(this->shader, nir_intrinsic_load_var);
>        load->num_components = ir->lhs->type->vector_elements;
> -      load->dest.is_ssa = true;
> -      nir_ssa_def_init(&load->instr, &load->dest.ssa,
> -                       num_components, NULL);
> +      nir_ssa_dest_init(&load->instr, &load->dest, num_components, NULL);
>        load->variables[0] = lhs_deref;
>        nir_instr_insert_after_cf_list(this->cf_node_list, &load->instr);
>
> @@ -717,9 +714,7 @@ nir_visitor::visit(ir_assignment *ir)
>           default: unreachable("Invalid number of components"); break;
>        }
>        nir_alu_instr *vec = nir_alu_instr_create(this->shader, vec_op);
> -      vec->dest.dest.is_ssa = true;
> -      nir_ssa_def_init(&vec->instr, &vec->dest.dest.ssa,
> -                       num_components, NULL);
> +      nir_ssa_dest_init(&vec->instr, &vec->dest.dest, num_components,
> NULL);
>        vec->dest.write_mask = (1 << num_components) - 1;
>
>        unsigned component = 0;
> @@ -805,8 +800,7 @@ nir_visitor::add_instr(nir_instr *instr, unsigned
> num_components)
>  {
>     nir_dest *dest = get_instr_dest(instr);
>
> -   dest->is_ssa = true;
> -   nir_ssa_def_init(instr, &dest->ssa, num_components, NULL);
> +   nir_ssa_dest_init(instr, dest, num_components, NULL);
>
>     nir_instr_insert_after_cf_list(this->cf_node_list, instr);
>     this->result = instr;
> diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
> index 89e21fd..16ad2da 100644
> --- a/src/glsl/nir/nir.c
> +++ b/src/glsl/nir/nir.c
> @@ -1791,6 +1791,14 @@ nir_ssa_def_init(nir_instr *instr, nir_ssa_def *def,
>     }
>  }
>
> +void
> +nir_ssa_dest_init(nir_instr *instr, nir_dest *dest,
> +                 unsigned num_components, const char *name)
> +{
> +   dest->is_ssa = true;
> +   nir_ssa_def_init(instr, &dest->ssa, num_components, name);
> +}
> +
>  struct ssa_def_rewrite_state {
>     void *mem_ctx;
>     nir_ssa_def *old;
> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> index 761f20a..8dc5222 100644
> --- a/src/glsl/nir/nir.h
> +++ b/src/glsl/nir/nir.h
> @@ -1457,6 +1457,8 @@ nir_const_value *nir_src_as_const_value(nir_src src);
>  bool nir_srcs_equal(nir_src src1, nir_src src2);
>  void nir_instr_rewrite_src(nir_instr *instr, nir_src *src, nir_src
> new_src);
>
> +void nir_ssa_dest_init(nir_instr *instr, nir_dest *dest,
> +                       unsigned num_components, const char *name);
>  void nir_ssa_def_init(nir_instr *instr, nir_ssa_def *def,
>                        unsigned num_components, const char *name);
>  void nir_ssa_def_rewrite_uses(nir_ssa_def *def, nir_src new_src, void
> *mem_ctx);
> diff --git a/src/glsl/nir/nir_from_ssa.c b/src/glsl/nir/nir_from_ssa.c
> index 9728b99..88212a2 100644
> --- a/src/glsl/nir/nir_from_ssa.c
> +++ b/src/glsl/nir/nir_from_ssa.c
> @@ -355,9 +355,8 @@ isolate_phi_nodes_block(nir_block *block, void
> *void_state)
>           entry->src = nir_src_copy(src->src, state->dead_ctx);
>           _mesa_set_add(src->src.ssa->uses, &pcopy->instr);
>
> -         entry->dest.is_ssa = true;
> -         nir_ssa_def_init(&pcopy->instr, &entry->dest.ssa,
> -                          phi->dest.ssa.num_components,
> src->src.ssa->name);
> +         nir_ssa_dest_init(&pcopy->instr, &entry->dest,
> +                           phi->dest.ssa.num_components,
> src->src.ssa->name);
>
>           struct set_entry *use_entry =
>              _mesa_set_search(src->src.ssa->uses, instr);
> @@ -379,9 +378,8 @@ isolate_phi_nodes_block(nir_block *block, void
> *void_state)
>                                                nir_parallel_copy_entry);
>        exec_list_push_tail(&block_pcopy->entries, &entry->node);
>
> -      entry->dest.is_ssa = true;
> -      nir_ssa_def_init(&block_pcopy->instr, &entry->dest.ssa,
> -                       phi->dest.ssa.num_components, phi->dest.ssa.name);
> +      nir_ssa_dest_init(&block_pcopy->instr, &entry->dest,
> +                        phi->dest.ssa.num_components, phi->dest.ssa.name
> );
>
>        nir_src entry_dest_src = {
>           .ssa = &entry->dest.ssa,
> diff --git a/src/glsl/nir/nir_lower_atomics.c
> b/src/glsl/nir/nir_lower_atomics.c
> index 874c534..a5c05ce 100644
> --- a/src/glsl/nir/nir_lower_atomics.c
> +++ b/src/glsl/nir/nir_lower_atomics.c
> @@ -86,8 +86,7 @@ lower_instr(nir_intrinsic_instr *instr,
> nir_function_impl *impl)
>           nir_instr_insert_before(&instr->instr,
> &atomic_counter_size->instr);
>
>           nir_alu_instr *mul = nir_alu_instr_create(mem_ctx, nir_op_imul);
> -         mul->dest.dest.is_ssa = true;
> -         nir_ssa_def_init(&mul->instr, &mul->dest.dest.ssa, 1, NULL);
> +         nir_ssa_dest_init(&mul->instr, &mul->dest.dest, 1, NULL);
>           mul->dest.write_mask = 0x1;
>           mul->src[0].src = nir_src_copy(deref_array->indirect, mem_ctx);
>           mul->src[1].src.is_ssa = true;
> @@ -95,8 +94,7 @@ lower_instr(nir_intrinsic_instr *instr,
> nir_function_impl *impl)
>           nir_instr_insert_before(&instr->instr, &mul->instr);
>
>           nir_alu_instr *add = nir_alu_instr_create(mem_ctx, nir_op_iadd);
> -         add->dest.dest.is_ssa = true;
> -         nir_ssa_def_init(&add->instr, &add->dest.dest.ssa, 1, NULL);
> +         nir_ssa_dest_init(&add->instr, &add->dest.dest, 1, NULL);
>           add->dest.write_mask = 0x1;
>           add->src[0].src.is_ssa = true;
>           add->src[0].src.ssa = &mul->dest.dest.ssa;
> @@ -112,9 +110,8 @@ lower_instr(nir_intrinsic_instr *instr,
> nir_function_impl *impl)
>     new_instr->src[0].ssa = offset_def;;
>
>     if (instr->dest.is_ssa) {
> -      new_instr->dest.is_ssa = true;
> -      nir_ssa_def_init(&new_instr->instr, &new_instr->dest.ssa,
> -                       instr->dest.ssa.num_components, NULL);
> +      nir_ssa_dest_init(&new_instr->instr, &new_instr->dest,
> +                        instr->dest.ssa.num_components, NULL);
>        nir_src new_dest_src = {
>           .is_ssa = true,
>           .ssa = &new_instr->dest.ssa,
> diff --git a/src/glsl/nir/nir_lower_io.c b/src/glsl/nir/nir_lower_io.c
> index af87c13..a36f2d7 100644
> --- a/src/glsl/nir/nir_lower_io.c
> +++ b/src/glsl/nir/nir_lower_io.c
> @@ -151,8 +151,7 @@ get_io_offset(nir_deref_var *deref, nir_instr *instr,
> nir_src *indirect,
>              mul->src[1].src = nir_src_copy(deref_array->indirect,
>                                             state->mem_ctx);
>              mul->dest.write_mask = 1;
> -            mul->dest.dest.is_ssa = true;
> -            nir_ssa_def_init(&mul->instr, &mul->dest.dest.ssa, 1, NULL);
> +            nir_ssa_dest_init(&mul->instr, &mul->dest.dest, 1, NULL);
>              nir_instr_insert_before(instr, &mul->instr);
>
>              if (found_indirect) {
> @@ -162,8 +161,7 @@ get_io_offset(nir_deref_var *deref, nir_instr *instr,
> nir_src *indirect,
>                 add->src[1].src.is_ssa = true;
>                 add->src[1].src.ssa = &mul->dest.dest.ssa;
>                 add->dest.write_mask = 1;
> -               add->dest.dest.is_ssa = true;
> -               nir_ssa_def_init(&add->instr, &add->dest.dest.ssa, 1,
> NULL);
> +               nir_ssa_dest_init(&add->instr, &add->dest.dest, 1, NULL);
>                 nir_instr_insert_before(instr, &add->instr);
>
>                 indirect->is_ssa = true;
> @@ -235,9 +233,8 @@ nir_lower_io_block(nir_block *block, void *void_state)
>              load->src[0] = indirect;
>
>           if (intrin->dest.is_ssa) {
> -            load->dest.is_ssa = true;
> -            nir_ssa_def_init(&load->instr, &load->dest.ssa,
> -                             intrin->num_components, NULL);
> +            nir_ssa_dest_init(&load->instr, &load->dest,
> +                              intrin->num_components, NULL);
>
>              nir_src new_src = {
>                 .is_ssa = true,
> diff --git a/src/glsl/nir/nir_lower_locals_to_regs.c
> b/src/glsl/nir/nir_lower_locals_to_regs.c
> index b187541..58a1dcb 100644
> --- a/src/glsl/nir/nir_lower_locals_to_regs.c
> +++ b/src/glsl/nir/nir_lower_locals_to_regs.c
> @@ -158,8 +158,7 @@ get_deref_reg_src(nir_deref_var *deref, nir_instr
> *instr,
>           mul->src[1].src.is_ssa = true;
>           mul->src[1].src.ssa = &load_const->def;
>           mul->dest.write_mask = 1;
> -         mul->dest.dest.is_ssa = true;
> -         nir_ssa_def_init(&mul->instr, &mul->dest.dest.ssa, 1, NULL);
> +         nir_ssa_dest_init(&mul->instr, &mul->dest.dest, 1, NULL);
>           nir_instr_insert_before(instr, &mul->instr);
>
>           src.reg.indirect->is_ssa = true;
> @@ -178,8 +177,7 @@ get_deref_reg_src(nir_deref_var *deref, nir_instr
> *instr,
>              add->src[1].src = nir_src_copy(deref_array->indirect,
>                                             state->mem_ctx);
>              add->dest.write_mask = 1;
> -            add->dest.dest.is_ssa = true;
> -            nir_ssa_def_init(&add->instr, &add->dest.dest.ssa, 1, NULL);
> +            nir_ssa_dest_init(&add->instr, &add->dest.dest, 1, NULL);
>              nir_instr_insert_before(instr, &add->instr);
>
>              src.reg.indirect->is_ssa = true;
> @@ -212,9 +210,8 @@ lower_locals_to_regs_block(nir_block *block, void
> *void_state)
>                                               &intrin->instr, state);
>           mov->dest.write_mask = (1 << intrin->num_components) - 1;
>           if (intrin->dest.is_ssa) {
> -            mov->dest.dest.is_ssa = true;
> -            nir_ssa_def_init(&mov->instr, &mov->dest.dest.ssa,
> -                             intrin->num_components, NULL);
> +            nir_ssa_dest_init(&mov->instr, &mov->dest.dest,
> +                              intrin->num_components, NULL);
>
>              nir_src new_src = {
>                 .is_ssa = true,
> diff --git a/src/glsl/nir/nir_lower_system_values.c
> b/src/glsl/nir/nir_lower_system_values.c
> index d1b4d26..93ac157 100644
> --- a/src/glsl/nir/nir_lower_system_values.c
> +++ b/src/glsl/nir/nir_lower_system_values.c
> @@ -71,9 +71,8 @@ convert_instr(nir_intrinsic_instr *instr)
>     nir_intrinsic_instr *new_instr = nir_intrinsic_instr_create(mem_ctx,
> op);
>
>     if (instr->dest.is_ssa) {
> -      new_instr->dest.is_ssa = true;
> -      nir_ssa_def_init(&new_instr->instr, &new_instr->dest.ssa,
> -                       instr->dest.ssa.num_components, NULL);
> +      nir_ssa_dest_init(&new_instr->instr, &new_instr->dest,
> +                        instr->dest.ssa.num_components, NULL);
>        nir_src new_dest_src = {
>           .is_ssa = true,
>           .ssa = &new_instr->dest.ssa,
> diff --git a/src/glsl/nir/nir_lower_var_copies.c
> b/src/glsl/nir/nir_lower_var_copies.c
> index a9d1a47..85ebb28 100644
> --- a/src/glsl/nir/nir_lower_var_copies.c
> +++ b/src/glsl/nir/nir_lower_var_copies.c
> @@ -155,8 +155,7 @@ emit_copy_load_store(nir_intrinsic_instr *copy_instr,
>           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->dest.is_ssa = true;
> -      nir_ssa_def_init(&load->instr, &load->dest.ssa, num_components,
> NULL);
> +      nir_ssa_dest_init(&load->instr, &load->dest, num_components, NULL);
>
>        nir_instr_insert_before(&copy_instr->instr, &load->instr);
>
> diff --git a/src/glsl/nir/nir_lower_vars_to_ssa.c
> b/src/glsl/nir/nir_lower_vars_to_ssa.c
> index 4df9bdd..089f0d5 100644
> --- a/src/glsl/nir/nir_lower_vars_to_ssa.c
> +++ b/src/glsl/nir/nir_lower_vars_to_ssa.c
> @@ -788,9 +788,8 @@ rename_variables_block(nir_block *block, struct
> lower_variables_state *state)
>              assert(intrin->dest.is_ssa);
>
>              mov->dest.write_mask = (1 << intrin->num_components) - 1;
> -            mov->dest.dest.is_ssa = true;
> -            nir_ssa_def_init(&mov->instr, &mov->dest.dest.ssa,
> -                             intrin->num_components, NULL);
> +            nir_ssa_dest_init(&mov->instr, &mov->dest.dest,
> +                              intrin->num_components, NULL);
>
>              nir_instr_insert_before(&intrin->instr, &mov->instr);
>              nir_instr_remove(&intrin->instr);
> @@ -832,9 +831,8 @@ rename_variables_block(nir_block *block, struct
> lower_variables_state *state)
>                 mov->src[0].swizzle[i] = 0;
>
>              mov->dest.write_mask = (1 << intrin->num_components) - 1;
> -            mov->dest.dest.is_ssa = true;
> -            nir_ssa_def_init(&mov->instr, &mov->dest.dest.ssa,
> -                             intrin->num_components, NULL);
> +            nir_ssa_dest_init(&mov->instr, &mov->dest.dest,
> +                              intrin->num_components, NULL);
>
>              nir_instr_insert_before(&intrin->instr, &mov->instr);
>
> @@ -968,9 +966,8 @@ insert_phi_nodes(struct lower_variables_state *state)
>
>              if (has_already[next->index] < iter_count) {
>                 nir_phi_instr *phi = nir_phi_instr_create(state->mem_ctx);
> -               phi->dest.is_ssa = true;
> -               nir_ssa_def_init(&phi->instr, &phi->dest.ssa,
> -                                glsl_get_vector_elements(node->type),
> NULL);
> +               nir_ssa_dest_init(&phi->instr, &phi->dest,
> +                                 glsl_get_vector_elements(node->type),
> NULL);
>                 nir_instr_insert_before_block(next, &phi->instr);
>
>                 _mesa_hash_table_insert(state->phi_table, phi, node);
> diff --git a/src/glsl/nir/nir_opt_peephole_select.c
> b/src/glsl/nir/nir_opt_peephole_select.c
> index 5d2f5d6..023fae5 100644
> --- a/src/glsl/nir/nir_opt_peephole_select.c
> +++ b/src/glsl/nir/nir_opt_peephole_select.c
> @@ -163,9 +163,8 @@ nir_opt_peephole_select_block(nir_block *block, void
> *void_state)
>           }
>        }
>
> -      sel->dest.dest.is_ssa = true;
> -      nir_ssa_def_init(&sel->instr, &sel->dest.dest.ssa,
> -                       phi->dest.ssa.num_components, phi->dest.ssa.name);
> +      nir_ssa_dest_init(&sel->instr, &sel->dest.dest,
> +                        phi->dest.ssa.num_components, phi->dest.ssa.name
> );
>        sel->dest.write_mask = (1 << phi->dest.ssa.num_components) - 1;
>
>        nir_src sel_dest_src = {
> diff --git a/src/glsl/nir/nir_search.c b/src/glsl/nir/nir_search.c
> index 35323f9..c84f5ac 100644
> --- a/src/glsl/nir/nir_search.c
> +++ b/src/glsl/nir/nir_search.c
> @@ -201,8 +201,7 @@ construct_value(const nir_search_value *value,
> nir_alu_type type,
>           num_components = nir_op_infos[expr->opcode].output_size;
>
>        nir_alu_instr *alu = nir_alu_instr_create(mem_ctx, expr->opcode);
> -      alu->dest.dest.is_ssa = true;
> -      nir_ssa_def_init(&alu->instr, &alu->dest.dest.ssa, num_components,
> NULL);
> +      nir_ssa_dest_init(&alu->instr, &alu->dest.dest, num_components,
> NULL);
>        alu->dest.write_mask = (1 << num_components) - 1;
>        alu->dest.saturate = false;
>
> @@ -305,9 +304,8 @@ nir_replace_instr(nir_alu_instr *instr, const
> nir_search_expression *search,
>      */
>     nir_alu_instr *mov = nir_alu_instr_create(mem_ctx, nir_op_imov);
>     mov->dest.write_mask = instr->dest.write_mask;
> -   mov->dest.dest.is_ssa = true;
> -   nir_ssa_def_init(&mov->instr, &mov->dest.dest.ssa,
> -                    instr->dest.dest.ssa.num_components, NULL);
> +   nir_ssa_dest_init(&mov->instr, &mov->dest.dest,
> +                     instr->dest.dest.ssa.num_components, NULL);
>
>     mov->src[0] = construct_value(replace,
> nir_op_infos[instr->op].output_type,
>                                   instr->dest.dest.ssa.num_components,
> &state,
> diff --git a/src/glsl/nir/nir_to_ssa.c b/src/glsl/nir/nir_to_ssa.c
> index b9b1cff..1eb81f7 100644
> --- a/src/glsl/nir/nir_to_ssa.c
> +++ b/src/glsl/nir/nir_to_ssa.c
> @@ -214,15 +214,12 @@ rewrite_def_forwards(nir_dest *dest, void *_state)
>     if (state->states[index].stack == NULL)
>        return true;
>
> -   dest->is_ssa = true;
> -
>     char *name = NULL;
>     if (dest->reg.reg->name)
>        name = ralloc_asprintf(state->mem_ctx, "%s_%u", dest->reg.reg->name,
>                               state->states[index].num_defs);
>
> -   nir_ssa_def_init(state->parent_instr, &dest->ssa,
> -                    reg->num_components, name);
> +   nir_ssa_dest_init(state->parent_instr, dest, reg->num_components,
> name);
>
>     /* push our SSA destination on the stack */
>     state->states[index].index++;
> @@ -270,9 +267,7 @@ rewrite_alu_instr_forward(nir_alu_instr *instr,
> rewrite_state *state)
>                                  reg->name, state->states[index].num_defs);
>
>        instr->dest.write_mask = (1 << num_components) - 1;
> -      instr->dest.dest.is_ssa = true;
> -      nir_ssa_def_init(&instr->instr, &instr->dest.dest.ssa,
> -                       num_components, name);
> +      nir_ssa_dest_init(&instr->instr, &instr->dest.dest, num_components,
> name);
>
>        if (nir_op_infos[instr->op].output_size == 0) {
>           /*
> --
> 2.1.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150121/c13a67f4/attachment-0001.html>


More information about the mesa-dev mailing list