Roland Scheidegger <srol...@vmware.com> writes: > Am 25.02.2016 um 11:15 schrieb Iago Toral Quiroga: >> 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. >> >> Currently, we implement this conversion as a cmp.nz operation with floats, >> subject to floating-point precision limitations, and as a result, relatively
The bool constructor *is* subject to floating-point precision
limitations AFAIK, just like anything else dealing with floating-point
numbers.
>> 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]);
>> + 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);
>> + break;
>> + }
>> +
>> case nir_op_i2b:
>> bld.CMP(result, op[0], brw_imm_d(0), BRW_CONDITIONAL_NZ);
>> break;
>>
>
> Does that fix anything? I don't really see a problem with the existing
> logic. Yes any "non-zero value" should be converted to true. But surely
> that definition cannot include denorms, which you are always allowed to
> flush to zero.
Yeah, one is allowed to flush denorms to zero on input to any operation,
including bool(), I don't see any reason in the above why bool()
shouldn't be equivalent to "!= 0".
> (Albeit I can't tell what the result would be with NaNs with the float
> compare, nor what the result actually should be in this case since glsl
> doesn't require NaNs neither.)
>
The hardware CMP instruction considers NaNs to be different from zero,
and AFAICT the implementation in this patch does the same.
> Roland
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
