soft-fp64/fsat: Correctly handle NaN
authorIan Romanick <ian.d.romanick@intel.com>
Tue, 3 Mar 2020 02:38:11 +0000 (18:38 -0800)
committerMarge Bot <eric+marge@anholt.net>
Wed, 18 Mar 2020 20:36:29 +0000 (20:36 +0000)
fsat is defined as min(max(a, 0.0), 1.0), and IEEE defines both min and
max to return the non-NaN value when one value is NaN.  Based on this,
fsat should definitely return 0.0 for NaN.

Results on the 308 shaders extracted from the fp64 portion of the OpenGL
CTS:

Tiger Lake and Ice Lake had similar results. (Tiger Lake shown)
total instructions in shared programs: 841666 -> 841647 (<.01%)
instructions in affected programs: 122033 -> 122014 (-0.02%)
helped: 7
HURT: 0
helped stats (abs) min: 1 max: 4 x̄: 2.71 x̃: 3
helped stats (rel) min: 0.01% max: 0.02% x̄: 0.02% x̃: 0.01%
95% mean confidence interval for instructions value: -3.74 -1.69
95% mean confidence interval for instructions %-change: -0.02% -0.01%
Instructions are helped.

total cycles in shared programs: 6927246 -> 6926904 (<.01%)
cycles in affected programs: 1038987 -> 1038645 (-0.03%)
helped: 7
HURT: 0
helped stats (abs) min: 18 max: 72 x̄: 48.86 x̃: 54
helped stats (rel) min: 0.03% max: 0.05% x̄: 0.03% x̃: 0.03%
95% mean confidence interval for cycles value: -67.38 -30.33
95% mean confidence interval for cycles %-change: -0.05% -0.02%
Cycles are helped.

Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Fixes: a42163cbbc1 ("compiler: Add lowering support for 64-bit saturate operations to software")
Closes: https://gitlab.freedesktop.org/mesa/mesa/issues/2585
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4142>

src/compiler/glsl/float64.glsl

index 6dd85e5cc57bf092544d3670a2c3dff5b94c796d..363248c9f593d85cc587e39feaa4ba1f29dff945 100644 (file)
@@ -258,10 +258,11 @@ __fge64(uint64_t a, uint64_t b)
 uint64_t
 __fsat64(uint64_t __a)
 {
-   if (__flt64(__a, 0ul))
+   /* fsat(NaN) should be zero. */
+   if (__is_nan(__a) || __flt64_nonnan(__a, 0ul))
       return 0ul;
 
-   if (__fge64(__a, 0x3FF0000000000000ul /* 1.0 */))
+   if (!__flt64_nonnan(__a, 0x3FF0000000000000ul /* 1.0 */))
       return 0x3FF0000000000000ul;
 
    return __a;