<div dir="ltr">Matt wanted shader-db numbers and here they are: total instructions in shared programs: 5998287 -> 5974070 (-0.40%) instructions in affected programs: 732041 -> 707824 (-3.31%) helped: 3135 HURT: 193 GAINED: 18 LOST: 0 </div><div class="gmail_extra"> <div class="gmail_quote">On Thu, Jan 29, 2015 at 3:41 AM, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wednesday, January 28, 2015 04:27:23 PM Jason Ekstrand wrote: > On Wed, Jan 28, 2015 at 4:02 PM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote: > > > On Tue, Jan 27, 2015 at 5:31 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> > > wrote: > > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > > > index de0d780..217fe09 100644 > > > @@ -689,9 +689,9 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr) > > > > > > struct brw_reg acc = retype(brw_acc_reg(dispatch_width), > > result.type); > > > > > > - emit_percomp(MUL(acc, op[0], op[1]), instr->dest.write_mask); > > > - emit_percomp(MACH(reg_null_d, op[0], op[1]), > > instr->dest.write_mask); > > > - emit_percomp(MOV(result, fs_reg(acc)), instr->dest.write_mask); > > > > Bah. How the hell did the old code pass piglit? > > > > I think channel expressions saved it. > > > > > > > + emit(MUL(acc, op[0], op[1])); > > > + emit(MACH(reg_null_d, op[0], op[1])); > > > + emit(MOV(result, fs_reg(acc))); > > > break; > > > } > > > > > > @@ -773,72 +767,38 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr) > > > case nir_op_ball_fequal3: > > > case nir_op_ball_iequal3: > > > case nir_op_ball_fequal4: > > > - case nir_op_ball_iequal4: { > > > - unsigned num_components = nir_op_infos[instr->op].input_sizes[0]; > > > - fs_reg temp = vgrf(num_components); > > > - emit_percomp(CMP(temp, op[0], op[1], BRW_CONDITIONAL_Z), > > > - (1 << num_components) - 1); > > > - emit_reduction(BRW_OPCODE_AND, result, temp, num_components); > > > - break; > > > - } > > > - > > > + case nir_op_ball_iequal4: > > > > We can save it for later, but it might be interesting to let the fs > > backend get the vector comparisons directly, if that's possible. > > > > See <a href="https://bugs.freedesktop.org/show_bug.cgi?id=77456" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=77456</a> > > > > We can. We just have to add a flag to turn that off in the > nir_lower_alu_to_scalar pass. > > > > > > > @@ -867,83 +827,67 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr) > > > unreachable("not reached: should be handled by ldexp_to_arith()"); > > > > > > case nir_op_fsqrt: > > > - emit_math_percomp(SHADER_OPCODE_SQRT, result, op[0], > > > - instr->dest.write_mask, instr->dest.saturate); > > > + emit_math(SHADER_OPCODE_SQRT, result, op[0]) > > > + ->saturate = instr->dest.saturate; > > > > I don't really like the ->saturate = ... as a single statement. I know > > I'm guilty of using it in a unit test recently. > > > > Yeah, I'm not sure what to do there. I really don't like having to assign > it to a fs_inst variable and then do the saturate. I guess we could. Or > we could add something to emit_math. Thoughts? </div></div>In the brw_eu_emit.c code I worked around this by creating a brw_last_inst macro that always refers to the most recently emitted instruction. Still not 100% crazy about it, but it's been pretty handy. *shrug*. It's an idea, anyway. I'm not too opposed to just creating fs_inst * temporaries. --Ken</blockquote></div> </div>