<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 25, 2016 at 8:19 AM, 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 Thu, Feb 25, 2016 at 2:15 AM, Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote:<br>
> From the OpenGL 4.2 spec:<br>
><br>
> "When a constructor is used to convert any integer or floating-point type to a<br>
>  bool, 0 and 0.0 are converted to false, and non-zero values are converted to<br>
>  true."<br>
><br>
> Thus, even the smallest non-zero floating value should be translated to true.<br>
> This behavior has been verified also with proprietary NVIDIA drivers.<br>
<br>
</span>Ooh, interesting.<br>
<span class=""><br>
> Currently, we implement this conversion as a <a href="http://cmp.nz" rel="noreferrer" target="_blank">cmp.nz</a> operation with floats,<br>
> subject to floating-point precision limitations, and as a result, relatively<br>
> small non-zero floating point numbers return false instead of true.<br>
><br>
> This patch fixes the problem by getting rid of the sign bit (to cover the case<br>
> of -0.0) and testing the result against 0u using an integer comparison instead.<br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 15 ++++++++++++---<br>
>  1 file changed, 12 insertions(+), 3 deletions(-)<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 db20c71..7d62d7e 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> @@ -913,9 +913,18 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)<br>
>        bld.MOV(result, negate(op[0]));<br>
>        break;<br>
><br>
> -   case nir_op_f2b:<br>
> -      bld.CMP(result, op[0], brw_imm_f(0.0f), BRW_CONDITIONAL_NZ);<br>
> -      break;<br>
> +   case nir_op_f2b: {<br>
> +      /* Because comparing to 0.0f is subject to precision limitations, do the<br>
> +       * comparison using integers (we need to get rid of the sign bit for that)<br>
> +       */<br>
> +      if (devinfo->gen >= 8)<br>
> +         op[0] = resolve_source_modifiers(op[0]);<br></span></blockquote><div><br></div><div>Hrm... I'm not sure what I think about this.  Neither abs nor neg *should* affect f2b since zero/non-zero should just pass through.  However, resolving the source modifiers could affect if they, for instance, flushed dnorms.  Incidentally, this means we probably want a NIR optimization to get rid of neg and abs right before != 0 if we can.<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>
</span>Oh, good thinking.<br>
<span class=""><br>
> +      op[0] = retype(op[0], BRW_REGISTER_TYPE_UD);<br>
> +      bld.AND(op[0], op[0], brw_imm_ud(0x7FFFFFFFu));<br>
> +      bld.CMP(result, op[0], brw_imm_ud(0u), BRW_CONDITIONAL_NZ);<br>
<br>
</span>The cmod_propagation is going to turn this into an <a href="http://AND.NZ" rel="noreferrer" target="_blank">AND.NZ</a>, which we<br>
may as well just do here:<br>
<br>
   set_condmod(BRW_CONDITIONAL_NZ,<br>
                          bld.AND(result, op[0], brw_imm_ud(0x7FFFFFFFu));<br>
<br>
> +       break;<br>
<br>
Bad indentation.<br>
<br>
With those two things changed in both patches, they are<br>
<br>
Reviewed-by: Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>><br>
<div class="HOEnZb"><div class="h5">_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div></div>