[Mesa-dev] [PATCH 075/133] i965/fs_nir: Handle SSA constants
Connor Abbott
cwabbott0 at gmail.com
Wed Dec 17 17:30:36 PST 2014
On Tue, Dec 16, 2014 at 1:05 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> ---
> src/glsl/nir/nir_from_ssa.c | 40 ++++++++++++++++++++-----
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 50 +++++++++++++++++++++-----------
> 2 files changed, 65 insertions(+), 25 deletions(-)
>
> diff --git a/src/glsl/nir/nir_from_ssa.c b/src/glsl/nir/nir_from_ssa.c
> index 62a54fe..9311bda 100644
It would be nice if you could squash all the following nir_from_ssa.c
changes into "nir: Add a better out-of-ssa pass", since nir_from_ssa.c
already has some stuff about special-casing SSA constants so this is
rather confusing.
> --- a/src/glsl/nir/nir_from_ssa.c
> +++ b/src/glsl/nir/nir_from_ssa.c
> @@ -469,6 +469,12 @@ get_register_for_ssa_def(nir_ssa_def *def, struct from_ssa_state *state)
> if (entry) {
> return (nir_register *)entry->data;
> } else {
> + /* We leave load_const SSA values alone. They act as immediates to
> + * the backend. If it got coalesced into a phi, that's ok.
> + */
> + if (def->parent_instr->type == nir_instr_type_load_const)
> + return NULL;
> +
> nir_register *reg = nir_local_reg_create(state->impl);
> reg->name = def->name;
> reg->num_components = def->num_components;
> @@ -486,12 +492,18 @@ rewrite_ssa_src(nir_src *src, void *void_state)
> struct from_ssa_state *state = void_state;
>
> if (src->is_ssa) {
> - /* We don't need to remove it from the uses set because that is going
> - * away. We just need to add it to the one for the register. */
> nir_register *reg = get_register_for_ssa_def(src->ssa, state);
> +
> + if (reg == NULL) {
> + assert(src->ssa->parent_instr->type == nir_instr_type_load_const);
> + return true;
> + }
> +
> memset(src, 0, sizeof *src);
> src->reg.reg = reg;
>
> + /* We don't need to remove it from the uses set because that is going
> + * away. We just need to add it to the one for the register. */
> _mesa_set_add(reg->uses, _mesa_hash_pointer(state->instr), state->instr);
> }
>
> @@ -504,10 +516,16 @@ rewrite_ssa_dest(nir_dest *dest, void *void_state)
> struct from_ssa_state *state = void_state;
>
> if (dest->is_ssa) {
> + nir_register *reg = get_register_for_ssa_def(&dest->ssa, state);
> +
> + if (reg == NULL) {
> + assert(dest->ssa.parent_instr->type == nir_instr_type_load_const);
> + return true;
> + }
> +
> _mesa_set_destroy(dest->ssa.uses, NULL);
> _mesa_set_destroy(dest->ssa.if_uses, NULL);
>
> - nir_register *reg = get_register_for_ssa_def(&dest->ssa, state);
> memset(dest, 0, sizeof *dest);
> dest->reg.reg = reg;
>
> @@ -534,7 +552,6 @@ resolve_registers_block(nir_block *block, void *void_state)
> instr->type == nir_instr_type_phi) {
> nir_instr_remove(instr);
> ralloc_steal(state->dead_ctx, instr);
> - continue;
> }
> }
> state->instr = NULL;
> @@ -543,11 +560,18 @@ resolve_registers_block(nir_block *block, void *void_state)
> if (following_if && following_if->condition.is_ssa) {
> nir_register *reg = get_register_for_ssa_def(following_if->condition.ssa,
> state);
> - memset(&following_if->condition, 0, sizeof following_if->condition);
> - following_if->condition.reg.reg = reg;
> + if (reg) {
> + memset(&following_if->condition, 0, sizeof following_if->condition);
> + following_if->condition.reg.reg = reg;
>
> - _mesa_set_add(reg->if_uses, _mesa_hash_pointer(following_if),
> - following_if);
> + _mesa_set_add(reg->if_uses, _mesa_hash_pointer(following_if),
> + following_if);
> + } else {
> + /* FIXME: We really shouldn't hit this. We should be doing
> + * constant control flow propagation.
> + */
> + assert(following_if->condition.ssa->parent_instr->type == nir_instr_type_load_const);
> + }
> }
>
> return true;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 3ec2fa6..019d649 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -981,25 +981,37 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
> fs_reg
> fs_visitor::get_nir_src(nir_src src)
> {
> - fs_reg reg;
> - if (src.reg.reg->is_global)
> - reg = nir_globals[src.reg.reg->index];
> - else
> - reg = nir_locals[src.reg.reg->index];
> + if (src.is_ssa) {
> + assert(src.ssa->parent_instr->type == nir_instr_type_load_const);
> + nir_load_const_instr *load = nir_instr_as_load_const(src.ssa->parent_instr);
> + fs_reg reg(GRF, virtual_grf_alloc(src.ssa->num_components),
> + BRW_REGISTER_TYPE_D);
>
> - /* to avoid floating-point denorm flushing problems, set the type by
> - * default to D - instructions that need floating point semantics will set
> - * this to F if they need to
> - */
> - reg.type = BRW_REGISTER_TYPE_D;
> - reg.reg_offset = src.reg.base_offset;
> - if (src.reg.indirect) {
> - reg.reladdr = new(mem_ctx) fs_reg();
> - *reg.reladdr = retype(get_nir_src(*src.reg.indirect),
> - BRW_REGISTER_TYPE_D);
> - }
> + for (unsigned i = 0; i < src.ssa->num_components; ++i)
> + emit(MOV(offset(reg, i), fs_reg(load->value.i[i])));
>
> - return reg;
> + return reg;
> + } else {
> + fs_reg reg;
> + if (src.reg.reg->is_global)
> + reg = nir_globals[src.reg.reg->index];
> + else
> + reg = nir_locals[src.reg.reg->index];
> +
> + /* to avoid floating-point denorm flushing problems, set the type by
> + * default to D - instructions that need floating point semantics will set
> + * this to F if they need to
> + */
> + reg.type = BRW_REGISTER_TYPE_D;
> + reg.reg_offset = src.reg.base_offset;
> + if (src.reg.indirect) {
> + reg.reladdr = new(mem_ctx) fs_reg();
> + *reg.reladdr = retype(get_nir_src(*src.reg.indirect),
> + BRW_REGISTER_TYPE_D);
> + }
> +
> + return reg;
> + }
> }
>
> fs_reg
> @@ -1652,6 +1664,10 @@ fs_visitor::nir_emit_texture(nir_tex_instr *instr)
> void
> fs_visitor::nir_emit_load_const(nir_load_const_instr *instr)
> {
> + /* Bail on SSA constant loads. These are used for immediates. */
> + if (instr->dest.is_ssa)
> + return;
> +
> fs_reg dest = get_nir_dest(instr->dest);
> dest.type = BRW_REGISTER_TYPE_UD;
> if (instr->array_elems == 0) {
> --
> 2.2.0
>
> _______________________________________________
> 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