Bug#946836: Build failure on powerpc architectures

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

Sebastien Bacher-2

Package: r-base
Version: 3.6.2-1

The current upload failed to build on powerpc
https://buildd.debian.org/status/fetch.php?pkg=r-base&arch=powerpc&ver=3.6.2-1&stamp=1576151048&raw=0

gcc -std=gnu99 -I../../src/extra  -I. -I../../src/include -I../../src/include   -I../../src/nmath -DHAVE_CONFIG_H   -fopenmp -fpic  -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -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[4]: *** [../../Makeconf:124: arithmetic.o] Error 1

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

Dirk Eddelbuettel

Hi Sebastian,

On 16 December 2019 at 13:52, Sebastien Bacher wrote:
| Package: r-base
| Version: 3.6.2-1
|
| The current upload failed to build on powerpc
| https://buildd.debian.org/status/fetch.php?pkg=r-base&arch=powerpc&ver=3.6.2-1&stamp=1576151048&raw=0
|
| gcc -std=gnu99 -I../../src/extra  -I. -I../../src/include -I../../src/include   -I../../src/nmath -DHAVE_CONFIG_H   -fopenmp -fpic  -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -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[4]: *** [../../Makeconf:124: arithmetic.o] Error 1

Our "colleague" at RH/FC noticed the same and sent a patch to the r-devel
list (which use an #ifdef for the architecture). CC an upstream dev as well.

One open question is whether 1.0L / LDBL_EPSILON would do.

Can you remind if/how I could get shell access to PowerPC to test?

Dirk

--
http://dirk.eddelbuettel.com | @eddelbuettel | [hidden email]

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

Sebastien Bacher-2
Le 16/12/2019 à 15:23, Dirk Eddelbuettel a écrit :
> Can you remind if/how I could get shell access to PowerPC to test?

https://wiki.debian.org/PorterBoxHowToUse should have the details needed

Seems like the ppc boxes have restricted access though...

I can test a patch/package on an Ubuntu porter box if that's useful

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

Martin Maechler-2
Thank you, Dirk, for keeping me in the loop.  I'm happy if you test to
see if the change works.

As a matter of fact, I think a slightly different patch should be used
rather than the one proposed.
Given what I've learned in the meantime, I think I'd  rather use

Index: src/main/arithmetic.c
===================================================================
--- src/main/arithmetic.c    (Revision 77566)
+++ src/main/arithmetic.c    (Arbeitskopie)
@@ -176,8 +176,12 @@
 #endif
 }

+
 #if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE > SIZEOF_DOUBLE)
-static LDOUBLE q_1_eps = 1 / LDBL_EPSILON;
+# ifndef __PPC64__
+static // PowerPC 64 (when gcc has -mlong-double-128) breaks when
'static' (gcc bug, there)
+# endif
+       LDOUBLE q_1_eps = 1 / LDBL_EPSILON;
 #else
 static double  q_1_eps = 1 / DBL_EPSILON;
 #endif


which would keep PowerPC 64 to use the high precision epsilon here,
rather than the one to use when there is no long double present.
Martin

On Mon, Dec 16, 2019 at 3:43 PM Sebastien Bacher <[hidden email]> wrote:

>
> Le 16/12/2019 à 15:23, Dirk Eddelbuettel a écrit :
> > Can you remind if/how I could get shell access to PowerPC to test?
>
> https://wiki.debian.org/PorterBoxHowToUse should have the details needed
>
> Seems like the ppc boxes have restricted access though...
>
> I can test a patch/package on an Ubuntu porter box if that's useful
>


--
Martin <[hidden email]>  https://stat.ethz.ch/~maechler
Seminar für Statistik, ETH Zürich  HG G 16      Rämistrasse 101
CH-8092 Zurich, SWITZERLAND
phone: +41-44-632-3408       fax: ...-1228      <><

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

Martin Maechler-2
On Mon, Dec 16, 2019 at 8:29 PM Martin Maechler
<[hidden email]> wrote:

