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

[Samuel Iglesias Gonsálvez]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256



On 02/05/16 10:38, Pohjolainen, Topi wrote:
> 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.
> 

Why not just:

op[0] =
   offset(op[0], bld, alu_parent->src[index].swizzle[channel]);

I think that's easier and doesn't complicate the rest of the code.

>> +               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));
> 

OK. Thanks,

Sam

>> 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
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJXKFTOAAoJEH/0ujLxfcNDpscP/imGsXaEkXCaEp9a7JLB4iOR
PuZDZQ6A+5ZlTYH9Ml+8VJbcXhyQ5Xy+gvfzJ11Ttl8yyMGRbTjCNZLcJj66y6gE
9ewHXrqWe6bgHOhQECrzmSJPFHl7nEv17ZE5QOUwaB6/xuX+THDqDMTyFoTvFzS2
KvmCTZD01wT+Ln0QWIk+jiOamGmJJstqJn3Sn/a6Ne2AhjgSdEcd8+ca9DOTo+qi
CzBT2pijfEIuyfEhZqfB9esLr6xsf/GbD5FuvCi5MbuCqfzt7NQEhaFDZTaVkWef
JEtXQrHuOvXD54qHCXvGM1D9ticz/TQDGS3eB1qaRP87Je9B5ANetfHU3Te2gb/X
ffr52s5Ra3yAWm6tLmvVm4L/Gn1lG/UB0qBU+j2933bUYj0kaM3zgWbmUXc//VTF
1XBXwLPj1JI58EbTP5mVb1b5DPfcp+UI8BFCDvq6ezQjw4J+DEyrDswYVGh+gPg3
6jOi49myrUonviGYfh5YbiQ/YpynDsLOc+sveon2le/4I417cLEGceDf0lyDd5o6
HGSLvCV6eCzQpJGoboL56Sw+kw7QHD6dixrRia+lzasC9MUt05h5F5ez6dnv6Is/
RroqVEYGN2EUnvWoel7iPgvY/hMs+2EVRUoyk66TPJRKkFAohq7v8fjr2wD1FYDk
ugdBtRXUZO05BCoaIKqH
=mvCV
-----END PGP SIGNATURE-----