[PATCH] remainder: Port from amd builtins

jvesely · March 12, 2018, 2:28am

Mostly ported form amd_builtins, uses only denormal path for fp32.
Passes piglit on turks
Passes CTS on carrizo

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>

jvesely · March 12, 2018, 10:14pm

Mostly ported form amd_builtins, uses only denormal path for fp32.
Passes piglit on turks

this one actually passes CTS on turks as well (needs single-threaded
invocation).

Jan

Aaron_Watry · March 12, 2018, 10:42pm

Cc

Mostly ported form amd_builtins, uses only denormal path for fp32.
Passes piglit on turks

this one actually passes CTS on turks as well (needs single-threaded
invocation).

Speaking of, do you object to removing the compute memory pool in r600g since its the source of the threading issues?

–Aaron

jvesely · March 13, 2018, 12:03am

Cc

> > Mostly ported form amd_builtins, uses only denormal path for fp32.
> > Passes piglit on turks
>
> this one actually passes CTS on turks as well (needs single-threaded
> invocation).
>
>

Speaking of, do you object to removing the compute memory pool in r600g
since its the source of the threading issues?

I don't think we really have a viable alternative. I think it'd just be
easier to add proper locking around the pool manipulation.

Jan

Aaron_Watry · March 13, 2018, 12:20am

Cc

Mostly ported form amd_builtins, uses only denormal path for fp32.
Passes piglit on turks

this one actually passes CTS on turks as well (needs single-threaded
invocation).

Speaking of, do you object to removing the compute memory pool in r600g
since its the source of the threading issues?

I don’t think we really have a viable alternative. I think it’d just be
easier to add proper locking around the pool manipulation.

Yeah, the existing implementation has multiple fundamental issues that prevent it from working with multiple contexts built from the same pipe_screen. We’d have to rebuild it as a per-context pool at a minimum. I have some local patches where I started trying to do just that, but ran into some problems that made me think it might be easier to scrap the existing implementation until we can rewrite it, if it’s even worth rewriting.

–Aaron

jvesely · March 13, 2018, 2:06am

> > Cc
> >
> >
> > > > Mostly ported form amd_builtins, uses only denormal path for fp32.
> > > > Passes piglit on turks
> > >
> > > this one actually passes CTS on turks as well (needs single-threaded
> > > invocation).
> > >
> > >
> >
> > Speaking of, do you object to removing the compute memory pool in r600g
> > since its the source of the threading issues?
>
> I don't think we really have a viable alternative. I think it'd just be
> easier to add proper locking around the pool manipulation.
>

Yeah, the existing implementation has multiple fundamental issues that
prevent it from working with multiple contexts built from the same
pipe_screen. We'd have to rebuild it as a per-context pool at a minimum. I
have some local patches where I started trying to do just that, but ran
into some problems that made me think it might be easier to scrap the
existing implementation until we can rewrite it, if it's even worth
rewriting.

I'm not sure what the intermediate situation would be. We need the
memory pool to support multiple buffers. Sacrificing that to support
multiple threads/contexts is imo not worth it. Applications that use
multiple buffers are more common.
Jan

jvesely · March 14, 2018, 4:02pm

>
> > > Cc
> > >
> > >
> > > > > Mostly ported form amd_builtins, uses only denormal path for fp32.
> > > > > Passes piglit on turks
> > > >
> > > > this one actually passes CTS on turks as well (needs single-threaded
> > > > invocation).
> > > >
> > > >
> > >
> > > Speaking of, do you object to removing the compute memory pool in r600g
> > > since its the source of the threading issues?
> >
> > I don't think we really have a viable alternative. I think it'd just be
> > easier to add proper locking around the pool manipulation.
> >
>
> Yeah, the existing implementation has multiple fundamental issues that
> prevent it from working with multiple contexts built from the same
> pipe_screen. We'd have to rebuild it as a per-context pool at a minimum. I
> have some local patches where I started trying to do just that, but ran
> into some problems that made me think it might be easier to scrap the
> existing implementation until we can rewrite it, if it's even worth
> rewriting.

I'm not sure what the intermediate situation would be. We need the
memory pool to support multiple buffers. Sacrificing that to support
multiple threads/contexts is imo not worth it. Applications that use
multiple buffers are more common.

hm, running integer_ops with more than one worker also hangs on GCN, so
there might be some issues with clover, that would help EG as well. I
haven't investigated, though.

Jan

jvesely · March 18, 2018, 7:26pm

ping

Aaron_Watry · March 19, 2018, 12:16am

Mostly ported form amd_builtins, uses only denormal path for fp32.
Passes piglit on turks
Passes CTS on carrizo

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>

generic/include/clc/clc.h | 1 +
generic/include/clc/math/remainder.h | 4 +
generic/include/math/clc_remainder.h | 4 +
generic/lib/SOURCES | 2 +
generic/lib/math/clc_remainder.cl | 218 +++++++++++++++++++++++++++++++++++
generic/lib/math/remainder.cl | 6 +
6 files changed, 235 insertions(+)
create mode 100644 generic/include/clc/math/remainder.h
create mode 100644 generic/include/math/clc_remainder.h
create mode 100644 generic/lib/math/clc_remainder.cl
create mode 100644 generic/lib/math/remainder.cl

diff --git a/generic/include/clc/clc.h b/generic/include/clc/clc.h
index 36ab134…75ca7d0 100644
— a/generic/include/clc/clc.h
+++ b/generic/include/clc/clc.h
@@ -103,6 +103,7 @@
#include <clc/math/pow.h>
#include <clc/math/pown.h>
#include <clc/math/powr.h>
+#include <clc/math/remainder.h>
#include <clc/math/rint.h>
#include <clc/math/rootn.h>
#include <clc/math/round.h>
diff --git a/generic/include/clc/math/remainder.h b/generic/include/clc/math/remainder.h
new file mode 100644
index 0000000…5b5d78d
— /dev/null
+++ b/generic/include/clc/math/remainder.h
@@ -0,0 +1,4 @@
+#define __CLC_FUNCTION remainder
+#define __CLC_BODY <clc/math/binary_decl_tt.inc>
+#include <clc/math/gentype.inc>
+#undef __CLC_FUNCTION
diff --git a/generic/include/math/clc_remainder.h b/generic/include/math/clc_remainder.h
new file mode 100644
index 0000000…db084cf
— /dev/null
+++ b/generic/include/math/clc_remainder.h
@@ -0,0 +1,4 @@
+#define __CLC_FUNCTION __clc_remainder
+#define __CLC_BODY <clc/math/binary_decl_tt.inc>
+#include <clc/math/gentype.inc>
+#undef __CLC_FUNCTION
diff --git a/generic/lib/SOURCES b/generic/lib/SOURCES
index 9c060ed…159950c 100644
— a/generic/lib/SOURCES
+++ b/generic/lib/SOURCES
@@ -158,6 +158,8 @@ math/clc_pown.cl
math/pown.cl
math/clc_powr.cl
math/powr.cl
+math/clc_remainder.cl
+math/remainder.cl
math/clc_rootn.cl
math/rootn.cl
math/sin.cl
diff --git a/generic/lib/math/clc_remainder.cl b/generic/lib/math/clc_remainder.cl
new file mode 100644
index 0000000…ba50ee3
— /dev/null
+++ b/generic/lib/math/clc_remainder.cl
@@ -0,0 +1,218 @@
+/*

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.

*/

