[PATCH] nv50/ir: saturate FRC result to avoid completely bogus values

[PATCH] nv50/ir: saturate FRC result to avoid completely bogus values

[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@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:
-- 
2.0.4

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Roland Scheidegger]

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@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?

Roland
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] nv50/ir: saturate FRC result to avoid completely bogus values

[Ilia Mirkin]

On Tue, Nov 18, 2014 at 8:54 AM, Roland Scheidegger <sroland@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@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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH] nv50/ir: saturate FRC result to avoid completely bogus values

[Roland Scheidegger]

Am 18.11.2014 um 15:05 schrieb Ilia Mirkin:
> On Tue, Nov 18, 2014 at 8:54 AM, Roland Scheidegger <sroland@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@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

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Jose Fonseca]

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@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@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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] nv50/ir: saturate FRC result to avoid completely bogus values

[Ilia Mirkin]

On Tue, Nov 18, 2014 at 9:53 AM, Jose Fonseca <jfonseca@vmware.com> wrote:
> 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@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@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.

Very cool. This patch wasn't in response to an actual issue but rather
a theoretical one. Sounds like I got the theory a little wrong... time
to think more about floats :)

  -ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev