[PATCH] libclc/asin: Switch to amd builtins version of asin

Fixes a wimpy-mode CTS failure for asin(float).

Passes non-wimpy for both float/double on RX580.

Signed-off-by: Aaron Watry <awatry@gmail.com>

Fixes a wimpy-mode CTS failure for asin(float).

Passes non-wimpy for both float/double on RX580.

Signed-off-by: Aaron Watry <awatry@gmail.com>

Nice, thanks!
I presume the failure appeared on llvm-11? the test passes ok on llvm-
10 on my Carrizo machine. Although this patch reduces max ulp from
2.91 to 0.91. It also fixes the same test on Turks.
Tested-by: Jan Vesely <jan.vesely@rutgers.edu>

One small nit below, otherwise
Reviewed-by: Jan Vesely <jan.vesely@rutgers.edu>

---
libclc/generic/lib/math/asin.cl | 168 ++++++++++++++++++++++++++++++-
libclc/generic/lib/math/asin.inc | 18 ----
2 files changed, 166 insertions(+), 20 deletions(-)
delete mode 100644 libclc/generic/lib/math/asin.inc

diff --git a/libclc/generic/lib/math/asin.cl b/libclc/generic/lib/math/asin.cl
index 7581a504093..e07dd298d9c 100644
--- a/libclc/generic/lib/math/asin.cl
+++ b/libclc/generic/lib/math/asin.cl
@@ -1,4 +1,168 @@
+/*
+ * Copyright (c) 2014 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+// TODO: Enable half precision when atan2 is implemented

The above TODO seems redundant since you switched the implementation
away from atan2.

#include <clc/clc.h>

-#define __CLC_BODY <asin.inc>
-#include <clc/math/gentype.inc>
+#include "math.h"
+#include "../clcmacro.h"
+
+_CLC_OVERLOAD _CLC_DEF float asin(float x) {
+ // Computes arcsin(x).
+ // The argument is first reduced by noting that arcsin(x)
+ // is invalid for abs(x) > 1 and arcsin(-x) = -arcsin(x).
+ // For denormal and small arguments arcsin(x) = x to machine
+ // accuracy. Remaining argument ranges are handled as follows.
+ // For abs(x) <= 0.5 use
+ // arcsin(x) = x + x^3*R(x^2)
+ // where R(x^2) is a rational minimax approximation to
+ // (arcsin(x) - x)/x^3.
+ // For abs(x) > 0.5 exploit the identity:
+ // arcsin(x) = pi/2 - 2*arcsin(sqrt(1-x)/2)
+ // together with the above rational approximation, and
+ // reconstruct the terms carefully.
+
+ const float piby2_tail = 7.5497894159e-08F; /* 0x33a22168 */
+ const float hpiby2_head = 7.8539812565e-01F; /* 0x3f490fda */
+ const float piby2 = 1.5707963705e+00F; /* 0x3fc90fdb */
+
+ uint ux = as_uint(x);
+ uint aux = ux & EXSIGNBIT_SP32;
+ uint xs = ux ^ aux;
+ float spiby2 = as_float(xs | as_uint(piby2));
+ int xexp = (int)(aux >> EXPSHIFTBITS_SP32) - EXPBIAS_SP32;
+ float y = as_float(aux);
+
+ // abs(x) >= 0.5
+ int transform = xexp >= -1;
+
+ float y2 = y * y;
+ float rt = 0.5f * (1.0f - y);
+ float r = transform ? rt : y2;
+
+ // Use a rational approximation for [0.0, 0.5]
+ float a = mad(r,
+ mad(r,
+ mad(r, -0.00396137437848476485201154797087F, -0.0133819288943925804214011424456F),
+ -0.0565298683201845211985026327361F),
+ 0.184161606965100694821398249421F);
+
+ float b = mad(r, -0.836411276854206731913362287293F, 1.10496961524520294485512696706F);
+ float u = r * MATH_DIVIDE(a, b);
+
+ float s = MATH_SQRT(r);
+ float s1 = as_float(as_uint(s) & 0xffff0000);
+ float c = MATH_DIVIDE(mad(-s1, s1, r), s + s1);
+ float p = mad(2.0f*s, u, -mad(c, -2.0f, piby2_tail));
+ float q = mad(s1, -2.0f, hpiby2_head);
+ float vt = hpiby2_head - (p - q);
+ float v = mad(y, u, y);
+ v = transform ? vt : v;
+
+ float ret = as_float(xs | as_uint(v));
+ ret = aux > 0x3f800000U ? as_float(QNANBITPATT_SP32) : ret;
+ ret = aux == 0x3f800000U ? spiby2 : ret;
+ ret = xexp < -14 ? x : ret;
+
+ return ret;
+}
+
+_CLC_UNARY_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, float, asin, float);
+
+#ifdef cl_khr_fp64
+
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable
+
+_CLC_OVERLOAD _CLC_DEF double asin(double x) {
+ // Computes arcsin(x).
+ // The argument is first reduced by noting that arcsin(x)
+ // is invalid for abs(x) > 1 and arcsin(-x) = -arcsin(x).
+ // For denormal and small arguments arcsin(x) = x to machine
+ // accuracy. Remaining argument ranges are handled as follows.
+ // For abs(x) <= 0.5 use
+ // arcsin(x) = x + x^3*R(x^2)
+ // where R(x^2) is a rational minimax approximation to
+ // (arcsin(x) - x)/x^3.
+ // For abs(x) > 0.5 exploit the identity:
+ // arcsin(x) = pi/2 - 2*arcsin(sqrt(1-x)/2)
+ // together with the above rational approximation, and
+ // reconstruct the terms carefully.
+
+ const double piby2_tail = 6.1232339957367660e-17; /* 0x3c91a62633145c07 */
+ const double hpiby2_head = 7.8539816339744831e-01; /* 0x3fe921fb54442d18 */
+ const double piby2 = 1.5707963267948965e+00; /* 0x3ff921fb54442d18 */
+
+ double y = fabs(x);
+ int xneg = as_int2(x).hi < 0;
+ int xexp = (as_int2(y).hi >> 20) - EXPBIAS_DP64;
+
+ // abs(x) >= 0.5
+ int transform = xexp >= -1;
+
+ double rt = 0.5 * (1.0 - y);
+ double y2 = y * y;
+ double r = transform ? rt : y2;
+
+ // Use a rational approximation for [0.0, 0.5]
+
+ double un = fma(r,
+ fma(r,
+ fma(r,
+ fma(r,
+ fma(r, 0.0000482901920344786991880522822991,
+ 0.00109242697235074662306043804220),
+ -0.0549989809235685841612020091328),
+ 0.275558175256937652532686256258),
+ -0.445017216867635649900123110649),
+ 0.227485835556935010735943483075);
+
+ double ud = fma(r,
+ fma(r,
+ fma(r,
+ fma(r, 0.105869422087204370341222318533,
+ -0.943639137032492685763471240072),
+ 2.76568859157270989520376345954),
+ -3.28431505720958658909889444194),
+ 1.36491501334161032038194214209);
+
+ double u = r * MATH_DIVIDE(un, ud);
+
+ // Reconstruct asin carefully in transformed region
+ double s = sqrt(r);
+ double sh = as_double(as_ulong(s) & 0xffffffff00000000UL);
+ double c = MATH_DIVIDE(fma(-sh, sh, r), s + sh);
+ double p = fma(2.0*s, u, -fma(-2.0, c, piby2_tail));
+ double q = fma(-2.0, sh, hpiby2_head);
+ double vt = hpiby2_head - (p - q);
+ double v = fma(y, u, y);
+ v = transform ? vt : v;
+
+ v = xexp < -28 ? y : v;
+ v = xexp >= 0 ? as_double(QNANBITPATT_DP64) : v;
+ v = y == 1.0 ? piby2 : v;
+
+ return xneg ? -v : v;
+}
+
+_CLC_UNARY_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, double, asin, double);
+
+#endif // cl_khr_fp64
diff --git a/libclc/generic/lib/math/asin.inc b/libclc/generic/lib/math/asin.inc
deleted file mode 100644
index b08c7bd29ce..00000000000
--- a/libclc/generic/lib/math/asin.inc
+++ /dev/null
@@ -1,18 +0,0 @@
-// TODO: Enable half precision when atan2 is implemented
-#if __CLC_FPSIZE > 16
-
-#if __CLC_FPSIZE == 64
-#define __CLC_CONST(x) x
-#elif __CLC_FPSIZE == 32
-#define __CLC_CONST(x) x ## f
-#elif __CLC_FPSIZE == 16
-#define __CLC_CONST(x) x ## h
-#endif
-
-_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE asin(__CLC_GENTYPE x) {
- return atan2(x, sqrt( (__CLC_GENTYPE)__CLC_CONST(1.0) - (x*x) ));
-}
-
-#undef __CLC_CONST
-
-#endif

