[Mesa-dev] [PATCH 12/78] i965/nir/vec4: Add nir_get_dst() and nir_get_src() methods

[Jason Ekstrand]

On Fri, Jun 26, 2015 at 1:06 AM, Eduardo Lima Mitev <elima at igalia.com> wrote:
> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>
> These methods are essential for the implementation of the NIR->vec4 pass. They
> work similar to their fs_nir counter-parts.
>
> When processing instructions, these methods are invoked to resolve the
> brw registers (source or destination) corresponding to the NIR sources
> or destination. It builds a map of NIR register index to brw register for
> all registers locally allocated in a block.
>
> In the case of get_nir_src(), it also builds immediate registers on-the-fly
> when queried for a SSA source which at this point can only correspond to
> constant values.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.h       |   7 ++
>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 102 +++++++++++++++++++++++++++++
>  2 files changed, 109 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
> index d837d90..a0f5935 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -411,6 +411,13 @@ public:
>     virtual void nir_emit_jump(nir_jump_instr *instr);
>     virtual void nir_emit_texture(nir_tex_instr *instr);
>
> +   dst_reg get_nir_dest(nir_dest dest, enum brw_reg_type type);
> +   dst_reg get_nir_dest(nir_dest dest, nir_alu_type type);
> +   dst_reg get_nir_dest(nir_dest dest);
> +   src_reg get_nir_src(nir_src src, enum brw_reg_type type);
> +   src_reg get_nir_src(nir_src src, nir_alu_type type);
> +   src_reg get_nir_src(nir_src src);
> +
>     virtual dst_reg *make_reg_for_system_value(int location,
>                                                const glsl_type *type) = 0;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> index 36c9dc0..1ec75ee 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> @@ -365,6 +365,108 @@ vec4_visitor::nir_emit_instr(nir_instr *instr)
>     }
>  }
>
> +static dst_reg
> +dst_reg_for_nir_reg(vec4_visitor *v, nir_register *nir_reg,
> +                    unsigned base_offset, nir_src *indirect)
> +{
> +   dst_reg reg;
> +
> +   reg = v->nir_locals[nir_reg->index];
> +
> +   reg = offset(reg, base_offset * nir_reg->num_components);
> +   if (indirect) {
> +      int multiplier = nir_reg->num_components;
> +
> +      reg.reladdr = new(v->mem_ctx) src_reg(dst_reg(GRF, v->alloc.allocate(1)));
> +      v->emit(v->MUL(dst_reg(*reg.reladdr),
> +                     retype(v->get_nir_src(*indirect), BRW_REGISTER_TYPE_D),
> +                     src_reg(multiplier)));

I'm not sure this is correct.  We need this in the FS backend because
each register is a scalar value.  In vec4, each array element should
correspond to a single vec4 register so you shouldn't need to
multiply.  Actually, I think it's currently dead code in the backend
because we lower it all away, so it may not be correct at all.

> +   }
> +
> +   return reg;
> +}
> +
> +dst_reg
> +vec4_visitor::get_nir_dest(nir_dest dest)
> +{
> +   return dst_reg_for_nir_reg(this, dest.reg.reg, dest.reg.base_offset,
> +                              dest.reg.indirect);
> +}
> +
> +dst_reg
> +vec4_visitor::get_nir_dest(nir_dest dest, enum brw_reg_type type)
> +{
> +   dst_reg reg = get_nir_dest(dest);
> +   return retype(reg, type);
> +}
> +
> +dst_reg
> +vec4_visitor::get_nir_dest(nir_dest dest, nir_alu_type type)
> +{
> +   dst_reg reg = get_nir_dest(dest);
> +   return retype(reg, brw_type_for_nir_type(type));
> +}
> +
> +src_reg
> +vec4_visitor::get_nir_src(nir_src src, enum brw_reg_type type)
> +{
> +   dst_reg reg;
> +
> +   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);
> +
> +      reg = dst_reg(GRF, alloc.allocate(src.ssa->num_components));

It's vec4, you only need one register, not num_components-many.

> +      reg = retype(reg, type);
> +
> +      /* @FIXME: while this is what fs_nir does, we can do this better in the VS
> +       * stage because we can emit vector operations and save some MOVs in
> +       * cases where the constants are representable in 8 bits.
> +       * So by now, we emit a MOV for each component.
> +       */
> +      for (unsigned i = 0; i < src.ssa->num_components; ++i) {
> +         reg.writemask = 1 << i;
> +
> +         switch (reg.type) {
> +         case BRW_REGISTER_TYPE_F:
> +            emit(MOV(reg, src_reg(load->value.f[i])));
> +            break;
> +         case BRW_REGISTER_TYPE_D:
> +            emit(MOV(reg, src_reg(load->value.i[i])));
> +            break;
> +         case BRW_REGISTER_TYPE_UD:
> +            emit(MOV(reg, src_reg(load->value.u[i])));
> +            break;
> +         default:
> +            unreachable("invalid register type");
> +         }
> +      }
> +
> +      /* Set final writemask */
> +      reg.writemask = brw_writemask_for_size(src.ssa->num_components);
> +   }
> +   else {
> +     reg = dst_reg_for_nir_reg(this, src.reg.reg, src.reg.base_offset,
> +                               src.reg.indirect);
> +     reg = retype(reg, type);

Why do you support indirects for destinations but not here?

> +   }
> +
> +   return src_reg(reg);
> +}
> +
> +src_reg
> +vec4_visitor::get_nir_src(nir_src src, nir_alu_type type)
> +{
> +   return get_nir_src(src, brw_type_for_nir_type(type));
> +}
> +
> +src_reg
> +vec4_visitor::get_nir_src(nir_src src)
> +{
> +   /* if type is not specified, default to signed int */
> +   return get_nir_src(src, nir_type_int);
> +}
> +
>  void
>  vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
>  {
> --
> 2.1.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev