Nouveau - Nov 2014 - [Mesa-dev] [PATCH] nv50/ir: saturate FRC result to avoid completely bogus values

On Tue, Nov 18, 2014 at 8:54 AM, Roland Scheidegger <sroland at
vmware.com> wrote:
> Am 18.11.2014 um 05:03 schrieb Ilia Mirkin:
>> For values above integer accuracy in floats, val - floor(val) might
>> actually produce a value greater than 1. For such large floats,
it's
>> reasonable to be imprecise, but it's unreasonable for FRC to return
a
>> value that is not between 0 and 1.
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> index 41b91e8..e5b767f 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> @@ -2512,7 +2512,8 @@ Converter::handleInstruction(const struct
tgsi_full_instruction *insn)
>>           src0 = fetchSrc(0, c);
>>           val0 = getScratch();
>>           mkOp1(OP_FLOOR, TYPE_F32, val0, src0);
>> -         mkOp2(OP_SUB, TYPE_F32, dst0[c], src0, val0);
>> +         mkOp2(OP_SUB, TYPE_F32, val0, src0, val0);
>> +         mkOp1(OP_SAT, TYPE_F32, dst0[c], val0);
>>        }
>>        break;
>>     case TGSI_OPCODE_ROUND:
>>
>
> I don't understand the math behind this. For any such large number, as
> far as I can tell floor(val) == val and hence the end result ought to be
> zero. Or doesn't your floor work like that?
I could be thinking about this backwards, but let's say that floats
lose integer precision at 10.0. And I do floor(12.5)... normally this
would be 12.0, but since that's not exactly representable, it might
actually be 11.0. (Or would it be 11.9987? I didn't consider that
possibility...) And then 12.5 - 11 = 1.5. Or am I thinking about this
backwards? I guess ideally I'd do something along the lines of y = x -
floor(x); return y - floor(y). That seems like it might be more
accurate... not sure.

Am 18.11.2014 um 15:05 schrieb Ilia Mirkin:
> On Tue, Nov 18, 2014 at 8:54 AM, Roland Scheidegger <sroland at
vmware.com> wrote:
>> Am 18.11.2014 um 05:03 schrieb Ilia Mirkin:
>>> For values above integer accuracy in floats, val - floor(val) might
>>> actually produce a value greater than 1. For such large floats,
it's
>>> reasonable to be imprecise, but it's unreasonable for FRC to
return a
>>> value that is not between 0 and 1.
>>>
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>>  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git
a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>> index 41b91e8..e5b767f 100644
>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>> @@ -2512,7 +2512,8 @@ Converter::handleInstruction(const struct
tgsi_full_instruction *insn)
>>>           src0 = fetchSrc(0, c);
>>>           val0 = getScratch();
>>>           mkOp1(OP_FLOOR, TYPE_F32, val0, src0);
>>> -         mkOp2(OP_SUB, TYPE_F32, dst0[c], src0, val0);
>>> +         mkOp2(OP_SUB, TYPE_F32, val0, src0, val0);
>>> +         mkOp1(OP_SAT, TYPE_F32, dst0[c], val0);
>>>        }
>>>        break;
>>>     case TGSI_OPCODE_ROUND:
>>>
>>
>> I don't understand the math behind this. For any such large number,
as
>> far as I can tell floor(val) == val and hence the end result ought to
be
>> zero. Or doesn't your floor work like that?
> 
> I could be thinking about this backwards, but let's say that floats
> lose integer precision at 10.0. And I do floor(12.5)... normally this
> would be 12.0, but since that's not exactly representable, it might
> actually be 11.0. (Or would it be 11.9987? I didn't consider that
> possibility...) And then 12.5 - 11 = 1.5. Or am I thinking about this
> backwards? I guess ideally I'd do something along the lines of y = x -
> floor(x); return y - floor(y). That seems like it might be more
> accurate... not sure.
> 
If your float is large enough that the next closest float is more than
1.0 away, then that float would have been an exact integer, thus floor()
doing nothing.

Roland

On 18/11/14 14:34, Roland Scheidegger wrote:
> Am 18.11.2014 um 15:05 schrieb Ilia Mirkin:
>> On Tue, Nov 18, 2014 at 8:54 AM, Roland Scheidegger <sroland at
vmware.com> wrote:
>>> Am 18.11.2014 um 05:03 schrieb Ilia Mirkin:
>>>> For values above integer accuracy in floats, val - floor(val)
might
>>>> actually produce a value greater than 1. For such large floats,
it's
>>>> reasonable to be imprecise, but it's unreasonable for FRC
to return a
>>>> value that is not between 0 and 1.
>>>>
>>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>>> ---
>>>>   src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 3
++-
>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git
a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> index 41b91e8..e5b767f 100644
>>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> @@ -2512,7 +2512,8 @@ Converter::handleInstruction(const struct
tgsi_full_instruction *insn)
>>>>            src0 = fetchSrc(0, c);
>>>>            val0 = getScratch();
>>>>            mkOp1(OP_FLOOR, TYPE_F32, val0, src0);
>>>> -         mkOp2(OP_SUB, TYPE_F32, dst0[c], src0, val0);
>>>> +         mkOp2(OP_SUB, TYPE_F32, val0, src0, val0);
>>>> +         mkOp1(OP_SAT, TYPE_F32, dst0[c], val0);
>>>>         }
>>>>         break;
>>>>      case TGSI_OPCODE_ROUND:
>>>>
>>>
>>> I don't understand the math behind this. For any such large
number, as
>>> far as I can tell floor(val) == val and hence the end result ought
to be
>>> zero. Or doesn't your floor work like that?
>>
>> I could be thinking about this backwards, but let's say that floats
>> lose integer precision at 10.0. And I do floor(12.5)... normally this
>> would be 12.0, but since that's not exactly representable, it might
>> actually be 11.0. (Or would it be 11.9987? I didn't consider that
>> possibility...) And then 12.5 - 11 = 1.5. Or am I thinking about this
>> backwards? I guess ideally I'd do something along the lines of y =
x -
>> floor(x); return y - floor(y). That seems like it might be more
>> accurate... not sure.
>>
> If your float is large enough that the next closest float is more than
> 1.0 away, then that float would have been an exact integer, thus floor()
> doing nothing.
>
> Roland
Roland's right -- it takes less mantissa bits to represent an integer x, 
than a fractional number between x and x + 1

The only case where `frac(x) = x - floor(x)` fails is when x is a 
negative denormal. It might give 1.0f instead of 0.0f, if the hardware 
is not setup to flush denormals to zero properly.

Jose