>>>>> Martin Maechler
>>>>> on Tue, 17 Dec 2019 11:25:31 +0100 writes:
>>>>> Tom Callaway
>>>>> on Fri, 13 Dec 2019 11:06:25 -0500 writes:
>> 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
> Thank you, Tom.
> You were right... and only A) is needed.
> In the mean time I've also been CC'ed in a corresponding debian
> bug report on the exact same architecture.
> In the end, after explanation and recommendation by Tomas
> Kalibera, I've committed a slightly better change to R's
> sources, both in the R-devel (trunk) and the "R-3.6.x patched"
> branch: Via a macro, it continues to use long double also for
> the PPC 64 in this case:
> $ svn diff -c77587
> Index: src/main/arithmetic.c
> ===================================================================
> --- src/main/arithmetic.c (Revision 77586)
> +++ src/main/arithmetic.c (Revision 77587)
> @@ -176,8 +176,14 @@
> #endif
> }
> +
> #if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE > SIZEOF_DOUBLE)
> +# ifdef __PPC64__
> + // PowerPC 64 (when gcc has -mlong-double-128) fails constant folding
with LDOUBLE
> +# define q_1_eps (1 / LDBL_EPSILON)
> +# else
> static LDOUBLE q_1_eps = 1 / LDBL_EPSILON;
> +# endif
> #else
> static double q_1_eps = 1 / DBL_EPSILON;
> #endif
> ------------- ------------- -------------
Now, Debian Bug#946836 has been reopened,
because __PPC64__ does not cover all powerpc architectures
and in their build farm they found non-PPC64 cases which also
needed to switch from a static variable to a macro,
the suggestion being to replace __PPC64__ by __powerpc__
which is what I'm going to commit now (for R-3.6.x patched and
for R-devel !).
I hope that these macros are +- universally working and not too
much depending on the exact compiler flavor.
Martin Maechler
ETH Zurich and R Core team
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