Jan

> Fixes a wimpy-mode CTS failure for asin(float).
>
> Passes non-wimpy for both float/double on RX580.
>
> Signed-off-by: Aaron Watry <awatry@gmail.com>

Nice, thanks!
I presume the failure appeared on llvm-11? the test passes ok on llvm-
10 on my Carrizo machine. Although this patch reduces max ulp from
2.91 to 0.91. It also fixes the same test on Turks.
Tested-by: Jan Vesely <jan.vesely@rutgers.edu>

Actually the issue was present on llvm 9.0,0, which is what my current
stack has installed.

I'll get rid of the TODO comment before pushing.

--Aaron

(re-adding libclc-dev)

> > > > Fixes a wimpy-mode CTS failure for asin(float).
> > > >
> > > > Passes non-wimpy for both float/double on RX580.
> > > >
> > > > Signed-off-by: Aaron Watry <awatry@gmail.com>
> > >
> > > Nice, thanks!
> > > I presume the failure appeared on llvm-11? the test passes ok on llvm-
> > > 10 on my Carrizo machine. Although this patch reduces max ulp from
> > > 2.91 to 0.91. It also fixes the same test on Turks.
> > > Tested-by: Jan Vesely <jan.vesely@rutgers.edu>
> > >
> >
> > Actually the issue was present on llvm 9.0,0, which is what my current
> > stack has installed.
>
> That's interesting. I wonder whether it's because of different code
> generated by LLVM or different accuracy of used instructions.
> Does acos pass on your setup?

Yup, acos passes for both float/double.

I do have a crash in cos(double), but I planned on dealing with that separately.

With that done, other than a failure to build some kernel related to a
NOT operation, all of the math_brute_force CTS tests at least pass
wimpy mode for me.

Did you want me to hold off pushing the acos fix for a bit, or treat
any other follow-up work as a separate thing?

No, please push asap.
math_brute_force has been passing for some time on my carrizo setup,
that's why I was surprised to see the failure.
Scalar versions of the tests even pass on Turks/EG, with the exception
of asin/acos and anythin using divide or sqrt.

Jan