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

powerpc64*: use --with-long-double-format=ieee #170215

Merged
merged 13 commits into from Apr 16, 2023
Merged

powerpc64*: use --with-long-double-format=ieee #170215

merged 13 commits into from Apr 16, 2023

Conversation

ghost
Copy link

@ghost ghost commented Apr 25, 2022

This has been made possible by #182538.

Description of changes

Use IEEE-standard floating point on powerpc64* instead of IBM-proprietary formats.

Motivation

Standardized floating-point is a good thing.

This PR will fix a lot of package test failures that happen on powerpc64* due to nonstandard floating point.

Background

The GCC wiki has more details on the history here.

Red Hat (i.e. IBM) has tried to do this in each of the last six releases, and finally shipped it in May with Fedora 36. Apparently glibc 2.35 fixes the last blocker.

Twice-vendored copies of gnulib are the root of all evil

Copies of gnulib are vendored within several packages. In fact, gettext vendors gnulib not once, but twice. These copies of gnulib must be updated in order to avoid errors like the following when using IEEE long double on powerpc64le:

gettext>   error: declaration for parameter '__obstack_vprintf_chk' but no such parameter
gettext>   105 | __LDBL_REDIR2_DECL (obstack_vprintf_chk)

The root cause of this problem is related to glibc's elaborate scheme for "redirecting" jumps based on which size of long double is in use; the scheme is part of glibc's claim that changing the semantics and bit-length of long double doesn't count as an ABI change.

The fix has been in gnulib for over a year at this point, but many packages have not had a release since then. Almost all of the projects have upgraded copies of gnulib merged into their git repositories.

Things done
  • Built on platform(s)
    • powerpc64le-linux
    • x86_64-linux: pkgsCross.powernv.stdenvBootstrapTools
  • Tested compilation of all packages that depend on this change
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Apr 25, 2022
@ghost ghost closed this Apr 26, 2022
@ghost

This comment was marked as outdated.

@ghost ghost reopened this Jul 24, 2022
@github-actions github-actions bot added 6.topic: kernel The Linux kernel 6.topic: lua 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python 6.topic: ruby 6.topic: rust 6.topic: stdenv Standard environment 6.topic: systemd 8.has: documentation 8.has: module (update) This PR changes an existing module in `nixos/` labels Jul 24, 2022
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 24, 2022
@ghost ghost mentioned this pull request Jul 24, 2022
13 tasks
@ghost ghost changed the base branch from master to staging July 24, 2022 21:55
@github-actions github-actions bot removed 8.has: documentation 6.topic: kernel The Linux kernel 6.topic: rust 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: lua 6.topic: python 6.topic: systemd 6.topic: ruby labels Jul 24, 2022
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 24, 2022
@ghost

This comment was marked as resolved.

@ghost ghost marked this pull request as draft January 2, 2023 02:54
@ghost ghost marked this pull request as ready for review January 2, 2023 02:55
@ghost

This comment was marked as resolved.

1 similar comment
@ghost

This comment was marked as resolved.

Adam Joseph and others added 13 commits April 5, 2023 15:51
This patch adds gnulib-longdouble-redirect.patch, which is a subset of
gnulib commit 776af40e09b476a41073131a90022572f448c189.

This patch must be applied to the copy of gnulib which is vendored
within a few packages, when compiling for platforms that use
longdouble-redirection (a glibc-specific feature).  Without it,
compile failures mentioning "__LDBL_REDIR1_DECL" will occur.

Currently there are two packages which need this patch to their
vendored gnulib: gettext and gnused.  Both projects have merged a
fixed version of gnulib but have not yet made a release with the fix.
We can drop this patch as soon as both of those projects are
version-bumped.
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
This commit bumps libpipeline and drops the gnulib patch since the
vendored copy of gnulib has been updated.  I avoided squashing this
commit into the previous one since knowing how to patch libpipeline
1.5.4 without upgrading might be useful (for example, in a git bisect
or cherry-pick).
Use IEEE-standard floating point on `powerpc64le` instead of
IBM-proprietary formats.  The GCC wiki has more details on the history
here:

  https://gcc.gnu.org/wiki/Ieee128PowerPC

Nixpkgs' stdenv has no legacy `powerpc64le` installs to deal with
(stdenv did not bootstrap on powerpc64le until very recenty), so it's
much easier decision for us.

Red Hat (i.e. IBM) has tried to do this in each of the last *six*
releases:

  https://fedoraproject.org/wiki/Changes/PPC64LE_Float128_Transition

they and finally shipped it in May with Fedora 36:

  https://bugzilla.redhat.com/show_bug.cgi?id=1649936

Apparently glibc 2.35 fixes the last blocker.

Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
This reverts commit dd2bc2a.

Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
This reverts commit d455973ece7f51907e86fa4e69947ae0eca6bfa1.
@ghost ghost marked this pull request as draft April 5, 2023 23:04
@github-actions github-actions bot removed the 6.topic: stdenv Standard environment label Apr 5, 2023
@ghost ghost marked this pull request as ready for review April 5, 2023 23:04
@ghost
Copy link
Author

ghost commented Apr 5, 2023

Rebased.

Now that #209870 has merged we have proper isolation of the bootstrapFiles' libraries, so I was able to drop the ugly hack from this PR.

@alyssais alyssais added the 6.topic: exotic Exotic hardware or software platform label Apr 5, 2023
@vcunat vcunat merged commit cdf4c59 into NixOS:staging Apr 16, 2023
@ghost ghost deleted the powerpc64le-use-ieee-long-double branch April 16, 2023 22:27
@ghost
Copy link
Author

ghost commented Apr 16, 2023

Thank you @vcunat!!

@vcunat
Copy link
Member

vcunat commented Apr 18, 2023

Patch won't apply in this case:
https://hydra.nixos.org/build/216634943/nixlog/1/tail

@ghost
Copy link
Author

ghost commented Apr 20, 2023

Patch won't apply in this case:
https://hydra.nixos.org/build/216634943/nixlog/1/tail

Testing a fix.

@ghost
Copy link
Author

ghost commented Apr 21, 2023

Patch won't apply in this case:
https://hydra.nixos.org/build/216634943/nixlog/1/tail

Testing a fix.

#227471

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.

7 participants