>
> Thank you, Dirk, for keeping me in the loop.  I'm happy if you test to
> see if the change works.
>
> As a matter of fact, I think a slightly different patch should be used
> rather than the one proposed.
> Given what I've learned in the meantime, I think I'd  rather use
>
> Index: src/main/arithmetic.c
> ===================================================================
> --- src/main/arithmetic.c    (Revision 77566)
> +++ src/main/arithmetic.c    (Arbeitskopie)
> @@ -176,8 +176,12 @@
>  #endif
>  }
>
> +
>  #if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE > SIZEOF_DOUBLE)
> -static LDOUBLE q_1_eps = 1 / LDBL_EPSILON;
> +# ifndef __PPC64__
> +static // PowerPC 64 (when gcc has -mlong-double-128) breaks when
> 'static' (gcc bug, there)
> +# endif
> +       LDOUBLE q_1_eps = 1 / LDBL_EPSILON;
>  #else
>  static double  q_1_eps = 1 / DBL_EPSILON;
>  #endif
>
>
> which would keep PowerPC 64 to use the high precision epsilon here,
> rather than the one to use when there is no long double present.
> Martin

Oops, no!
Tomas the expert has explained me why that would not be good enough,
instead recommending a macro  (which would be "constant folded" i.e.,
computed at compile time were available);
If I conditionalize that .. in order to be back compatible outside of
PPC_64,  I'd get

Index: src/main/arithmetic.c
===================================================================
--- src/main/arithmetic.c    (Revision 77583)
+++ src/main/arithmetic.c    (Arbeitskopie)
@@ -176,8 +176,14 @@
 #endif
 }

+
 #if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE > SIZEOF_DOUBLE)
+# ifdef __PPC64__
+ // PowerPC 64 (when gcc has -mlong-double-128) breaks here ...
+#  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

-------------
The above may work correctly  (I tried a version where I replaced
__PPC64__  by  1    such as to test with the gcc 9.2 (on my
architecture).
I'd be glad if you could confirm for  powerPC 64 ..
then I think we could commit it.

Martin

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

Dirk Eddelbuettel

On 16 December 2019 at 22:04, Martin Maechler wrote:
| Oops, no!
| Tomas the expert has explained me why that would not be good enough,
| instead recommending a macro  (which would be "constant folded" i.e.,
| computed at compile time were available);
| If I conditionalize that .. in order to be back compatible outside of
| PPC_64,  I'd get
|
| Index: src/main/arithmetic.c
| ===================================================================
| --- src/main/arithmetic.c    (Revision 77583)
| +++ src/main/arithmetic.c    (Arbeitskopie)
| @@ -176,8 +176,14 @@
|  #endif
|  }
|
| +
|  #if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE > SIZEOF_DOUBLE)
| +# ifdef __PPC64__
| + // PowerPC 64 (when gcc has -mlong-double-128) breaks here ...
| +#  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
|
| -------------
| The above may work correctly  (I tried a version where I replaced
| __PPC64__  by  1    such as to test with the gcc 9.2 (on my
| architecture).
| I'd be glad if you could confirm for  powerPC 64 ..
| then I think we could commit it.

Nice. I applied that, and prepare a 'local' package.  Sebastien, could you
test it to be sure?

Files are here:

   http://dirk.eddelbuettel.com/tmp/r-base/

As usual you can probably ignore the .deb files but it was simpler / cleaner
to copy the whole batch.

Dirk

--
http://dirk.eddelbuettel.com | @eddelbuettel | [hidden email]

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

Sebastien Bacher-2
Hey Dirk,

Le 16/12/2019 à 22:55, Dirk Eddelbuettel a écrit :
> Nice. I applied that, and prepare a 'local' package.  Sebastien, could you
> test it to be sure?

I've tested your package in a ppa and it builds fine on all
architectures, including ppc, seems good to go to me :-)

Thanks!

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

Martin Maechler-2
>>>>> Sebastien Bacher
>>>>>     on Tue, 17 Dec 2019 09:44:11 +0100 writes:

    > Hey Dirk, Le 16/12/2019 à 22:55, Dirk Eddelbuettel a
    > écrit :
    >> Nice. I applied that, and prepare a 'local' package.
    >> Sebastien, could you test it to be sure?

    > I've tested your package in a ppa and it builds fine on
    > all architectures, including ppc, seems good to go to me
    > :-)

    > Thanks!

And thank you, Sebastien, Dirk, and Tomas.

