[Mesa-dev] [PATCH 1/4] nir/from_ssa: add a flag to not convert everything to SSA

Connor Abbott cwabbott0 at gmail.com
Thu Jun 25 15:23:38 PDT 2015


On Thu, Jun 25, 2015 at 1:36 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Thu, Jun 25, 2015 at 12:29 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>> We already don't convert constants out of SSA, and in our backend we'd
>> like to have only one way of saying something is still in SSA.
>>
>> The one tricky part about this is that we may now leave some undef
>> instructions around if they aren't part of a phi-web, so we have to be
>> more careful about deleting them.
>>
>> Signed-off-by: Connor Abbott <cwabbott0 at gmail.com>
>> ---
>>  src/gallium/drivers/vc4/vc4_program.c |  2 +-
>>  src/glsl/nir/nir.h                    |  7 ++++++-
>>  src/glsl/nir/nir_from_ssa.c           | 25 ++++++++++++++++++-------
>>  src/mesa/drivers/dri/i965/brw_nir.c   |  2 +-
>>  4 files changed, 26 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/gallium/drivers/vc4/vc4_program.c b/src/gallium/drivers/vc4/vc4_program.c
>> index 2061631..1a550e1 100644
>> --- a/src/gallium/drivers/vc4/vc4_program.c
>> +++ b/src/gallium/drivers/vc4/vc4_program.c
>> @@ -2102,7 +2102,7 @@ vc4_shader_ntq(struct vc4_context *vc4, enum qstage stage,
>>
>>          nir_remove_dead_variables(c->s);
>>
>> -        nir_convert_from_ssa(c->s);
>> +        nir_convert_from_ssa(c->s, true);
>>
>>          if (vc4_debug & VC4_DEBUG_SHADERDB) {
>>                  fprintf(stderr, "SHADER-DB: %s prog %d/%d: %d NIR instructions\n",
>> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
>> index 697d37e..2116f60 100644
>> --- a/src/glsl/nir/nir.h
>> +++ b/src/glsl/nir/nir.h
>> @@ -1676,7 +1676,12 @@ bool nir_ssa_defs_interfere(nir_ssa_def *a, nir_ssa_def *b);
>>
>>  void nir_convert_to_ssa_impl(nir_function_impl *impl);
>>  void nir_convert_to_ssa(nir_shader *shader);
>> -void nir_convert_from_ssa(nir_shader *shader);
>> +
>> +/* If convert_everything is true, convert all values (even those not involved
>> + * in a phi node) to registers. If false, only convert SSA values involved in
>> + * phi nodes to registers.
>> + */
>> +void nir_convert_from_ssa(nir_shader *shader, bool convert_everything);
>
> I don't think "convert everything" is really what we want to call it.
> A better idea might be to flip the bool and call it phi_webs_only.
>
> With that changed,
> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

Yeah, that sounds like a better name...

>
>>
>>  bool nir_opt_algebraic(nir_shader *shader);
>>  bool nir_opt_algebraic_late(nir_shader *shader);
>> diff --git a/src/glsl/nir/nir_from_ssa.c b/src/glsl/nir/nir_from_ssa.c
>> index 67733e6..966c2fe 100644
>> --- a/src/glsl/nir/nir_from_ssa.c
>> +++ b/src/glsl/nir/nir_from_ssa.c
>> @@ -37,6 +37,7 @@
>>  struct from_ssa_state {
>>     void *mem_ctx;
>>     void *dead_ctx;
>> +   bool convert_everything;
>>     struct hash_table *merge_node_table;
>>     nir_instr *instr;
>>     nir_function_impl *impl;
>> @@ -482,6 +483,9 @@ rewrite_ssa_def(nir_ssa_def *def, void *void_state)
>>
>>        reg = node->set->reg;
>>     } else {
>> +      if (!state->convert_everything)
>> +         return true;
>> +
>>        /* We leave load_const SSA values alone.  They act as immediates to
>>         * the backend.  If it got coalesced into a phi, that's ok.
>>         */
>> @@ -505,8 +509,15 @@ rewrite_ssa_def(nir_ssa_def *def, void *void_state)
>>     nir_ssa_def_rewrite_uses(def, nir_src_for_reg(reg), state->mem_ctx);
>>     assert(list_empty(&def->uses) && list_empty(&def->if_uses));
>>
>> -   if (def->parent_instr->type == nir_instr_type_ssa_undef)
>> +   if (def->parent_instr->type == nir_instr_type_ssa_undef) {
>> +      /* If it's an ssa_undef instruction, remove it since we know we just got
>> +       * rid of all its uses.
>> +       */
>> +      nir_instr *parent_instr = def->parent_instr;
>> +      nir_instr_remove(parent_instr);
>> +      ralloc_steal(state->dead_ctx, parent_instr);
>>        return true;
>> +   }
>>
>>     assert(def->parent_instr->type != nir_instr_type_load_const);
>>
>> @@ -523,7 +534,7 @@ rewrite_ssa_def(nir_ssa_def *def, void *void_state)
>>  }
>>
>>  /* Resolves ssa definitions to registers.  While we're at it, we also
>> - * remove phi nodes and ssa_undef instructions
>> + * remove phi nodes.
>>   */
>>  static bool
>>  resolve_registers_block(nir_block *block, void *void_state)
>> @@ -534,8 +545,7 @@ resolve_registers_block(nir_block *block, void *void_state)
>>        state->instr = instr;
>>        nir_foreach_ssa_def(instr, rewrite_ssa_def, state);
>>
>> -      if (instr->type == nir_instr_type_ssa_undef ||
>> -          instr->type == nir_instr_type_phi) {
>> +      if (instr->type == nir_instr_type_phi) {
>>           nir_instr_remove(instr);
>>           ralloc_steal(state->dead_ctx, instr);
>>        }
>> @@ -765,13 +775,14 @@ resolve_parallel_copies_block(nir_block *block, void *void_state)
>>  }
>>
>>  static void
>> -nir_convert_from_ssa_impl(nir_function_impl *impl)
>> +nir_convert_from_ssa_impl(nir_function_impl *impl, bool convert_everything)
>>  {
>>     struct from_ssa_state state;
>>
>>     state.mem_ctx = ralloc_parent(impl);
>>     state.dead_ctx = ralloc_context(NULL);
>>     state.impl = impl;
>> +   state.convert_everything = convert_everything;
>>     state.merge_node_table = _mesa_hash_table_create(NULL, _mesa_hash_pointer,
>>                                                      _mesa_key_pointer_equal);
>>
>> @@ -801,10 +812,10 @@ nir_convert_from_ssa_impl(nir_function_impl *impl)
>>  }
>>
>>  void
>> -nir_convert_from_ssa(nir_shader *shader)
>> +nir_convert_from_ssa(nir_shader *shader, bool convert_everything)
>>  {
>>     nir_foreach_overload(shader, overload) {
>>        if (overload->impl)
>> -         nir_convert_from_ssa_impl(overload->impl);
>> +         nir_convert_from_ssa_impl(overload->impl, convert_everything);
>>     }
>>  }
>> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c
>> index dffb8ab..3e154c1 100644
>> --- a/src/mesa/drivers/dri/i965/brw_nir.c
>> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
>> @@ -156,7 +156,7 @@ brw_create_nir(struct brw_context *brw,
>>        nir_print_shader(nir, stderr);
>>     }
>>
>> -   nir_convert_from_ssa(nir);
>> +   nir_convert_from_ssa(nir, true);
>>     nir_validate_shader(nir);
>>
>>     /* This is the last pass we run before we start emitting stuff.  It
>> --
>> 2.4.3
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list