[RFC] ppc64le enablement #45340

CrystalGamma · 2018-08-19T15:27:54Z

This PR enables the ppc64le architecture (and, theoretically, also Big Endian ppc64, but it's broken).

Building the ppc64le-glibc bootstrap files was successfully tested on x86-64 and ppc64le-glibc (building them on ppc64le-musl fails because of some doubly-defined symbols).
Building the ppc64le-musl bootstrap files was tested on x86-64, ppc64le-glibc and ppc64le-musl.
This obviously includes building the ppc64le-glibc and ppc64le-musl stdenv.

I have written a short roadmap/overview for what's in the pipe for NixOS/nixpkgs on POWER: https://crystalgamma.github.io/nixpkgs-prs.html .

As for this particular PR, should the changes be broken up into smaller commits or is this fine?

Ericson2314 · 2018-08-20T03:37:59Z

See also #44092, which I had every intention to merge once the remaining issues were fixed. In particular, that mentioned contrary to yours that the big endian variant is in fact a different earlier 64-bit attempt, in practice.

CrystalGamma · 2018-08-20T07:06:04Z

I didn't know a previous PR for POWER existed …

ppc64 is just one bit flip in the MSR (machine status register) away from ppc64le (just like 32-bit mode), so every ppc64le computer could (and I have every intention to at least try) run ppc64 code (even in the same OS if the kernel supports it, which Linux doesn't AFAIK). The author of the previous PR was right in that the firmware of older ppc64 hardware and pretty much all ppc32 hardware could only boot big-endian OSs. In fact OpenPower systems which are generally marketed as ppc64le hardware actually boot in big-endian mode AFAIK, and then switch to whatever the OS is compiled for. That's why I introduced a more general isPower inspection predicate (and endian-specific options can then check isBigEndian or isLittleEndian) for platforms.

I did see a GCC config option (--with-long-double-128) in that PR which could have saved me about 50h of my life had I known about it earlier. Using that allowed me to remove some hackery in glibc. (I can squash before merging, if that's desired)

You may note that this PR also allows you to build a musl stdenv, which the earlier one didn't, AFAICS.

Ericson2314 · 2018-08-20T07:55:20Z

pkgs/development/compilers/gcc/7/default.nix

+    # the float128 detection in cross-gcc and glibc is a little busted. since glibc doesn't compile without float128 support, fix it.
+    sed 's/-mfloat128/-mfloat128-type -mfloat128/g' -i libgcc/configure
+    # this file also includes totally unneeded headers which are not available in cross-compilation. remove them.
+    sed -E 's/#include <(string|stdlib|ctype).h>//' -i libgcc/config/rs6000/float128-ifunc.c


Probably good to put the comment outside the string, so changing the explanation doesn't cause rebuilds.

Makes sense. So like this?

# the float128 detection in cross-gcc and glibc is a little busted. since glibc doesn't compile without float128 support, fix it. + "\nsed 's/-mfloat128/-mfloat128-type -mfloat128/g' -i libgcc/configure" # this file also includes totally unneeded headers which are not available in cross-compilation. remove them. +"\nsed -E 's/#include <(string|stdlib|ctype).h>//' -i libgcc/config/rs6000/float128-ifunc.c"

Anyway, it turns out this hack was also obsoleted by just using --with-long-double-128 --enable-softfloat and I just overlooked it when cleaning up …

Ericson2314 · 2018-08-20T07:55:55Z

pkgs/development/compilers/gcc/7/default.nix

@@ -325,6 +331,10 @@ stdenv.mkDerivation ({
    ]
    ++ optional (targetPlatform == hostPlatform && targetPlatform.libc == "musl") "--disable-libsanitizer"
    ++ optional (targetPlatform.isAarch64) "--enable-fix-cortex-a53-843419"
+    ++ optionals (targetPlatform.isPower && targetPlatform.libc == "glibc") [
+      "--with-long-double-128"
+      "--enable-softfloat"


It would be nice to leverage https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/gcc/common/platform-flags.nix for these, or at least the first.

AFAICS platform-flags only has --with-xyz=abc with a select few 'xyz'. Unless you'd like me to extend it …

(Also, __float128 being required is only for the combination glibc, little endian, 64 bits … I might actually want to check endianness and word size too …)

Ericson2314 · 2018-08-20T07:56:43Z

pkgs/development/compilers/gcc/8/default.nix

+    ++ optionals (targetPlatform.isPower && targetPlatform.libc == "glibc") [
+      "--with-long-double-128"
+      "--enable-softfloat"
+    ]


Don't forget to update pkgs/development/compilers/gcc/snapshot/default.nix too

Will do. BTW, is 'snapshot' supposed to hold any particular version? Or is it just updated whenever someone feels like it?

Yeah it's some arbitrary vcs revision. Should be newer than the rest so it would become the next release version, GCC 9 here.

Ericson2314 · 2018-08-20T07:57:38Z

pkgs/stdenv/default.nix

@@ -47,6 +47,7 @@ in
    "aarch64-linux" = stagesLinux;
    "mipsel-linux" = stagesLinux;
    "powerpc-linux" = /* stagesLinux */ stagesNative;
+    powerpc64le-linux = stagesLinux;


Use " here and other such places for consistency

I think you got them all but the one I commented on, haha :)

CrystalGamma · 2018-08-20T16:15:31Z

So, I made the quotes consistent, removed an obsolete hack and updated the flags for gcc7, gcc8 and gcc-snapshot. I am not certain how to best abstract the flags into the platform definition using gcc/common/platform-flags.nix though.

Ericson2314 · 2018-08-20T17:18:55Z

So for the 128-bit float, I'd add

(lib.optional
  (let tp = targetPlatform; in tp.isPower && tp.libc == "glibc" && tp.is64bit && tp.isLittleEndian)
  "--with-long-double-128")

the the concatentated list in https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/gcc/common/platform-flags.nix#L12

For --enable-softfloat, I'd need to know what that is used for. That is a multilib flag, and our wrapper scripts don't really play well with multilib. [I am working on packaging the GCC runtime libraries separately for a "nix-driven multilib", side-stepping this restriction.] So I think it be better to use --with-float and friends to force float support one way or the other, if possible.

…s.nix

CrystalGamma · 2018-08-20T18:37:37Z

I blindly copied the --enable-softfloat flag from the earlier PR. I assumed it was necessary because I had seen linker errors referencing 128-bit float emulation routines when dabbling in cross-building glibc. Thanks for making me challenge that hypothesis :D

I was thinking that maybe passing the --with-long-double-128 flag to all versions of GCC may cause failures in older versions. But since versions pre-7 didn't support little-endian ppc64 (IIRC), I guess it wouldn't matter either way …

Ericson2314 · 2018-08-20T21:56:43Z

Sounds good! Did no --enable-softfloat work for cross then? Is this ready to merge?

CrystalGamma · 2018-08-21T05:09:13Z

It did. Should I squash these changes before merging?

Ericson2314 · 2018-08-21T06:41:16Z

Glad to hear it! I can just "squash and merge", but if you want to divide things into more than 1 commit or something that's fine too. I'll check this again when I wake up.

CrystalGamma · 2018-08-21T10:12:57Z

No, squashing is fine by me. Thanks for helping me bring support for more niche architectures to NixOS :)

Ericson2314 · 2018-08-21T19:31:25Z

Of course!

dtzWill

Excited to see this landing, here's some feedback. Will read through the discussion/your website post more thoroughly in a bit.

dtzWill · 2018-08-22T15:51:42Z

pkgs/os-specific/linux/musl/default.nix

@@ -61,11 +61,12 @@ stdenv.mkDerivation rec {
    configureFlagsArray+=("--syslibdir=$out/lib")
  '';

+  CFLAGS="-fstack-protector-strong" + lib.optionalString hostPlatform.isPower " -mlong-double-64";


This isn't the same as only setting CFLAGS during configure-- have you checked the output is identical on x86_64? Hopefully doesn't matter but better safe than sorry....

And just curious about why this flag is needed, but I haven't read through the discussion/your website yet so apologies if all this has been addressed.

The rationale here was that since CFLAGS might contain a space now, it might show up as two options, one of which ./configure will not understand.

The original problem is that long double defaults to 128-bits on ppc64le (even without --with-long-double-128 which is required for glibc interestingly) but musl wants 64-bit long double.

dtzWill · 2018-08-22T15:54:50Z

pkgs/stdenv/linux/bootstrap-tools/scripts/unpack-bootstrap-tools.sh

@@ -9,6 +9,9 @@ echo Patching the bootstrap tools...
 if test -f $out/lib/ld.so.?; then
   # MIPS case
   LD_BINARY=$out/lib/ld.so.?
+elif test -f $out/lib/ld64.so.?; then
+   # ppc64(le)
+   LD_BINARY=$out/lib/ld64.so.?


Does the musl unpack script need similar updating? Not sure what it's called in that situation.

It certainly works without any additional changes, so I guess not …

* ppc64le enablement * gcc, glibc: properly handle __float128 * lib/systems, stdenv: syntax cleanup * gcc7: remove ugly hack * gcc: add/update __float128 flags * stdenv: add another pair of quotes for consistency * gcc: move __float128 flag for ppc64le-glibc into common/platform-flags.nix

ppc64le enablement

d6bb7d4

CrystalGamma requested review from Ericson2314 and nbp as code owners August 19, 2018 15:27

GrahamcOfBorg added 6.topic: stdenv Standard environment 10.rebuild-linux-stdenv 10.rebuild-darwin: 0 10.rebuild-linux: 501+ labels Aug 19, 2018

gcc, glibc: properly handle __float128

70a11c6

Ericson2314 reviewed Aug 20, 2018

View reviewed changes

Jona Stubbe added 3 commits August 20, 2018 17:27

lib/systems, stdenv: syntax cleanup

19db7d7

gcc7: remove ugly hack

57c8d7a

gcc: add/update __float128 flags

e7453cb

CrystalGamma force-pushed the ppc-stdenv_2 branch from 1a29c6a to e7453cb Compare August 20, 2018 16:09

Jona Stubbe added 2 commits August 20, 2018 20:27

stdenv: add another pair of quotes for consistency

ac2684c

gcc: move __float128 flag for ppc64le-glibc into common/platform-flag…

b693fed

…s.nix

Ericson2314 merged commit 72d161f into NixOS:staging Aug 21, 2018

Ericson2314 mentioned this pull request Aug 22, 2018

POWER9 architecture support : Add initial support to create a powerpc64le boostrap #44092

Closed

9 tasks

dtzWill reviewed Aug 22, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] ppc64le enablement #45340

[RFC] ppc64le enablement #45340

CrystalGamma commented Aug 19, 2018

Ericson2314 commented Aug 20, 2018 •

edited

CrystalGamma commented Aug 20, 2018 •

edited

Ericson2314 Aug 20, 2018

CrystalGamma Aug 20, 2018

Ericson2314 Aug 20, 2018 •

edited

CrystalGamma Aug 20, 2018

Ericson2314 Aug 20, 2018

CrystalGamma Aug 20, 2018

Ericson2314 Aug 20, 2018

Ericson2314 Aug 20, 2018

Ericson2314 Aug 20, 2018

CrystalGamma commented Aug 20, 2018

Ericson2314 commented Aug 20, 2018 •

edited

CrystalGamma commented Aug 20, 2018 •

edited

Ericson2314 commented Aug 20, 2018 •

edited

CrystalGamma commented Aug 21, 2018 •

edited

Ericson2314 commented Aug 21, 2018 •

edited

CrystalGamma commented Aug 21, 2018

Ericson2314 commented Aug 21, 2018

dtzWill left a comment

dtzWill Aug 22, 2018

dtzWill Aug 22, 2018

CrystalGamma Aug 22, 2018 •

edited

CrystalGamma Aug 23, 2018

dtzWill Aug 22, 2018

CrystalGamma Aug 22, 2018

[RFC] ppc64le enablement #45340

[RFC] ppc64le enablement #45340

Conversation

CrystalGamma commented Aug 19, 2018

Ericson2314 commented Aug 20, 2018 • edited

CrystalGamma commented Aug 20, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ericson2314 Aug 20, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CrystalGamma commented Aug 20, 2018

Ericson2314 commented Aug 20, 2018 • edited

CrystalGamma commented Aug 20, 2018 • edited

Ericson2314 commented Aug 20, 2018 • edited

CrystalGamma commented Aug 21, 2018 • edited

Ericson2314 commented Aug 21, 2018 • edited

CrystalGamma commented Aug 21, 2018

Ericson2314 commented Aug 21, 2018

dtzWill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CrystalGamma Aug 22, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ericson2314 commented Aug 20, 2018 •

edited

CrystalGamma commented Aug 20, 2018 •

edited

Ericson2314 Aug 20, 2018 •

edited

Ericson2314 commented Aug 20, 2018 •

edited

CrystalGamma commented Aug 20, 2018 •

edited

Ericson2314 commented Aug 20, 2018 •

edited

CrystalGamma commented Aug 21, 2018 •

edited

Ericson2314 commented Aug 21, 2018 •

edited

CrystalGamma Aug 22, 2018 •

edited