[bug#48825] [PATCH] gnu: Simplify the use of --with-long-double-128 on powerpc64le.

[bug#48825] [PATCH] gnu: Simplify the use of --with-long-double-128 on powerpc64le.

[Chris Marusich]

[-- Attachment #1.1: Type: text/plain, Size: 1257 bytes --]

Hi,

Currently in Guix, we explicitly specify --with-long-double-128 on
powerpc64le-linux (and potentially other powerpc64-* systems) in two
places.  It turns out that this is not necessary.  This patch simplifies
our code.

I originally authored this in order to try fixing bug 47698, but it
didn't solve that problem, and it turns out the --with-long-double-128
option is probably unrelated to that bug.  However, it's still worth
making this change to simplify our code.

If nobody has any issues with this, then in 2 weeks I will commit it.
One question is: where should I commit it?  I'd like to commit it to
master, but it causes many rebuilds on powerpc64le-linux (it does not
cause rebuilds on any other platforms), so per the guidelines ((guix)
Submitting Patches) I ought to commit it to core-updates.  However,
because the glibc upgrade from 2.31 to 2.32 on core-updates causes many
problems for powerpc64le-linux, core-updates has never once been a
viable branch for powerpc64le-linux in the time since support was first
added on master.  So if I commit this patch to core-updates, it won't
really do anyone any good right now.  With all this in mind, I think
master is the right place to commit this patch.

-- 
Chris

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-gnu-Simplify-the-use-of-with-long-double-128-on-powe.patch --]
[-- Type: text/x-patch, Size: 4484 bytes --]

From ad89f9f59d22cc10fbf7dd6f738ce15a6e79b640 Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Sat, 10 Apr 2021 18:16:17 -0700
Subject: [PATCH] gnu: Simplify the use of --with-long-double-128 on
 powerpc64le.

In short, this change adds the "--with-long-double-128" configure option in
one place and removes it from two other (now-redundant) places.  It does not
cause any rebuilds on systems other than powerpc64le-linux.

* gnu/packages/gcc.scm (gcc-configure-flags-for-triplet): Add a clause for
targets starting with "powerpc64le-" which adds the "--with-long-double-128"
option.  This causes any package using this procedure to be built using this
new option on powerpc64le systems.  In particular, this affects the gcc
package and the gcc-final package, in addition to all the other versions of
GCC defined in (gnu packages gcc).
* gnu/packages/commencement.scm (gcc-boot0)[#:configure-flags]: Remove the
code that adds the "--with-long-double-128" configure option for powerpc64le,
since it is now redundant. The gcc-boot0 package uses (and adds to) the gcc
package's configure options. This means that the above change in gcc.scm is
sufficient to ensure that the gcc-boot0 package's configure options will
include "--with-long-double-128" on powerpc64le systems.
* gnu/packages/cross-base.scm (cross-gcc-arguments)[#:configure-flags]: Remove
the code that adds the "--with-long-double-128" configure option for
powerpc64le, since it is now redundant. The cross-gcc-arguments procedure
uses (and adds to) the configure options of its xgcc argument (a package).
This means that regardless of which gcc from gcc.scm is used as the xgcc, the
above change in gcc.scm is sufficient to ensure that the cross-gcc-arguments
procedure's configure options will include "--with-long-double-128" on
powerpc64le systems.
---
 gnu/packages/commencement.scm | 7 -------
 gnu/packages/cross-base.scm   | 6 ------
 gnu/packages/gcc.scm          | 3 +++
 3 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index d4511ed914..db564db9c4 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -2819,13 +2819,6 @@ exec " gcc "/bin/" program
                            "--disable-shared"
                            "--enable-languages=c,c++"
 
-                           ;; boot-triplet inserts "guix" in the triplet.
-                           ,@(if (equal? "powerpc64le-guix-linux-gnu" (boot-triplet))
-                                 ;; On POWER9 (little endian) glibc needs the
-                                 ;; 128-bit long double type.
-                                 '("--with-long-double-128")
-                                 '())
-
                            ;; libstdc++ cannot be built at this stage
                            ;; ("Link tests are not allowed after
                            ;; GCC_NO_EXECUTABLES.").
diff --git a/gnu/packages/cross-base.scm b/gnu/packages/cross-base.scm
index 180594509b..c1e5f2eb79 100644
--- a/gnu/packages/cross-base.scm
+++ b/gnu/packages/cross-base.scm
@@ -153,12 +153,6 @@ base compiler and using LIBC (which may be either a libc package or #f.)"
                                "--disable-decimal-float" ;would need libc
                                "--disable-libcilkrts"
 
-                              ,@(if (string-prefix? "powerpc64le-" target)
-                                   ;; On POWER9 (little endian) glibc needs
-                                   ;; the 128-bit long double type.
-                                   '("--with-long-double-128")
-                                   '())
-
                                ;; When target is any OS other than 'none' these
                                ;; libraries will fail if there is no libc
                                ;; present. See
diff --git a/gnu/packages/gcc.scm b/gnu/packages/gcc.scm
index a412c93c29..22a0f35422 100644
--- a/gnu/packages/gcc.scm
+++ b/gnu/packages/gcc.scm
@@ -79,6 +79,9 @@ where the OS part is overloaded to denote a specific ABI---into GCC
          ;; Cilk has been removed from GCC 8 anyway.
          '("--disable-libcilkrts"))
 
+        ((string-prefix? "powerpc64le-" target)
+         '("--with-long-double-128"))
+
         (else
          ;; TODO: Add `arm.*-gnueabi', etc.
          '())))
-- 
2.30.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

[bug#48825] [PATCH] gnu: Simplify the use of --with-long-double-128 on powerpc64le.

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 6829 bytes --]

On Thu, Jun 03, 2021 at 10:08:00PM -0700, Chris Marusich wrote:
> Hi,
> 
> Currently in Guix, we explicitly specify --with-long-double-128 on
> powerpc64le-linux (and potentially other powerpc64-* systems) in two
> places.  It turns out that this is not necessary.  This patch simplifies
> our code.
> 
> I originally authored this in order to try fixing bug 47698, but it
> didn't solve that problem, and it turns out the --with-long-double-128
> option is probably unrelated to that bug.  However, it's still worth
> making this change to simplify our code.
> 
> If nobody has any issues with this, then in 2 weeks I will commit it.
> One question is: where should I commit it?  I'd like to commit it to
> master, but it causes many rebuilds on powerpc64le-linux (it does not
> cause rebuilds on any other platforms), so per the guidelines ((guix)
> Submitting Patches) I ought to commit it to core-updates.  However,
> because the glibc upgrade from 2.31 to 2.32 on core-updates causes many
> problems for powerpc64le-linux, core-updates has never once been a
> viable branch for powerpc64le-linux in the time since support was first
> added on master.  So if I commit this patch to core-updates, it won't
> really do anyone any good right now.  With all this in mind, I think
> master is the right place to commit this patch.
> 
> -- 
> Chris

couple of thoughts:
powerpc64le is in 'technology preview', so IMO it's fine to make big
changes to it as needed.

On master (and probably on core-updates too) we have a patch for glibc
to force ... something (that I don't remember) on powerpc architectures,
which has the side effect of needing '--with-long-double-128' on
powerpc-linux also in commencement.scm. If we could drop that patch then
I don't think we would need it anymore for powerpc.

> From ad89f9f59d22cc10fbf7dd6f738ce15a6e79b640 Mon Sep 17 00:00:00 2001
> From: Chris Marusich <cmmarusich@gmail.com>
> Date: Sat, 10 Apr 2021 18:16:17 -0700
> Subject: [PATCH] gnu: Simplify the use of --with-long-double-128 on
>  powerpc64le.
> 
> In short, this change adds the "--with-long-double-128" configure option in
> one place and removes it from two other (now-redundant) places.  It does not
> cause any rebuilds on systems other than powerpc64le-linux.
> 
> * gnu/packages/gcc.scm (gcc-configure-flags-for-triplet): Add a clause for
> targets starting with "powerpc64le-" which adds the "--with-long-double-128"
> option.  This causes any package using this procedure to be built using this
> new option on powerpc64le systems.  In particular, this affects the gcc
> package and the gcc-final package, in addition to all the other versions of
> GCC defined in (gnu packages gcc).
> * gnu/packages/commencement.scm (gcc-boot0)[#:configure-flags]: Remove the
> code that adds the "--with-long-double-128" configure option for powerpc64le,
> since it is now redundant. The gcc-boot0 package uses (and adds to) the gcc
> package's configure options. This means that the above change in gcc.scm is
> sufficient to ensure that the gcc-boot0 package's configure options will
> include "--with-long-double-128" on powerpc64le systems.
> * gnu/packages/cross-base.scm (cross-gcc-arguments)[#:configure-flags]: Remove
> the code that adds the "--with-long-double-128" configure option for
> powerpc64le, since it is now redundant. The cross-gcc-arguments procedure
> uses (and adds to) the configure options of its xgcc argument (a package).
> This means that regardless of which gcc from gcc.scm is used as the xgcc, the
> above change in gcc.scm is sufficient to ensure that the cross-gcc-arguments
> procedure's configure options will include "--with-long-double-128" on
> powerpc64le systems.
> ---
>  gnu/packages/commencement.scm | 7 -------
>  gnu/packages/cross-base.scm   | 6 ------
>  gnu/packages/gcc.scm          | 3 +++
>  3 files changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
> index d4511ed914..db564db9c4 100644
> --- a/gnu/packages/commencement.scm
> +++ b/gnu/packages/commencement.scm
> @@ -2819,13 +2819,6 @@ exec " gcc "/bin/" program
>                             "--disable-shared"
>                             "--enable-languages=c,c++"
>  

I've adjusted this on core-updates to also take effect on powerpc-linux.

> -                           ;; boot-triplet inserts "guix" in the triplet.
> -                           ,@(if (equal? "powerpc64le-guix-linux-gnu" (boot-triplet))
> -                                 ;; On POWER9 (little endian) glibc needs the
> -                                 ;; 128-bit long double type.
> -                                 '("--with-long-double-128")
> -                                 '())
> -
>                             ;; libstdc++ cannot be built at this stage
>                             ;; ("Link tests are not allowed after
>                             ;; GCC_NO_EXECUTABLES.").
> diff --git a/gnu/packages/cross-base.scm b/gnu/packages/cross-base.scm
> index 180594509b..c1e5f2eb79 100644
> --- a/gnu/packages/cross-base.scm
> +++ b/gnu/packages/cross-base.scm
> @@ -153,12 +153,6 @@ base compiler and using LIBC (which may be either a libc package or #f.)"
>                                 "--disable-decimal-float" ;would need libc
>                                 "--disable-libcilkrts"
>  
> -                              ,@(if (string-prefix? "powerpc64le-" target)
> -                                   ;; On POWER9 (little endian) glibc needs
> -                                   ;; the 128-bit long double type.
> -                                   '("--with-long-double-128")
> -                                   '())
> -
>                                 ;; When target is any OS other than 'none' these
>                                 ;; libraries will fail if there is no libc
>                                 ;; present. See
> diff --git a/gnu/packages/gcc.scm b/gnu/packages/gcc.scm
> index a412c93c29..22a0f35422 100644
> --- a/gnu/packages/gcc.scm
> +++ b/gnu/packages/gcc.scm
> @@ -79,6 +79,9 @@ where the OS part is overloaded to denote a specific ABI---into GCC
>           ;; Cilk has been removed from GCC 8 anyway.
>           '("--disable-libcilkrts"))
>  

This can be just 'powerpc64le'

> +        ((string-prefix? "powerpc64le-" target)
> +         '("--with-long-double-128"))
> +
>          (else
>           ;; TODO: Add `arm.*-gnueabi', etc.
>           '())))
> -- 
> 2.30.2
> 




-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#48825] [PATCH] gnu: Simplify the use of --with-long-double-128 on powerpc64le.

[Chris Marusich]

[-- Attachment #1.1: Type: text/plain, Size: 2270 bytes --]

Hi Efraim,

Efraim Flashner <efraim@flashner.co.il> writes:

> couple of thoughts:
> powerpc64le is in 'technology preview', so IMO it's fine to make big
> changes to it as needed.

That's true, but since we now have a stable place from which to work
(master), I feel less concerned about getting changes into master.  I
thought about this again now, and I think I will plan to push this
change to core-updates instead of master.

> On master (and probably on core-updates too) we have a patch for glibc
> to force ... something (that I don't remember) on powerpc architectures,
> which has the side effect of needing '--with-long-double-128' on
> powerpc-linux also in commencement.scm. If we could drop that patch then
> I don't think we would need it anymore for powerpc.

I'm not sure what change this would be, but if you ever figure it out,
please do let me know!

>> diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
>> index d4511ed914..db564db9c4 100644
>> --- a/gnu/packages/commencement.scm
>> +++ b/gnu/packages/commencement.scm
>> @@ -2819,13 +2819,6 @@ exec " gcc "/bin/" program
>>                             "--disable-shared"
>>                             "--enable-languages=c,c++"
>>  
>
> I've adjusted this on core-updates to also take effect on powerpc-linux.

Thank you for mentioning this.  In light of your change, I needed to
modify my patch.  I've attached a new patch which takes powerpc into
account.  I modified the commit message a bit, too.

I'm confident the attached patch is correct for powerpc64le-linux, but
if you could take a peek and make sure I didn't miss something related
to powerpc, I would appreciate it!

> This can be just 'powerpc64le'

When checking the string prefix, that does probably work for little
endian powerpc 64, since I don't think there are any other architectures
that start with "powerpc64le".  However, if you did something similar
for powerpc (like in your change on core-updates), please keep in mind
that it will affect not only powerpc64le, but also big-endian powerpc64,
and any other powerpc architecture.  For that reason, I personally
prefer to keep the hyphen when I have a specific architecture in mind.

-- 
Chris

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: [PATCH] gnu: Simplify the use of --with-long-double-128. --]
[-- Type: text/x-patch, Size: 4938 bytes --]

From 1d6043ea33236a7f35f990935e457ef440b365c4 Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Fri, 18 Jun 2021 11:26:31 -0700
Subject: [PATCH] gnu: Simplify the use of --with-long-double-128.

In short, this change adds the hard-coded "--with-long-double-128" configure
option in one place and removes it from two other places.  This changes and
simplifies the use of this option for various architectures that start with
the string "powerpc".

* gnu/packages/gcc.scm (gcc-configure-flags-for-triplet): Add a clause for
targets starting with "powerpc64le-" or "powerpc-" which adds the
"--with-long-double-128" option.  This causes any package using this procedure
to be built using this new option on these architectures.  In particular, this
affects the gcc package and the gcc-final package, in addition to all the
other versions of GCC defined in (gnu packages gcc).
* gnu/packages/commencement.scm (gcc-boot0)[#:configure-flags]: Remove the
code that adds the "--with-long-double-128" configure option for all
architectures starting with "powerpc", since it is now redundant on the
architectures where it is needed. The gcc-boot0 package uses (and adds to) the
gcc package's configure options. This means that the above change in gcc.scm
is sufficient to ensure that the gcc-boot0 package's configure options will
include "--with-long-double-128" on powerpc64le and powerpc architectures.
Additionally, since the option is apparently not required on the big-endian
powerpc64 architecture, this change also has the nice effect of omitting the
option in that case.
* gnu/packages/cross-base.scm (cross-gcc-arguments)[#:configure-flags]: Remove
the code that adds the "--with-long-double-128" configure option for
powerpc64le, since it is now redundant. The cross-gcc-arguments procedure uses
(and adds to) the configure options of its xgcc argument (a package).  This
means that regardless of which gcc from gcc.scm is used as the xgcc, the above
change in gcc.scm is sufficient to ensure that the cross-gcc-arguments
procedure's configure options will include "--with-long-double-128" on the
powerpc64le and powerpc architectures.
---
 gnu/packages/commencement.scm | 7 -------
 gnu/packages/cross-base.scm   | 6 ------
 gnu/packages/gcc.scm          | 5 +++++
 3 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index d44d1dd3ca..af61512129 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -2733,13 +2733,6 @@ exec " gcc "/bin/" program
                            "--disable-shared"
                            "--enable-languages=c,c++"
 
-                           ;; On POWER9 (little endian) glibc needs the 128-bit
-                           ;; long double type.  32-bit PPC is affected by the
-                           ;; changes applied for powerpc64le.
-                           ,@(if (string-prefix? "powerpc" (boot-triplet))
-                               '("--with-long-double-128")
-                               '())
-
                            ;; libstdc++ cannot be built at this stage
                            ;; ("Link tests are not allowed after
                            ;; GCC_NO_EXECUTABLES.").
diff --git a/gnu/packages/cross-base.scm b/gnu/packages/cross-base.scm
index 926b00ccdf..ced226ef34 100644
--- a/gnu/packages/cross-base.scm
+++ b/gnu/packages/cross-base.scm
@@ -153,12 +153,6 @@ base compiler and using LIBC (which may be either a libc package or #f.)"
                                "--disable-decimal-float" ;would need libc
                                "--disable-libcilkrts"
 
-                              ,@(if (string-prefix? "powerpc64le-" target)
-                                   ;; On POWER9 (little endian) glibc needs
-                                   ;; the 128-bit long double type.
-                                   '("--with-long-double-128")
-                                   '())
-
                                ;; When target is any OS other than 'none' these
                                ;; libraries will fail if there is no libc
                                ;; present. See
diff --git a/gnu/packages/gcc.scm b/gnu/packages/gcc.scm
index 5d114dca87..31c7997fd0 100644
--- a/gnu/packages/gcc.scm
+++ b/gnu/packages/gcc.scm
@@ -79,6 +79,11 @@ where the OS part is overloaded to denote a specific ABI---into GCC
          ;; Cilk has been removed from GCC 8 anyway.
          '("--disable-libcilkrts"))
 
+        ;; glibc needs the 128-bit long double type on these architectures.
+        ((or (string-prefix? "powerpc64le-" target)
+             (string-prefix? "powerpc-" target))
+         '("--with-long-double-128"))
+
         (else
          ;; TODO: Add `arm.*-gnueabi', etc.
          '())))
-- 
2.30.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

bug#48825: [PATCH] gnu: Simplify the use of --with-long-double-128 on powerpc64le.

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 72 bytes --]


Committed in 45dd2b4505095d24e253bd62d74474cad135cf3b.

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]