Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gcc: if isPower64, add --with-long-double-64 --without-long-double-128 #170857

Closed
wants to merge 7 commits into from
Closed

gcc: if isPower64, add --with-long-double-64 --without-long-double-128 #170857

wants to merge 7 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 29, 2022

Description of changes

Only the final commit is unique to this PR.

Reviewers: pleases click "Commits" then click the last commit in order to see the diff. All the other commits are part of PRs which will (I would hope) merge before this one: #169378 (first five commits) and #170400 (second-to-last commit).

Description of changes

This commit causes powerpc*-*-gnu to be configured with 64-bit long double (the same as ordinary double) rather than the nonstandard, non-IEEE "IBM double" format from AIX.

This hopefully resolves a large number of package test failures caused by weird nonstandard floating-point semantics.

There is a rather long comment included in gcc/common/platform-flags.nix explaining the situation and the reasoning. I will move that into the commit message as this PR gets closer to mergeability.

Background

This is a continuation of the work in #170402, which I don't seem to be able to reopen even though I closed it (I guess you can only reopen self-closed bugs, not PRs?).

I think we have a working combination here, and pkgsStatic now interacts correctly with the rest of nixpkgs, the way it does on arm64 and x86, with no weird link failures. Still marked as draft while I finish rebuilding the world and use this for a while on my workstation.

Notes to be integrated into final commit message
  • The "fat binary" scheme is called a GNU IFUNC.

  • Here is where glibc says that it won't let you build it without AIX 128-bit long-doubles unless you also give up IEEE 128-bit long-doubles. To have only one long-double and have it be IEEE, glibc insists you use 64-bit long-doubles. I'm starting to understand why the musl people are so insistent about not wanting anything to do with this...

Things done
  • Built on platform(s)
    • powerpc64le-linux
    • x86_64-linux (in progress)
    • aarch64-linux (in progress)
    • mips64le-linux (in progress)
    • x86_64-darwin
    • aarch64-darwin
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Adam Joseph and others added 7 commits April 19, 2022 17:09
As explained in the comment, this ensures that stage4-coreutils does
not leak a reference to the bootstrap-files by way of libgmp.  This
will allow the next patch in this series to build stage4-coreutils
using a dynamically-linked (rather than statically-linked) libgmp.
The usage of `makeStaticLibraries` in stdenv/linux/default.nix is
prefaced by this comment:

  # Link GCC statically against GMP etc.  This makes sense because
  # these builds of the libraries are only used by GCC, so it
  # reduces the size of the stdenv closure.

However "these builds of the libraries are only used by GCC" is not
actually true.  As currently written, the stage4 coreutils links
against these customized, static-ified libraries.

Beside the fact that the code doesn't actually do what it says, this
causes other problems as well.  One example is #168983, which arises
because have a dynamically-linked binary (coreutils) which is built
from statically-linked libraries (libgmp.a); doing this causes mayhem
on platforms where `-fstack-protector` needs an auxiliary
`libssp.{so,a}` library; we end up with link failures because some
parts of the resulting binary want `libssp.so` and other parts want
`libssp_nonshared.a`.

Let's make the code actually do what the comment says, by moving these
definitions into the `gcc-unwrapped` override.  This will cause the
stage4-coreutils to link against libgmp dynamically, rather than
statically.  For this reason this commit depends on the previous
commit, which allows that to be done without creating a forbidden
reference from stdenv-final to the bootstrap-files.
During stdenv bootstrapping, coreutils is built twice.  This makes
troubleshooting very difficult, because both packages have
name="coreutils", so it is a hassle to figure out "which coreutils am
I using / is not building"?

The first of these builds is used only in stage4, and is not part of
the final stdenv.  Let's label that one with a different `name`
attribute to make it obvious which is which.
Co-authored-by: sternenseemann <sternenseemann@systemli.org>
A bug in libgcc which manifests when building static binaries using
musl-libc was fixed in gcc 11.1.0, but the fix has not been backported
to gcc 10.  This commit cherry-picks the commit from upstream and
applies it in the case (isPower64 && isMusl) where it matters:

  https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=cda41ce0e8414aec59e6b9fbe645d96e6e8193e2
This commit causes powerpc*-*-gnu to be configured with 64-bit
IEEE-compliant `long double` (the same as ordinary `double`) rather
than the nonstandard, non-IEEE "IBM double" format from AIX.  Support
for using only IEEE 128-bit doubles in glibc is not yet ready.

This resolves several package test failures caused by weird
nonstandard floating-point semantics.  It also eliminates a lot of
difficulties that arise when using glibc-linked tools to compile for
musl-libc and vice versa.

There is a long comment included in gcc/common/platform-flags.nix
explaining the situation and the reasoning.
@github-actions github-actions bot added the 6.topic: stdenv Standard environment label Apr 29, 2022
@@ -67,6 +67,7 @@ let majorVersion = "10";
++ optional langAda ../gnat-cflags.patch
++ optional langFortran ../gfortran-driving.patch
++ optional (targetPlatform.libc == "musl" && targetPlatform.isPower) ../ppc-musl.patch
++ optional (targetPlatform.isMusl && targetPlatform.isPower64) ./backport-libgcc-fix-PR97653.patch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would help to instead default the platform to gcc11? (like x86_64-linux and some others)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try that as soon as my current rebuild-the-world finishes.

However even in that case could we still keep the line above? At least that way if somebody needs gcc10 for some reason it will work.

PS, that line is actually from #170400, which this PR builds on top of. That PR is ready for review and possible merge; this one is still WIP but getting very close.

@ghost
Copy link
Author

ghost commented Apr 29, 2022

This is looking really promising.

I have verified that with this PR, the tests for goffice pass. Meaning that if this merges, #170567 can be reverted.

I am optimistic that this is going to save us having to scatter a ton of doCheck = !stdenv.hostPlatform.isPower64's all over the codebase wherever a package's test involves floating point rounding behavior.

@ghost
Copy link
Author

ghost commented Apr 30, 2022

Everything rebuilt, and I'm running it now.

The only catch is that glibc doesn't want to let itself be built by a compiler that was built using either --without-long-double-128 or --with-long-double-64. So I was able to build everything because stdenv uses the bootstrap-tools's compiler (rather than its own compiler) to build glibc. But I can't rebuild the bootstrap-files.

Other than that, it works.

It may take me a while to resolve the glibc issue.

Also, I noticed that stdenv builds a somewhat Frankensteined compiler... gcc is compiled by itself (using the stage1/stage2 thing that it does as part of its internal bootstrap), but the final (stage2) gcc gets linked against a libgmp, libmpc, libisl, and libmpfr that were compiled by the bootstrap-tools' compiler.

Has there been any effort put into splitting gcc's two-step bootstrap into two separate derivations? Its build machinery has support for that (--disable-bootstrap). Then we could build gmp/mpc/isl/mpfr using gcc-stage1 and provide those to gcc-stage2 to get a uniformly self-compiled compiler. This would probably be an additional "stage" in pkgs/stdenv/linux/default.nix, but the only additional compilation would be one extra recompile of gmp/mpc/isl/mpfr (all of them very quick). In particular, there shouldn't be any extra recompilations of gcc.

@vcunat
Copy link
Member

vcunat commented Apr 30, 2022

I'm not aware of such efforts. I wouldn't expect issues from some parts being built by a bit older compiler. There are many cycles that are hard to avoid among stdenv parts, e.g. also glibc.

@ghost
Copy link
Author

ghost commented May 1, 2022

I'm not aware of such efforts.

I might look into writing a PR for it.

I wouldn't expect issues from some parts being built by a bit older compiler.

It's only a problem when there is an ABI change without a new multiarch tuple. In theory that should never happen. But this whole IBM long double mess appears to be a possible counterexample.

@ghost
Copy link
Author

ghost commented May 11, 2022

So Fedora finally managed to ship a release with IEEE long doubles after attempting six times: this bug is marked CLOSED CURRENTRELEASE.

The word on the street is that this stuff simply did not work until glibc 2.35. That would definitely explain the bruise on my head and the damage to my office wall as a result of one being banged against the other.

I will revisit this after nixpkgs' glibc upgrades to glibc 2.35 or later.

@ghost
Copy link
Author

ghost commented Jun 29, 2022

Also, I noticed that stdenv builds a somewhat Frankensteined compiler... gcc is compiled by itself (using the stage1/stage2 thing that it does as part of its internal bootstrap), but the final (stage2) gcc gets linked against a libgmp, libmpc, libisl, and libmpfr that were compiled by the bootstrap-tools' compiler.

This didn't sit well with me, but until now I couldn't articulate why.

wouldn't expect issues from some parts being built by a bit older compiler.

I think I found two examples:

  1. The current situation prevents you from using the bootstrap-tools from the 32-bit version of an architecture to bootstrap a 64-bit stdenv on a machine that can execute both 32-bit and 64-bit code. The bootstrap process will attempt to link 32-bit lib{gmp,mpc,isl,mpfr} into the final 64-bit gcc.

Some people will undoubtedly say "oh, 32-bit is so unmodern, nobody cares about that ancient stuff". But:

  1. The same situation comes up with ABIs: both x86 and MIPS have a Knuthian "64-bit integers with 32-bit pointers" ABI. Neither is an earlier/later "version of" the other (like with the powerpc long-double stuff), and neither is strictly-superior/inferior-to the other (so you can't assume one will "kill off" the other at some point).

In both the 32/64-bit case and the ABI case you can always bootstrap a complete nixpkgs using the bootstrap-files you have (e.g. bootstrap a complete 32-bit nixpkgs on a 64-bit processor) and then use that to "cross-compile" your own (e.g. 64-bit) bootstrap-files. So it isn't a major problem. But it is a reason to prefer having the stage2 gcc be compiled entirely by the stage1 gcc, if it were easy to do that. Right now it probably isn't easy.

Edit, 2022-Jul-20: @trofi found another one: #181943 (comment)

@ghost ghost mentioned this pull request Jul 21, 2022
13 tasks
@ghost
Copy link
Author

ghost commented Jul 24, 2022

Closing in favor of #170215 which is now feasible due to glibc-2.35.

@ghost ghost deleted the powerpc64-use-long-double+ branch January 23, 2024 06:47
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant