This change seems correct, but as we don't have any non-PPC64 systems or build targets in the Fedora buildsystem, it won't affect Fedora if it doesn't make it.
Tom On Wed, Mar 25, 2020, 6:16 AM peter dalgaard <pda...@gmail.com> wrote: > Do note that 3.6-patched will only be live for a day or two as we branch > for 4.0.0 on Friday. Anything committed there is unlikely to make it into > an official release (in principle, the 3.6 branch can be revived but it > would take a very strong incentive to do so.) > > If you want an R-3.6.3-for-ppc, I think a vendor patch is the way. AFAIR > (it's been more than a decade since I looked at this stuff) the RPM spec > files make it fairly easy to apply changes to the sources before building. > > -pd > > > On 25 Mar 2020, at 10:00 , Martin Maechler <maech...@stat.math.ethz.ch> > wrote: > > > >>>>>> 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 > > -- > Peter Dalgaard, Professor, > Center for Statistics, Copenhagen Business School > Solbjerg Plads 3, 2000 Frederiksberg, Denmark > Phone: (+45)38153501 > Office: A 4.23 > Email: pd....@cbs.dk Priv: pda...@gmail.com > > > > > > > > > > [[alternative HTML version deleted]] ______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel