On Thu, Jun 25, 2015 at 1:36 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
> On Thu, Jun 25, 2015 at 12:29 PM, Connor Abbott <cwabbo...@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 <cwabbo...@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.ekstr...@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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
