<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 28, 2015 at 4:02 PM, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Tue, Jan 27, 2015 at 5:31 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> index de0d780..217fe09 100644<br>
</span><span class="">> @@ -689,9 +689,9 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)<br>
><br>
>        struct brw_reg acc = retype(brw_acc_reg(dispatch_width), result.type);<br>
><br>
> -      emit_percomp(MUL(acc, op[0], op[1]), instr->dest.write_mask);<br>
> -      emit_percomp(MACH(reg_null_d, op[0], op[1]), instr->dest.write_mask);<br>
> -      emit_percomp(MOV(result, fs_reg(acc)), instr->dest.write_mask);<br>
<br>
</span>Bah. How the hell did the old code pass piglit?<br></blockquote><div><br></div><div>I think channel expressions saved it.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> +      emit(MUL(acc, op[0], op[1]));<br>
> +      emit(MACH(reg_null_d, op[0], op[1]));<br>
> +      emit(MOV(result, fs_reg(acc)));<br>
>        break;<br>
>     }<br>
><br>
</span><span class="">> @@ -773,72 +767,38 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)<br>
>     case nir_op_ball_fequal3:<br>
>     case nir_op_ball_iequal3:<br>
>     case nir_op_ball_fequal4:<br>
> -   case nir_op_ball_iequal4: {<br>
> -      unsigned num_components = nir_op_infos[instr->op].input_sizes[0];<br>
> -      fs_reg temp = vgrf(num_components);<br>
> -      emit_percomp(CMP(temp, op[0], op[1], BRW_CONDITIONAL_Z),<br>
> -                   (1 << num_components) - 1);<br>
> -      emit_reduction(BRW_OPCODE_AND, result, temp, num_components);<br>
> -      break;<br>
> -   }<br>
> -<br>
> +   case nir_op_ball_iequal4:<br>
<br>
</span>We can save it for later, but it might be interesting to let the fs<br>
backend get the vector comparisons directly, if that's possible.<br>
<br>
See <a href="https://bugs.freedesktop.org/show_bug.cgi?id=77456" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=77456</a><br></blockquote><div><br></div><div>We can.  We just have to add a flag to turn that off in the nir_lower_alu_to_scalar pass.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> @@ -867,83 +827,67 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)<br>
>        unreachable("not reached: should be handled by ldexp_to_arith()");<br>
><br>
>     case nir_op_fsqrt:<br>
> -      emit_math_percomp(SHADER_OPCODE_SQRT, result, op[0],<br>
> -                        instr->dest.write_mask, instr->dest.saturate);<br>
> +      emit_math(SHADER_OPCODE_SQRT, result, op[0])<br>
> +               ->saturate = instr->dest.saturate;<br>
<br>
</span>I don't really like the ->saturate = ... as a single statement. I know<br>
I'm guilty of using it in a unit test recently.<br></blockquote><div><br></div><div>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?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I guess if we remove saturate as a destination modifier from NIR we'll<br>
get rid of most instances of this pattern, leaving just ->predicate.<br>
Are we planning to remove src/dst modifiers from NIR?<br>
</blockquote></div><br></div><div class="gmail_extra">Not until we get SSA in the backend.  It's so much easier to apply them in SSA.  If we'd like to drop one of them we can always add some flags to the lowering pass that adds them back in.<br></div><div class="gmail_extra">--Jason<br></div></div>