<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 29, 2016 at 8:51 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"><div class="HOEnZb"><div class="h5">On Fri, Mar 25, 2016 at 4:12 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp   | 28 ++++++++++++++++++++++++++++<br>
>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 25 +++++++++++++++++++++++++<br>
>  2 files changed, 53 insertions(+)<br>
><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 7aff042..5255434 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> @@ -1016,6 +1016,34 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)<br>
>        inst->saturate = instr->dest.saturate;<br>
>        break;<br>
><br>
> +   case nir_op_fquantize2f16: {<br>
> +      fs_reg tmp16 = bld.vgrf(BRW_REGISTER_TYPE_D);<br>
> +      fs_reg tmp32 = bld.vgrf(BRW_REGISTER_TYPE_F);<br>
> +      fs_reg zero = bld.vgrf(BRW_REGISTER_TYPE_F);<br>
> +<br>
> +      /* The destination stride must be at least as big as the source stride. */<br>
> +      tmp16.type = BRW_REGISTER_TYPE_W;<br>
> +      tmp16.stride = 2;<br>
> +<br>
> +      /* Check for denormal */<br>
> +      fs_reg abs_src0 = op[0];<br>
> +      abs_src0.abs = true;<br>
> +      bld.CMP(bld.null_reg_f(), abs_src0, brw_imm_f(ldexpf(1.0, -14)),<br>
> +              BRW_CONDITIONAL_L);<br>
> +      /* Get the appropriately signed zero */<br>
> +      bld.AND(retype(zero, BRW_REGISTER_TYPE_UD),<br>
> +              retype(op[0], BRW_REGISTER_TYPE_UD),<br>
> +              brw_imm_ud(0x80000000));<br>
> +      /* Do the actual F32 -> F16 -> F32 conversion */<br>
> +      bld.emit(BRW_OPCODE_F32TO16, tmp16, op[0]);<br>
> +      bld.emit(BRW_OPCODE_F16TO32, tmp32, tmp16);<br>
> +      /* Select that or zero based on normal status */<br>
> +      inst = bld.SEL(result, zero, tmp32);<br>
> +      inst->predicate = BRW_PREDICATE_NORMAL;<br>
> +      inst->saturate = instr->dest.saturate;<br>
> +      break;<br>
> +   }<br>
> +<br>
>     case nir_op_fmin:<br>
>     case nir_op_imin:<br>
>     case nir_op_umin:<br>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp<br>
> index c18694f..2e714fa 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp<br>
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp<br>
> @@ -1213,6 +1213,31 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr)<br>
>        inst->saturate = instr->dest.saturate;<br>
>        break;<br>
><br>
> +   case nir_op_fquantize2f16: {<br>
> +      /* See also vec4_visitor::emit_pack_half_2x16() */<br>
> +      src_reg tmp16 = src_reg(this, glsl_type::uvec4_type);<br>
> +      src_reg tmp32 = src_reg(this, glsl_type::vec4_type);<br>
> +      src_reg zero = src_reg(this, glsl_type::vec4_type);<br>
> +<br>
> +      /* Check for denormal */<br>
> +      src_reg abs_src0 = op[0];<br>
> +      abs_src0.abs = true;<br>
> +      emit(CMP(dst_null_f(), abs_src0, brw_imm_f(ldexpf(1.0, -14)),<br>
> +               BRW_CONDITIONAL_L));<br>
<br>
</div></div>This, and elsewhere in this patch and series, should be using the builder.<span class=""><br></span></blockquote><div><br></div><div>We aren't using the builder for the rest of brw_vec4_nir.  Can we mix builder and non-builder things?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> +      /* Get the appropriately signed zero */<br>
> +      emit(AND(retype(dst_reg(zero), BRW_REGISTER_TYPE_UD),<br>
> +               retype(op[0], BRW_REGISTER_TYPE_UD),<br>
> +               brw_imm_ud(0x80000000)));<br>
> +      /* Do the actual F32 -> F16 -> F32 conversion */<br>
> +      emit(F32TO16(dst_reg(tmp16), op[0]));<br>
> +      emit(F16TO32(dst_reg(tmp32), tmp16));<br>
> +      /* Select that or zero based on normal status */<br>
> +      inst = emit(BRW_OPCODE_SEL, dst, zero, tmp32);<br>
> +      inst->predicate = BRW_PREDICATE_NORMAL;<br>
> +      inst->saturate = instr->dest.saturate;<br>
<br>
</span>There are builder methods for these things: set_predicate, set_saturate.<br>
</blockquote></div><br></div><div class="gmail_extra">I can do that.  I didn't because I felt it was actually substantially messier but I don't care that much.<br></div><div class="gmail_extra"><br></div></div>