On Tue, May 03, 2016 at 09:35:42AM +0200, Samuel Iglesias Gons?lvez wrote:
> -----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 <ito...@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.
Sure, which ever way you feel best.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
