From: Juan A. Suarez Romero <jasuarez@igalia.com>
Date: Tue, 10 Mar 2020 10:49:42 +0000 (+0000)
Subject: nir/lower_double_ops: relax lower mod()
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=acd0dd3b4b223a423fbe9ffd118c3fbbf119d993;p=mesa.git

nir/lower_double_ops: relax lower mod()

Currently when lowering mod() we add an extra instruction so if
mod(a,b) == b then 0 is returned instead of b, as mathematically
mod(a,b) is in the interval [0, b).

But Vulkan spec has relaxed this restriction, and allows the result to
be in the interval [0, b].

For the OpenGL case, while the spec does not allow this behaviour, due
the allowed precision errors we can end up having the same result, so
from a practical point of view, this behaviour is allowed (see
https://github.com/KhronosGroup/VK-GL-CTS/issues/51).

This commit takes this in account to remove the extra instruction
required to return 0 instead.

Reviewed-by: Daniel Schürmann <daniel@schuermann.dev>
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4118>
---

diff --git a/src/compiler/nir/nir_lower_double_ops.c b/src/compiler/nir/nir_lower_double_ops.c
index e0617211d45..bb2476523ec 100644
--- a/src/compiler/nir/nir_lower_double_ops.c
+++ b/src/compiler/nir/nir_lower_double_ops.c
@@ -426,19 +426,32 @@ lower_mod(nir_builder *b, nir_ssa_def *src0, nir_ssa_def *src1)
     *
     * If the division is lowered, it could add some rounding errors that make
     * floor() to return the quotient minus one when x = N * y. If this is the
-    * case, we return zero because mod(x, y) output value is [0, y).
+    * case, we should return zero because mod(x, y) output value is [0, y).
+    * But fortunately Vulkan spec allows this kind of errors; from Vulkan
+    * spec, appendix A (Precision and Operation of SPIR-V instructions:
     *
-    * Worth to note that Vulkan allows the output value to be in range [0, y],
-    * so mod(x, y) could return y; but as OpenGL does not allow this, we add
-    * the extra instruction to ensure the value is always in [0, y).
+    *   "The OpFRem and OpFMod instructions use cheap approximations of
+    *   remainder, and the error can be large due to the discontinuity in
+    *   trunc() and floor(). This can produce mathematically unexpected
+    *   results in some cases, such as FMod(x,x) computing x rather than 0,
+    *   and can also cause the result to have a different sign than the
+    *   infinitely precise result."
+    *
+    * In practice this means the output value is actually in the interval
+    * [0, y].
+    *
+    * While Vulkan states this behaviour explicitly, OpenGL does not, and thus
+    * we need to assume that value should be in range [0, y); but on the other
+    * hand, mod(a,b) is defined as "a - b * floor(a/b)" and OpenGL allows for
+    * some error in division, so a/a could actually end up being 1.0 - 1ULP;
+    * so in this case floor(a/a) would end up as 0, and hence mod(a,a) == a.
+    *
+    * In summary, in the practice mod(a,a) can be "a" both for OpenGL and
+    * Vulkan.
     */
    nir_ssa_def *floor = nir_ffloor(b, nir_fdiv(b, src0, src1));
-   nir_ssa_def *mod = nir_fsub(b, src0, nir_fmul(b, src1, floor));
 
-   return nir_bcsel(b,
-                    nir_fne(b, mod, src1),
-                    mod,
-                    nir_imm_double(b, 0.0));
+   return nir_fsub(b, src0, nir_fmul(b, src1, floor));
 }
 
 static nir_ssa_def *