Mesa (staging/21.1): tgsi_exec: Fix NaN behavior of min and max

[GitLab Mirror]

Module: Mesa
Branch: staging/21.1
Commit: 924affef2696b2c375a9e3ff80ff9ad6cd187acf
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=924affef2696b2c375a9e3ff80ff9ad6cd187acf

Author: Ian Romanick <ian.d.romanick at intel.com>
Date:   Thu Apr 22 17:46:39 2021 -0700

tgsi_exec: Fix NaN behavior of min and max

Modern shader APIs, like DX10 and GLSL 1.30, want min() and max() to
"cleanse" NaN.  If one source is NaN, the other value should be chosen.
If both sources are NaN, the result may be either.

There are many cases where TGSI is generate from NIR, and many
optimizations in NIR expect this behavior.  Not meeting these
expectations can lead to unexpected results.

Reviewed-by: Eric Anholt <eric at anholt.net>
Fixes: ffe58739da9 ("Softpipe: import TGSI tree. Not hooked-up yet.")
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10419>
(cherry picked from commit 939bf7a4198cecff57510b7fe5d38ef9b5dd22be)

---

 .pick_status.json                                  |  2 +-
 src/gallium/auxiliary/tgsi/tgsi_exec.c             | 32 +++++++++++-----------
 src/gallium/drivers/softpipe/ci/softpipe-quick.txt |  4 ---
 3 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index 1b698215fa3..dff69294592 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -67,7 +67,7 @@
         "description": "tgsi_exec: Fix NaN behavior of min and max",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "master_sha": null,
         "because_sha": "ffe58739da9eee2e99682747cc8f26e412c87430"
     },
diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c b/src/gallium/auxiliary/tgsi/tgsi_exec.c
index b4c6f60f888..fbfb04ef360 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
@@ -263,20 +263,20 @@ static void
 micro_dmax(union tgsi_double_channel *dst,
            const union tgsi_double_channel *src)
 {
-   dst->d[0] = src[0].d[0] > src[1].d[0] ? src[0].d[0] : src[1].d[0];
-   dst->d[1] = src[0].d[1] > src[1].d[1] ? src[0].d[1] : src[1].d[1];
-   dst->d[2] = src[0].d[2] > src[1].d[2] ? src[0].d[2] : src[1].d[2];
-   dst->d[3] = src[0].d[3] > src[1].d[3] ? src[0].d[3] : src[1].d[3];
+   dst->d[0] = src[0].d[0] > src[1].d[0] || isnan(src[1].d[0]) ? src[0].d[0] : src[1].d[0];
+   dst->d[1] = src[0].d[1] > src[1].d[1] || isnan(src[1].d[1]) ? src[0].d[1] : src[1].d[1];
+   dst->d[2] = src[0].d[2] > src[1].d[2] || isnan(src[1].d[2]) ? src[0].d[2] : src[1].d[2];
+   dst->d[3] = src[0].d[3] > src[1].d[3] || isnan(src[1].d[3]) ? src[0].d[3] : src[1].d[3];
 }
 
 static void
 micro_dmin(union tgsi_double_channel *dst,
            const union tgsi_double_channel *src)
 {
-   dst->d[0] = src[0].d[0] < src[1].d[0] ? src[0].d[0] : src[1].d[0];
-   dst->d[1] = src[0].d[1] < src[1].d[1] ? src[0].d[1] : src[1].d[1];
-   dst->d[2] = src[0].d[2] < src[1].d[2] ? src[0].d[2] : src[1].d[2];
-   dst->d[3] = src[0].d[3] < src[1].d[3] ? src[0].d[3] : src[1].d[3];
+   dst->d[0] = src[0].d[0] < src[1].d[0] || isnan(src[1].d[0]) ? src[0].d[0] : src[1].d[0];
+   dst->d[1] = src[0].d[1] < src[1].d[1] || isnan(src[1].d[1]) ? src[0].d[1] : src[1].d[1];
+   dst->d[2] = src[0].d[2] < src[1].d[2] || isnan(src[1].d[2]) ? src[0].d[2] : src[1].d[2];
+   dst->d[3] = src[0].d[3] < src[1].d[3] || isnan(src[1].d[3]) ? src[0].d[3] : src[1].d[3];
 }
 
 static void
