[Mesa-dev] [PATCH 32/59] i965/fs: optimize unpack double

[Pohjolainen, Topi]

On Fri, Apr 29, 2016 at 01:29:29PM +0200, Samuel Iglesias Gons?lvez wrote:
> From: Iago Toral Quiroga <itoral at igalia.com>
> 
> When we are actually unpacking from a double that we have previously
> packed from its 32-bit components we can bypass the pack operation
> and source from its arguments directly.
> ---
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 4332575..0ce25a0 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -1118,12 +1118,32 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
>        break;
>  
>     case nir_op_unpack_double_2x32_split_x:
> -      bld.MOV(result, stride(retype(op[0], BRW_REGISTER_TYPE_UD), 2));
> -      break;
> +   case nir_op_unpack_double_2x32_split_y: {
> +      /* Optimize the common case where we are unpacking from a double we have
> +       * previously packed. In this case we can just bypass the pack operation
> +       * and source directly from its arguments.
> +       */
> +      unsigned index = (instr->op == nir_op_unpack_double_2x32_split_x) ? 0 : 1;
> +      if (instr->src[0].src.is_ssa) {
> +         nir_instr *parent_instr = instr->src[0].src.ssa->parent_instr;
> +         if (parent_instr->type == nir_instr_type_alu) {
> +            nir_alu_instr *alu_parent = nir_instr_as_alu(parent_instr);
> +            if (alu_parent->op == nir_op_pack_double_2x32_split) {
> +               op[0] = retype(get_nir_src(alu_parent->src[index].src),
> +                              BRW_REGISTER_TYPE_UD);
> +               op[0] = offset(op[0], bld, alu_parent->src[index].swizzle[channel]);

Overflowing line, I tried further down hot it looks with less indentation.
I don't know if it was better or worse.

> +               bld.MOV(result, op[0]);
> +               break;
> +            }
> +         }
> +      }

         if (!instr->src[0].src.is_ssa) {
            break;
         }

         const nir_instr *parent_instr = instr->src[0].src.ssa->parent_instr;
         if (parent_instr->type != nir_instr_type_alu) {
            break;
         }

         const nir_alu_instr *alu_parent = nir_instr_as_alu(parent_instr);
         if (alu_parent->op != nir_op_pack_double_2x32_split) {
            break;
         }

         op[0] = retype(get_nir_src(alu_parent->src[index].src),
                        BRW_REGISTER_TYPE_UD);
         op[0] = offset(op[0], bld, alu_parent->src[index].swizzle[channel]);
         bld.MOV(result, op[0]);
         break;
> +      }
>  
>  
> -   case nir_op_unpack_double_2x32_split_y:
> -      bld.MOV(result, stride(horiz_offset(retype(op[0], BRW_REGISTER_TYPE_UD), 1), 2));
> +      if (instr->op == nir_op_unpack_double_2x32_split_x)
> +         bld.MOV(result, stride(retype(op[0], BRW_REGISTER_TYPE_UD), 2));
> +      else
> +         bld.MOV(result, stride(horiz_offset(retype(op[0], BRW_REGISTER_TYPE_UD), 1), 2));

Same here, maybe:

           bld.MOV(
              result,
              stride(horiz_offset(retype(op[0], BRW_REGISTER_TYPE_UD), 1), 2));

>        break;
> +   }
>  
>     case nir_op_fpow:
>        inst = bld.emit(SHADER_OPCODE_POW, result, op[0], op[1]);
> -- 
> 2.5.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev