On Thu, Feb 25, 2016 at 8:19 AM, Matt Turner <matts...@gmail.com> wrote:
> On Thu, Feb 25, 2016 at 2:15 AM, Iago Toral Quiroga <ito...@igalia.com> > wrote: > > From the OpenGL 4.2 spec: > > > > "When a constructor is used to convert any integer or floating-point > type to a > > bool, 0 and 0.0 are converted to false, and non-zero values are > converted to > > true." > > > > Thus, even the smallest non-zero floating value should be translated to > true. > > This behavior has been verified also with proprietary NVIDIA drivers. > > Ooh, interesting. > > > Currently, we implement this conversion as a cmp.nz operation with > floats, > > subject to floating-point precision limitations, and as a result, > relatively > > small non-zero floating point numbers return false instead of true. > > > > This patch fixes the problem by getting rid of the sign bit (to cover > the case > > of -0.0) and testing the result against 0u using an integer comparison > instead. > > --- > > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > > index db20c71..7d62d7e 100644 > > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > > @@ -913,9 +913,18 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, > nir_alu_instr *instr) > > bld.MOV(result, negate(op[0])); > > break; > > > > - case nir_op_f2b: > > - bld.CMP(result, op[0], brw_imm_f(0.0f), BRW_CONDITIONAL_NZ); > > - break; > > + case nir_op_f2b: { > > + /* Because comparing to 0.0f is subject to precision limitations, > do the > > + * comparison using integers (we need to get rid of the sign bit > for that) > > + */ > > + if (devinfo->gen >= 8) > > + op[0] = resolve_source_modifiers(op[0]); > 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. > > Oh, good thinking. > > > + op[0] = retype(op[0], BRW_REGISTER_TYPE_UD); > > + bld.AND(op[0], op[0], brw_imm_ud(0x7FFFFFFFu)); > > + bld.CMP(result, op[0], brw_imm_ud(0u), BRW_CONDITIONAL_NZ); > > The cmod_propagation is going to turn this into an AND.NZ, which we > may as well just do here: > > set_condmod(BRW_CONDITIONAL_NZ, > bld.AND(result, op[0], brw_imm_ud(0x7FFFFFFFu)); > > > + break; > > Bad indentation. > > With those two things changed in both patches, they are > > Reviewed-by: Matt Turner <matts...@gmail.com> > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev