diff mbox series

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

Message ID 87k0mr6mae.fsf_-_@gmail.com
State Accepted
Headers show
Series [bug#48825] gnu: Simplify the use of --with-long-double-128 on powerpc64le. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Christopher Marusich June 18, 2021, 6:49 p.m. UTC
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.

Comments

Christopher Marusich June 25, 2021, 3:47 a.m. UTC | #1
Committed in 45dd2b4505095d24e253bd62d74474cad135cf3b.
diff mbox series

Patch

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