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>
>
> 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
