[Rd] Build failure on powerpc64

[Tom Callaway]

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 <pdalgd using 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 <maechler using 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 using 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.mes using cbs.dk  Priv: PDalgd using gmail.com
>
>
>
>
>
>
>
>
>
>

	[[alternative HTML version deleted]]