+#include <clc/clc.h>
+
+#include <math/clc_remainder.h>
+#include “…/clcmacro.h”
+#include “config.h”
+#include “math.h”
+
+_CLC_DEF _CLC_OVERLOAD float __clc_remainder(float x, float y)
+{

int ux = as_int(x);

int ax = ux & EXSIGNBIT_SP32;

float xa = as_float(ax);

int sx = ux ^ ax;

int ex = ax >> EXPSHIFTBITS_SP32;

int uy = as_int(y);

int ay = uy & EXSIGNBIT_SP32;

float ya = as_float(ay);

int ey = ay >> EXPSHIFTBITS_SP32;

float xr = as_float(0x3f800000 | (ax & 0x007fffff));

float yr = as_float(0x3f800000 | (ay & 0x007fffff));

int c;

int k = ex - ey;

uint q = 0;

while (k > 0) {

c = xr >= yr;

q = (q << 1) | c;

xr -= c ? yr : 0.0f;

xr += xr;

–k;

}

c = xr > yr;

q = (q << 1) | c;

xr -= c ? yr : 0.0f;

int lt = ex < ey;

q = lt ? 0 : q;

xr = lt ? xa : xr;

yr = lt ? ya : yr;

c = (yr < 2.0f * xr) | ((yr == 2.0f * xr) & ((q & 0x1) == 0x1));

xr -= c ? yr : 0.0f;

q += c;

float s = as_float(ey << EXPSHIFTBITS_SP32);

xr *= lt ? 1.0f : s;

c = ax == ay;

xr = c ? 0.0f : xr;

xr = as_float(sx ^ as_int(xr));

c = ax > PINFBITPATT_SP32 | ay > PINFBITPATT_SP32 | ax == PINFBITPATT_SP32 | ay == 0;

xr = c ? as_float(QNANBITPATT_SP32) : xr;

return xr;

+}
+_CLC_BINARY_VECTORIZE(_CLC_DEF _CLC_OVERLOAD, float, __clc_remainder, float, float);
+
+#ifdef cl_khr_fp64
+_CLC_DEF _CLC_OVERLOAD double __clc_remainder(double x, double y)
+{

ulong ux = as_ulong(x);

ulong ax = ux & ~SIGNBIT_DP64;

ulong xsgn = ux ^ ax;

double dx = as_double(ax);

int xexp = convert_int(ax >> EXPSHIFTBITS_DP64);

int xexp1 = 11 - (int) clz(ax & MANTBITS_DP64);

xexp1 = xexp < 1 ? xexp1 : xexp;

ulong uy = as_ulong(y);

ulong ay = uy & ~SIGNBIT_DP64;

double dy = as_double(ay);

int yexp = convert_int(ay >> EXPSHIFTBITS_DP64);

int yexp1 = 11 - (int) clz(ay & MANTBITS_DP64);

yexp1 = yexp < 1 ? yexp1 : yexp;

int qsgn = ((ux ^ uy) & SIGNBIT_DP64) == 0UL ? 1 : -1;

// First assume |x| > |y|

// Set ntimes to the number of times we need to do a

// partial remainder. If the exponent of x is an exact multiple

// of 53 larger than the exponent of y, and the mantissa of x is

// less than the mantissa of y, ntimes will be one too large

// but it doesn’t matter - it just means that we’ll go round

// the loop below one extra time.

int ntimes = max(0, (xexp1 - yexp1) / 53);

double w = ldexp(dy, ntimes * 53);

w = ntimes == 0 ? dy : w;

double scale = ntimes == 0 ? 1.0 : 0x1.0p-53;

// Each time round the loop we compute a partial remainder.

// This is done by subtracting a large multiple of w

// from x each time, where w is a scaled up version of y.

// The subtraction must be performed exactly in quad

// precision, though the result at each stage can

// fit exactly in a double precision number.

int i;

double t, v, p, pp;

for (i = 0; i < ntimes; i++) {

// Compute integral multiplier

t = trunc(dx / w);

// Compute w * t in quad precision

p = w * t;

pp = fma(w, t, -p);

// Subtract w * t from dx

v = dx - p;

dx = v + (((dx - v) - p) - pp);

// If t was one too large, dx will be negative. Add back one w.

dx += dx < 0.0 ? w : 0.0;

// Scale w down by 2^(-53) for the next iteration

w *= scale;

}

// One more time

// Variable todd says whether the integer t is odd or not

t = floor(dx / w);

long lt = (long)t;

int todd = lt & 1;

p = w * t;

pp = fma(w, t, -p);

v = dx - p;

dx = v + (((dx - v) - p) - pp);

i = dx < 0.0;

todd ^= i;

dx += i ? w : 0.0;

// At this point, dx lies in the range [0,dy)

// For the fmod function, we’re done apart from setting the correct sign.

//

// For the remainder function, we need to adjust dx

// so that it lies in the range (-y/2, y/2] by carefully

// subtracting w (== dy == y) if necessary. The rigmarole

// with todd is to get the correct sign of the result

// when x/y lies exactly half way between two integers,

// when we need to choose the even integer.

int al = (2.0dx > w) | (todd & (2.0dx == w));

double dxl = dx - (al ? w : 0.0);

int ag = (dx > 0.5w) | (todd & (dx == 0.5w));

double dxg = dx - (ag ? w : 0.0);

dx = dy < 0x1.0p+1022 ? dxl : dxg;

double ret = as_double(xsgn ^ as_ulong(dx));

dx = as_double(ax);

// Now handle |x| == |y|

int c = dx == dy;

t = as_double(xsgn);

ret = c ? t : ret;

// Next, handle |x| < |y|

c = dx < dy;

ret = c ? x : ret;

c &= (yexp < 1023 & 2.0dx > dy) | (dx > 0.5dy);

// we could use a conversion here instead since qsgn = ±1

p = qsgn == 1 ? -1.0 : 1.0;

t = fma(y, p, x);

ret = c ? t : ret;

// We don’t need anything special for |x| == 0

// |y| is 0

c = dy == 0.0;

ret = c ? as_double(QNANBITPATT_DP64) : ret;

// y is ±Inf, NaN

c = yexp > BIASEDEMAX_DP64;

t = y == y ? x : y;

ret = c ? t : ret;

// x is +=Inf, NaN

c = xexp > BIASEDEMAX_DP64;

ret = c ? as_double(QNANBITPATT_DP64) : ret;

return ret;
+}
+_CLC_BINARY_VECTORIZE(_CLC_DEF _CLC_OVERLOAD, double, __clc_remainder, double, double);
+#endif
diff --git a/generic/lib/math/remainder.cl b/generic/lib/math/remainder.cl
new file mode 100644
index 0000000…0a22ee8
— /dev/null
+++ b/generic/lib/math/remainder.cl
@@ -0,0 +1,6 @@
+#include <clc/clc.h>
+#include <math/clc_remainder.h>

+#define __CLC_FUNC remainder
+#define __CLC_BODY <clc_sw_binary.inc>
+#include <clc/math/gentype.inc>

ping

I’ve run this successfully through the 1.2 CTS on Polaris and in single-threaded mode on my Barts.

Glad to hear that this is finally passing CTS, unlike the version that I had sent ~2 years ago (although much has changed since then).

Looks ok to me.

–Aaron