[Mesa-dev] [PATCH 1/3] i965/fs: Add and use get_nir_src_imm().

[Kenneth Graunke]

On Wednesday, May 4, 2016 3:54:12 PM PDT Matt Turner wrote:
> Terrible name. Suggest something else, or even a better way to do this.
> 
> Basically, the next patch wants to inspect the LOD argument and do
> something different if it's 0.0f. But at that point we've emitted a MOV
> for it and we just have a register to look at.
> ---
> This is an alternative approach to the 4 patch series I sent ast night.
> 
>  src/mesa/drivers/dri/i965/brw_fs.h       |  1 +
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 36 ++++++++++++++++++++++++++
+-----
>  2 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/
brw_fs.h
> index ba6bd3f..29fa593 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -273,6 +273,7 @@ public:
>     void nir_emit_jump(const brw::fs_builder &bld,
>                        nir_jump_instr *instr);
>     fs_reg get_nir_src(nir_src src);
> +   fs_reg get_nir_src_imm(nir_src src);
>     fs_reg get_nir_dest(nir_dest dest);
>     fs_reg get_nir_image_deref(const nir_deref_var *deref);
>     fs_reg get_indirect_offset(nir_intrinsic_instr *instr);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/
dri/i965/brw_fs_nir.cpp
> index 4d14fda..fb59800 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -1245,6 +1245,32 @@ fs_visitor::get_nir_src(nir_src src)
>  }
>  
>  fs_reg
> +fs_visitor::get_nir_src_imm(nir_src src)
> +{
> +   fs_reg reg;
> +   if (src.is_ssa) {
> +      if (src.ssa->parent_instr->type == nir_instr_type_load_const) {
> +         nir_load_const_instr *load_const =
> +            nir_instr_as_load_const(src.ssa->parent_instr);
> +         return brw_imm_ud(load_const->value.u32[0]);
> +      }
> +
> +      reg = nir_ssa_values[src.ssa->index];
> +   } else {
> +      /* We don't handle indirects on locals */
> +      assert(src.reg.indirect == NULL);
> +      reg = offset(nir_locals[src.reg.reg->index], bld,
> +                   src.reg.base_offset * src.reg.reg->num_components);
> +   }
> +
> +   /* 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
> +    */
> +   return retype(reg, BRW_REGISTER_TYPE_D);
> +}

I actually wrote one of these in commit ce1511c627da2aa2a of my tree,
and it's much simpler:

/**
 * Return an IMM for constants; otherwise call get_nir_src() as normal.
 */
fs_reg
fs_visitor::get_nir_src_imm(nir_src src)
{
   nir_const_value *val = nir_src_as_const_value(src);
   return val ? fs_reg(val->i[0]) : get_nir_src(src);
}

I ended up dropping the patch because I just used nir_src_as_const_value()
in the callers directly, but it's probably nice to have one of these.

> +
> +fs_reg
>  fs_visitor::get_nir_dest(nir_dest dest)
>  {
>     if (dest.is_ssa) {
> @@ -3444,7 +3470,7 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, 
nir_tex_instr *instr)
>        fs_reg src = get_nir_src(instr->src[i].src);
>        switch (instr->src[i].src_type) {
>        case nir_tex_src_bias:
> -         lod = retype(src, BRW_REGISTER_TYPE_F);
> +         lod = retype(get_nir_src_imm(instr->src[i].src), 
BRW_REGISTER_TYPE_F);
>           break;
>        case nir_tex_src_comparitor:
>           shadow_comparitor = retype(src, BRW_REGISTER_TYPE_F);
> @@ -3462,7 +3488,7 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, 
nir_tex_instr *instr)
>           }
>           break;
>        case nir_tex_src_ddx:
> -         lod = retype(src, BRW_REGISTER_TYPE_F);
> +         lod = retype(get_nir_src_imm(instr->src[i].src), 
BRW_REGISTER_TYPE_F);
>           lod_components = nir_tex_instr_src_size(instr, i);
>           break;
>        case nir_tex_src_ddy:
> @@ -3471,13 +3497,13 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, 
nir_tex_instr *instr)
>        case nir_tex_src_lod:
>           switch (instr->op) {
>           case nir_texop_txs:
> -            lod = retype(src, BRW_REGISTER_TYPE_UD);
> +            lod = retype(get_nir_src_imm(instr->src[i].src), 
BRW_REGISTER_TYPE_UD);
>              break;
>           case nir_texop_txf:
> -            lod = retype(src, BRW_REGISTER_TYPE_D);
> +            lod = retype(get_nir_src_imm(instr->src[i].src), 
BRW_REGISTER_TYPE_D);
>              break;
>           default:
> -            lod = retype(src, BRW_REGISTER_TYPE_F);
> +            lod = retype(get_nir_src_imm(instr->src[i].src), 
BRW_REGISTER_TYPE_F);
>              break;
>           }
>           break;
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160504/8bb04ea9/attachment.sig>