An excellent question. It is important to remember two key facts:
1. With gcc on ppc64, long doubles exist, they can be used, just not safely
as constants (and maybe they still can be used safely under specific
conditions?).
2. I am not an expert in either PowerPC64 or gcc. :)
Looking at connections.c, we have (in order):
* handling long double as a valid case in a switch statement checking size
* adding long double as a field in the u union define
* handling long double as a valid case in a switch statement checking size
* handling long double as a valid case in a switch statement checking size
* memcpy from the address of a long double
In format.c, we have (in order):
* conditionally creating private_nearbyintl for R_nearbyintl
* defining a static const long double tbl[]
* use exact scaling factor in long double precision
For most of these, it seems safe to leave them as is for ppc64. I would
have thought that the gcc compiler might have had issue with:
connections.c:
static long double ld1;
for (i = 0, j = 0; i < len; i++, j += size) {
ld1 = (long double) REAL(object)[i];
format.c:
static const long double tbl[] =
... but it doesn't. Perhaps the original code at issue:
arithmetic.c:
static LDOUBLE q_1_eps = 1 / LDBL_EPSILON;
only makes gcc unhappy because of the very large value trying to be stored
in the static long double, which would make it span the "folded double" on
that architecture.
*****
It seems that the options are:
A) Patch the one place where the compiler determines it is not safe to use
a static long double on ppc64.
B) Treat PPC64 as a platform where it is never safe to use a static long
double
FWIW, I did run the test suite after applying my patch and all of the tests
pass on ppc64.
Tom
On Fri, Dec 13, 2019 at 5:44 AM Martin Maechler <maech...@stat.math.ethz.ch>
wrote:
> >>>>> Tom Callaway
> >>>>> on Thu, 12 Dec 2019 14:21:10 -0500 writes:
>
> > Hi R folks,
>
> > Went to build R 3.6.2 for Fedora/EPEL and got failures across the
> board.
>
> > Disabling the test suite for all non-intel architectures resolves
> most of
> > the failures, but powerpc64 dies in the compiler, specifically here:
>
> > gcc -m64 -I../../src/extra/xdr -I. -I../../src/include
> -I../../src/include
> > -I/usr/local/include -I../../src/nmath -DHAVE_CONFIG_H -fopenmp
> -fPIC
> > -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
> > -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong
> > -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
> > -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mcpu=power8
> > -mtune=power8 -fasynchronous-unwind-tables -fstack-clash-protection
> -c
> > arithmetic.c -o arithmetic.o
> > arithmetic.c:180:26: error: initializer element is not constant
> > 180 | static LDOUBLE q_1_eps = (1 / LDBL_EPSILON);
> > | ^
> > make[3]: *** [../../Makeconf:124: arithmetic.o] Error 1
>
> > Took me a bit to figure out why, but this is happening because on
> > powerpc64, gcc is compiled with -mlong-double-128, and the long
> double
> > format used on PPC is IBM's 128bit long double which is two doubles
> added
> > together. As a result, gcc can't safely do const assignments to long
> > doubles on ppc64, so it dies there.
>
> > The fix is easy enough, do not try to assign a value to a static long
> > double on ppc64.
> > --- ./src/main/arithmetic.c.orig 2019-12-12
> 18:30:12.416334062 +0000
> > +++ ./src/main/arithmetic.c 2019-12-12 18:30:44.966334062 +0000
> > @@ -179,7 +179,10 @@ void attribute_hidden InitArithmetic()
> > #endif
> > }
>
> > -#if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE > SIZEOF_DOUBLE)
> > +/* PowerPC 64 (when gcc has -mlong-double-128) breaks here because
> > + * of issues constant folding 128bit IBM long doubles.
> > + */
> > +#if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE > SIZEOF_DOUBLE) &&
> !__PPC64__
> > static LDOUBLE q_1_eps = 1 / LDBL_EPSILON;
> > #else
> > static double q_1_eps = 1 / DBL_EPSILON;
>
>
> > Hope that helps someone else.
> > Tom
>
> Thank you, Tom. That is "interesting" ...
>
> The piece in question had been added by me,
> ------------------------------------------------------------------------
> r77193 | maechler | 2019-09-18 13:21:49 +0200 (Wed, 18 Sep 2019) | 1 line
>
> x %% +/- Inf now typically return non-NaN; fix the "FIXME" in C
> ------------------------------------------------------------------------
> in order to use double precision in order to deal with finite cases
> close to Inf, etc.
>
> IIUC, your proposed patch is really a workaround a bug on PPC64 ?
>
> But note the check on LONG_DOUBLE is not the only one in R's
> sources: Via 'grep' in src/main/*.? I also see
>
> connections.c 4285:#if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE >
> SIZEOF_DOUBLE)
> connections.c 4329:#if HAVE_LONG_DOUBLE
> connections.c 4379:#if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE >
> SIZEOF_DOUBLE)
> connections.c 4514:#if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE >
> SIZEOF_DOUBLE)
> connections.c 4592:#if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE >
> SIZEOF_DOUBLE)
> format.c 250:#if defined(HAVE_LONG_DOUBLE) && (SIZEOF_LONG_DOUBLE >
> SIZEOF_DOUBLE)
> format.c 285:#if defined(HAVE_LONG_DOUBLE) && (SIZEOF_LONG_DOUBLE >
> SIZEOF_DOUBLE)
> format.c 339:#if defined(HAVE_LONG_DOUBLE) && (SIZEOF_LONG_DOUBLE >
> SIZEOF_DOUBLE)
>
> Do they also need protection against this PPC64 bug ?
>
> Best,
>
> Martin Maechler
> ETH Zurich and R Core Team
>
[[alternative HTML version deleted]]
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