@@ -1357,10 +1357,10 @@ micro_max(union tgsi_exec_channel *dst,
           const union tgsi_exec_channel *src0,
           const union tgsi_exec_channel *src1)
 {
-   dst->f[0] = src0->f[0] > src1->f[0] ? src0->f[0] : src1->f[0];
-   dst->f[1] = src0->f[1] > src1->f[1] ? src0->f[1] : src1->f[1];
-   dst->f[2] = src0->f[2] > src1->f[2] ? src0->f[2] : src1->f[2];
-   dst->f[3] = src0->f[3] > src1->f[3] ? src0->f[3] : src1->f[3];
+   dst->f[0] = src0->f[0] > src1->f[0] || isnan(src1->f[0]) ? src0->f[0] : src1->f[0];
+   dst->f[1] = src0->f[1] > src1->f[1] || isnan(src1->f[1]) ? src0->f[1] : src1->f[1];
+   dst->f[2] = src0->f[2] > src1->f[2] || isnan(src1->f[2]) ? src0->f[2] : src1->f[2];
+   dst->f[3] = src0->f[3] > src1->f[3] || isnan(src1->f[3]) ? src0->f[3] : src1->f[3];
 }
 
 static void
@@ -1368,10 +1368,10 @@ micro_min(union tgsi_exec_channel *dst,
           const union tgsi_exec_channel *src0,
           const union tgsi_exec_channel *src1)
 {
-   dst->f[0] = src0->f[0] < src1->f[0] ? src0->f[0] : src1->f[0];
-   dst->f[1] = src0->f[1] < src1->f[1] ? src0->f[1] : src1->f[1];
-   dst->f[2] = src0->f[2] < src1->f[2] ? src0->f[2] : src1->f[2];
-   dst->f[3] = src0->f[3] < src1->f[3] ? src0->f[3] : src1->f[3];
+   dst->f[0] = src0->f[0] < src1->f[0] || isnan(src1->f[0]) ? src0->f[0] : src1->f[0];
+   dst->f[1] = src0->f[1] < src1->f[1] || isnan(src1->f[1]) ? src0->f[1] : src1->f[1];
+   dst->f[2] = src0->f[2] < src1->f[2] || isnan(src1->f[2]) ? src0->f[2] : src1->f[2];
+   dst->f[3] = src0->f[3] < src1->f[3] || isnan(src1->f[3]) ? src0->f[3] : src1->f[3];
 }
 
 static void
diff --git a/src/gallium/drivers/softpipe/ci/softpipe-quick.txt b/src/gallium/drivers/softpipe/ci/softpipe-quick.txt
index 45de3631706..9bebc9b5f73 100644
--- a/src/gallium/drivers/softpipe/ci/softpipe-quick.txt
+++ b/src/gallium/drivers/softpipe/ci/softpipe-quick.txt
@@ -2691,10 +2691,6 @@ spec/glsl-1.10/execution/fs-dfdx-accuracy: warn
 spec/glsl-1.10/execution/fs-dfdy-accuracy: warn
 spec/glsl-1.10/preprocessor/extension-defined-test: skip
 spec/glsl-1.10/preprocessor/extension-if-1: skip
-spec/glsl-1.20/execution/fs-nan-builtin-max: fail
-spec/glsl-1.20/execution/fs-nan-builtin-min: fail
-spec/glsl-1.20/execution/vs-nan-builtin-max: fail
-spec/glsl-1.20/execution/vs-nan-builtin-min: fail
 spec/glsl-1.30/execution/tex-miplevel-selection texturegrad 1d: fail
 spec/glsl-1.30/execution/tex-miplevel-selection texturegrad 1darray: fail
 spec/glsl-1.30/execution/tex-miplevel-selection texturegrad 1darrayshadow: fail