<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Feb 14, 2017 at 11:29 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The optimization in unpack_64 is clearly subsumed with the opt_algebraic optimizations in the previous commit. The pack optimization may not be quite handled by opt_algebraic but opt_algebraic should get the really bad cases. Also, it's been broken since it was merged and we've never noticed so it must not be doing anything. </blockquote><div> </div><div>Ok, that's not quite fair. It has been broken since it was merged but the breakage is pretty hard to hit since it requires a NIR register. However, this doesn't really cause any regressions either since, prior to patch 1/5, it was basically never getting triggered. </div><div>I did look at some of the shaders mentioned in the bug as being the reason for this optimization and none of them were hurt by removing it (even after it was fixed). I think back-end copy-prop may have just gotten good enough to make it not needed. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> --- src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 50 -------------------------------- 1 file changed, 50 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp index 91c14eb..9647300 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp @@ -1213,61 +1213,11 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr) break; case nir_op_pack_64_2x32_split: - /* Optimize the common case where we are re-packing a double with - * the result of a previous double unpack. In this case we can take the - * 32-bit value to use in the re-pack from the original double and bypass - * the unpack operation. - */ - for (int i = 0; i < 2; i++) { - if (!instr->src[i].src.is_ssa) - continue; - - const nir_instr *parent_instr = instr->src[i].src.ssa->parent_instr; - if (parent_instr->type == nir_instr_type_alu) - continue; - - const nir_alu_instr *alu_parent = nir_instr_as_alu(parent_instr); - if (alu_parent->op == nir_op_unpack_64_2x32_split_x || - alu_parent->op == nir_op_unpack_64_2x32_split_y) - continue; - - if (!alu_parent->src[0].src.is_ssa) - continue; - - op[i] = get_nir_src(alu_parent->src[0].src); - op[i] = offset(retype(op[i], BRW_REGISTER_TYPE_DF), bld, - alu_parent->src[0].swizzle[channel]); - if (alu_parent->op == nir_op_unpack_64_2x32_split_y) - op[i] = subscript(op[i], BRW_REGISTER_TYPE_UD, 1); - else - op[i] = subscript(op[i], BRW_REGISTER_TYPE_UD, 0); - } bld.emit(FS_OPCODE_PACK, result, op[0], op[1]); break; case nir_op_unpack_64_2x32_split_x: case nir_op_unpack_64_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_64_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_64_2x32_split && - alu_parent->src[index].src.is_ssa) { - 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; - } - } - } - if (instr->op == nir_op_unpack_64_2x32_split_x) bld.MOV(result, subscript(op[0], BRW_REGISTER_TYPE_UD, 0)); else -- 2.5.0.400.gff86faf </blockquote></div> </div></div>