[Mesa-stable] [Mesa-dev] [PATCH 24/53] st/nine: Handle RSQ special cases

[Roland Scheidegger]

Am 12.01.2015 um 17:27 schrieb Tom Stellard:
> On Thu, Jan 08, 2015 at 01:46:32PM +0100, Marek Olšák wrote:
>> On Wed, Jan 7, 2015 at 7:09 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>> On Wed, Jan 7, 2015 at 11:36 AM, Axel Davy <axel.davy at ens.fr> wrote:
>>>> We should use the absolute value of the input as input to ureg_RSQ.
>>>>
>>>> Moreover, an input of 0.0 should return FLT_MAX.
>>>>
>>>> Reviewed-by: David Heidelberg <david at ixit.cz>
>>>> Signed-off-by: Axel Davy <axel.davy at ens.fr>
>>>>
>>>> Cc: "10.4" <mesa-stable at lists.freedesktop.org>
>>>> ---
>>>>  src/gallium/state_trackers/nine/nine_shader.c | 13 ++++++++++++-
>>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/gallium/state_trackers/nine/nine_shader.c b/src/gallium/state_trackers/nine/nine_shader.c
>>>> index da77da5..4dee5f5 100644
>>>> --- a/src/gallium/state_trackers/nine/nine_shader.c
>>>> +++ b/src/gallium/state_trackers/nine/nine_shader.c
>>>> @@ -1957,6 +1957,17 @@ DECL_SPECIAL(POW)
>>>>      return D3D_OK;
>>>>  }
>>>>
>>>> +DECL_SPECIAL(RSQ)
>>>> +{
>>>> +    struct ureg_program *ureg = tx->ureg;
>>>> +    struct ureg_dst dst = tx_dst_param(tx, &tx->insn.dst[0]);
>>>> +    struct ureg_src src = tx_src_param(tx, &tx->insn.src[0]);
>>>> +    struct ureg_dst tmp = tx_scratch(tx);
>>>> +    ureg_RSQ(ureg, tmp, ureg_abs(src));
>>>> +    ureg_MIN(ureg, dst, ureg_imm1f(ureg, FLT_MAX), ureg_src(tmp));
>>>
>>> When would this MIN not return the value in tmp? In the description
>>> you say that RSQ(0.0) should return FLT_MAX... is the theory that
>>> MIN(NaN, FLT_MAX) == FLT_MAX? Is RSQ(0) in tgsi defined to return NaN?
>>
>> No, because legacy versions of MIN and MAX aren't commutative. To
>> clarify this more, the legacy versions of MIN/MAX return x if either
>> operand is NaN (non-commutative), while DX10 versions return the one
>> that isn't NaN (commutative). For compatibility with the legacy
>> versions, the FLT_MAX constant must be in x.
>>
> 
> Is it documented anywhere how min/max and other tgsi comparison instructions
> are supposed to handle NaN and +-inf?  If not, we really need to get this
> documented somewhere.
> 

Well, llvmpipe certainly assumes it is d3d10 behavior (I thought this
would also be ok for glsl, though apparently if taking the wording at
face value and this isn't a spec bug it's not). Interestingly, opencl
leaves this undefined, though I'm not really sure what the problem with
infinites is?
This is probably what newer hardware does. Older ones might not have
NaNs in which case it wouldn't be a problem.
I wouldn't really have a problem with documenting this NaN behavior as a
requirement, though since this goes against the current glsl spec it
might involve extra work everywhere for no good reason (if you don't
care about d3d10). Generally though NaN (or just basic floating point)
behavior is not really specified in tgsi right now, there's just some
expectation that if your driver supports "new enough" features things
generally behave in accordance to ieee754-2008. But clearly, the same
opcodes are used for old hardware - obviously r300 fragment operations
are anything but ieee754 compliant, so there can't really be a hard
requirement of the operations being in accordance to that - not unless
we have some means of specifying that explicitly in tgsi (be it shader
properties or whatever).

Roland