I've committed the  "ifdef PPC64  macro" version to R's sources,
both in the development trunk *and* in the R-3-6 series
(which IIUC should apply to Debian's  R-3.6.2-x package),
in svn rev 77587 and 77588, respectively.

Martin

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

John Paul Adrian Glaubitz
In reply to this post by Sebastien Bacher-2
(Re-sent as the bug report had to be unarchived first)

Control: reopen -1
User: [hidden email]
Usertags: powerpc
X-Debbugs-CC: [hidden email]

Hello!

This bug is still present [1], not sure why this bug report was closed.

A Debian powerpc/ppc64 porterbox is available, no restrictions apply [2],
it's available to all Debian Developers and anyone with a Debian guest
account.

The GCC compile farm also has PowerPC boxes available [3], any open source
developer can request access to a PowerPC for free [4].

It would be nice if this bug could be fixed as the missing r-base package
is blocking a lot of other packages on powerpc.

PS: Please always set the proper usertags and CC the architecture mailing list
    when filing architecture-specific bugs.

Thanks,
Adrian

> [1] https://buildd.debian.org/status/fetch.php?pkg=r-base&arch=powerpc&ver=3.6.3-1&stamp=1583981629&raw=0
> [2] https://db.debian.org/machines.cgi?host=perotto
> [3] https://gcc.gnu.org/wiki/CompileFarm
> [4] https://cfarm.tetaneutral.net/users/new/

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

John Paul Adrian Glaubitz
Hi!

On 3/24/20 8:01 PM, John Paul Adrian Glaubitz wrote:
> It would be nice if this bug could be fixed as the missing r-base package
> is blocking a lot of other packages on powerpc.

This can be easily fixed by replacing __PPC64__ with __powerpc__ which is defined
on all PowerPC architectures (ppc64el, ppc64, powerpc).

Suggestion to change the patch into:

 #if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE > SIZEOF_DOUBLE)
+# ifdef __powerpc__
+ // PowerPC (when gcc has -mlong-double-128) breaks here ...
+#  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

Proof that __powerpc__ is available on all three PowerPC targets.

On powerpc architecture:

(sid-powerpc-sbuild2)root@kapitsa:/# gcc -dM -E - < /dev/null|grep __powerpc__
#define __powerpc__ 1
(sid-powerpc-sbuild2)root@kapitsa:/#

On ppc64 architecture:

(sid-ppc64-sbuild2)root@kapitsa:/# gcc -dM -E - < /dev/null|grep __powerpc__
#define __powerpc__ 1
(sid-ppc64-sbuild2)root@kapitsa:/#

On ppc64el architecture:

(sid_ppc64el-dchroot)glaubitz@plummer:~$ gcc -dM -E - < /dev/null|grep __powerpc__
#define __powerpc__ 1
(sid_ppc64el-dchroot)glaubitz@plummer:~$

Thanks,
Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

Dirk Eddelbuettel
In reply to this post by John Paul Adrian Glaubitz

Tomas, Martin,

The issue is still the long double.  The log pointed to by John reveals

gcc -std=gnu99 -I../../src/extra  -I. -I../../src/include -I../../src/include   -I../../src/nmath -DHAVE_CONFIG_H   -fopenmp -fpic  -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -c arithmetic.c -o arithmetic.o
arithmetic.c:185:26: error: initializer element is not constant
  185 | static LDOUBLE q_1_eps = 1 / LDBL_EPSILON;
      |                          ^
make[4]: *** [../../Makeconf:124: arithmetic.o] Error 1

And I thought we had that licked?  Any suggestions about what to try next?

In r-devel I see

#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

which is the same as what we have in R 3.6.3. So unless fixed, R 4.0.0 will
also fail :-/

My bad for noticing this earlier, and thanks to John for reopening the ticket.

Dirk

--
http://dirk.eddelbuettel.com | @eddelbuettel | [hidden email]

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

John Paul Adrian Glaubitz
On 3/24/20 8:32 PM, Dirk Eddelbuettel wrote:

> In r-devel I see
>
> #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
>
> which is the same as what we have in R 3.6.3. So unless fixed, R 4.0.0 will
> also fail :-/

See my other mail. Just use __powerpc__ instead of __PPC64__ and
adjust the comment.

Uploading a manually built package for powerpc in the meantime, so don't
be confused it shows as fixed.

Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

Martin Maechler-2
In reply to this post by John Paul Adrian Glaubitz
>>>>> John Paul Adrian Glaubitz
>>>>>     on Tue, 24 Mar 2020 20:32:14 +0100 writes:

    > Hi!
    > On 3/24/20 8:01 PM, John Paul Adrian Glaubitz wrote:
    >> It would be nice if this bug could be fixed as the missing r-base package
    >> is blocking a lot of other packages on powerpc.

    > This can be easily fixed by replacing __PPC64__ with __powerpc__ which is defined
    > on all PowerPC architectures (ppc64el, ppc64, powerpc).

Thanks a lot, Adrian!

{ When I had added  __PPC64__ it was because it "seemed clear"
  (by the OP, I don't have access to these architectures) that the
  clause should only be used for powerpc architectures where
    -mlong-double-128
  is available / in used .. and so I would have "concluded"
  would only apply to 64-bit pppc ..
}

But it seems using the macro instead of the static variable is
safer (albeit possibly somewhat slower when the code is run, but
that will probably also depend much on the compilation
optimizations ..).

I will change to use  __powerpc__ now   and wait some hours (on
a possible reply from you !) before committing the change to the
upstream sources.

Martin


    > Suggestion to change the patch into:

    > #if HAVE_LONG_DOUBLE && (SIZEOF_LONG_DOUBLE > SIZEOF_DOUBLE)
    > +# ifdef __powerpc__
    > + // PowerPC (when gcc has -mlong-double-128) breaks here ...
    > +#  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

    > Proof that __powerpc__ is available on all three PowerPC targets.

    > On powerpc architecture:

    > (sid-powerpc-sbuild2)root@kapitsa:/# gcc -dM -E - < /dev/null|grep __powerpc__
    > #define __powerpc__ 1
    > (sid-powerpc-sbuild2)root@kapitsa:/#

    > On ppc64 architecture:

    > (sid-ppc64-sbuild2)root@kapitsa:/# gcc -dM -E - < /dev/null|grep __powerpc__
    > #define __powerpc__ 1
    > (sid-ppc64-sbuild2)root@kapitsa:/#

    > On ppc64el architecture:

    > (sid_ppc64el-dchroot)glaubitz@plummer:~$ gcc -dM -E - < /dev/null|grep __powerpc__
    > #define __powerpc__ 1
    > (sid_ppc64el-dchroot)glaubitz@plummer:~$

    > Thanks,
    > Adrian

    > --
    > .''`.  John Paul Adrian Glaubitz
    > : :' :  Debian Developer - [hidden email]
    > `. `'   Freie Universitaet Berlin - [hidden email]
    > `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

John Paul Adrian Glaubitz
Hi Martin!

On 3/25/20 9:27 AM, Martin Maechler wrote:
> Thanks a lot, Adrian!
>
> { When I had added  __PPC64__ it was because it "seemed clear"
>   (by the OP, I don't have access to these architectures) that the
>   clause should only be used for powerpc architectures where
>     -mlong-double-128
>   is available / in used .. and so I would have "concluded"
>   would only apply to 64-bit pppc ..
> }

As mentioned in my earlier mail, you can get free access to a POWER
machine (and many other architectures) through the GCC compile farm,
you just need to apply for an account.

> https://gcc.gnu.org/wiki/CompileFarm

> But it seems using the macro instead of the static variable is
> safer (albeit possibly somewhat slower when the code is run, but
> that will probably also depend much on the compilation
> optimizations ..).
>
> I will change to use  __powerpc__ now   and wait some hours (on
> a possible reply from you !) before committing the change to the
> upstream sources.

I can only say that the workaround is needed on 32-bit PowerPC
as well, so it should match against "__powerpc__" and not just
"__PPC64__", the former is defined on all PowerPC targets.

Thanks for fixing the issue upstream, much appreciated.

Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Reply | Threaded
Open this post in threaded view
|

Bug#946836: Build failure on powerpc architectures

Dirk Eddelbuettel

On 25 March 2020 at 09:45, John Paul Adrian Glaubitz wrote:
| Thanks for fixing the issue upstream, much appreciated.

Seconded. I'll make a fixed release of 3.6.3 with an updated patch as well,
just in case.

Dirk

--
http://dirk.eddelbuettel.com | @eddelbuettel | [hidden email]