guix: native GCC 10 toolchain for Linux builds #25076
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
prusnak
approved these changes
May 6, 2022
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
hebasto
reviewed
May 7, 2022
There was a problem hiding this comment.
Concept ACK.
Guix builds on x86_64:
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
385ce95450c90c3a4512f39c049d97cae27519343add3db85764c71d7ab7e84a guix-build-7a698359f5cf/output/aarch64-linux-gnu/SHA256SUMS.part
44b836da91c4e0eb7631a79f58a8dbb2d3abe645c6c90cccc044380888d1ced4 guix-build-7a698359f5cf/output/aarch64-linux-gnu/bitcoin-7a698359f5cf-aarch64-linux-gnu-debug.tar.gz
5769408685dce864f2e63d78a55068252e6068760a71286b7801044aa5fe8ab9 guix-build-7a698359f5cf/output/aarch64-linux-gnu/bitcoin-7a698359f5cf-aarch64-linux-gnu.tar.gz
a7b6af3dd2386705fffb024e13504b6667251d1d9183be08f3cf134eb38f115b guix-build-7a698359f5cf/output/arm-linux-gnueabihf/SHA256SUMS.part
66aac029c54e6b3ff4dd400ab033231a54440d3c50cb5b7918d112adbacbd3b2 guix-build-7a698359f5cf/output/arm-linux-gnueabihf/bitcoin-7a698359f5cf-arm-linux-gnueabihf-debug.tar.gz
a15a88539909f9b544311327f534e6e43a8452a235cade877125d07820b275ac guix-build-7a698359f5cf/output/arm-linux-gnueabihf/bitcoin-7a698359f5cf-arm-linux-gnueabihf.tar.gz
66743e7bcb7509e4a37336a3d813c912a6bda187f41ac8937b525e0516e27911 guix-build-7a698359f5cf/output/arm64-apple-darwin/SHA256SUMS.part
7d0c509fa0b9e91d8d40b21cd9cc56c47c3ccb1b7080e2c11f512b1a5a7d3bf4 guix-build-7a698359f5cf/output/arm64-apple-darwin/bitcoin-7a698359f5cf-arm64-apple-darwin-unsigned.dmg
f8eccb444a594ebe281b175a7577d4251b62f696ec2d66a1b050a1dd2ecc29e3 guix-build-7a698359f5cf/output/arm64-apple-darwin/bitcoin-7a698359f5cf-arm64-apple-darwin-unsigned.tar.gz
97fad06dd7f12b8616dbc8d312a3a6703e0467325de9f336e164c1f692b8fefa guix-build-7a698359f5cf/output/arm64-apple-darwin/bitcoin-7a698359f5cf-arm64-apple-darwin.tar.gz
2cfded396620220377b9c7a3a56d6aaa41582e8f49d956a7b6afb13e7494f501 guix-build-7a698359f5cf/output/dist-archive/bitcoin-7a698359f5cf.tar.gz
7eb15de435e29121bbdfdbc50c1c93403a976a5bf6fba176b0b809ffa1cbb370 guix-build-7a698359f5cf/output/powerpc64-linux-gnu/SHA256SUMS.part
d86cf519d098d7d39e6c634025f624f15ea3454aa5dd646256d3d910ea208e19 guix-build-7a698359f5cf/output/powerpc64-linux-gnu/bitcoin-7a698359f5cf-powerpc64-linux-gnu-debug.tar.gz
69cab1e7466f50954a19dc2d60a399932fe65f52ac26fec5d9647ed1955f9ac1 guix-build-7a698359f5cf/output/powerpc64-linux-gnu/bitcoin-7a698359f5cf-powerpc64-linux-gnu.tar.gz
52718f958578be941c6e67352930f6d2e7e594f48920bcfec370f8095e3b764f guix-build-7a698359f5cf/output/powerpc64le-linux-gnu/SHA256SUMS.part
20888021a5c15bb54eb12b6ba3d7959a5975563d58c689af57083cfc46406855 guix-build-7a698359f5cf/output/powerpc64le-linux-gnu/bitcoin-7a698359f5cf-powerpc64le-linux-gnu-debug.tar.gz
fb00d94a0f606c95060c65836a6908a48b4fbdfc0e6a652328f231b4b6c6397f guix-build-7a698359f5cf/output/powerpc64le-linux-gnu/bitcoin-7a698359f5cf-powerpc64le-linux-gnu.tar.gz
65b9945d76f275612d4651b786c359cd70c38712418469230ffae61242be843b guix-build-7a698359f5cf/output/riscv64-linux-gnu/SHA256SUMS.part
1a205d567a9c13d9dd0a6aedf600889e2682f792f53e0057f37ada9ab804f1be guix-build-7a698359f5cf/output/riscv64-linux-gnu/bitcoin-7a698359f5cf-riscv64-linux-gnu-debug.tar.gz
66ce641f9827be6ca90ee2ad1d1d166e739748be94ac9d01f75d6cd4f5eafcca guix-build-7a698359f5cf/output/riscv64-linux-gnu/bitcoin-7a698359f5cf-riscv64-linux-gnu.tar.gz
784e21f2bbc38539913a0003a7d0e2227e710abbdc95fe97276528b637fc08d1 guix-build-7a698359f5cf/output/x86_64-apple-darwin/SHA256SUMS.part
49e2d505f08e7e9d7b7332955d285b306a31d65c8694c74f14a9bfbaeaf55d8d guix-build-7a698359f5cf/output/x86_64-apple-darwin/bitcoin-7a698359f5cf-x86_64-apple-darwin-unsigned.dmg
51af3ed9fd858ead1c6f1c2e29d11c171c4d29b4f998f07c93fb60171996c228 guix-build-7a698359f5cf/output/x86_64-apple-darwin/bitcoin-7a698359f5cf-x86_64-apple-darwin-unsigned.tar.gz
4b6d2a921d911b9483d5895c4272306232c3d216a853a2a3857915285d99c0ed guix-build-7a698359f5cf/output/x86_64-apple-darwin/bitcoin-7a698359f5cf-x86_64-apple-darwin.tar.gz
6c31b29fa47cfcc0fc672d7ba59434f9ed31a89aafd9d5f1c3d12d1520a52aec guix-build-7a698359f5cf/output/x86_64-linux-gnu/SHA256SUMS.part
7ecd0ac579416f44dd1e16163195459bedee7e1ef63d4dad0266d8d01e89238f guix-build-7a698359f5cf/output/x86_64-linux-gnu/bitcoin-7a698359f5cf-x86_64-linux-gnu-debug.tar.gz
f3778b4f7acbdbc22bf8f1a637bb64fef4cc01e6f0cc6ba4f8d896bf3ac772ad guix-build-7a698359f5cf/output/x86_64-linux-gnu/bitcoin-7a698359f5cf-x86_64-linux-gnu.tar.gz
cc8df483b00d3e3dc8cedeec615e1d3012e903428b9b191833fe68e5f5fb7ea8 guix-build-7a698359f5cf/output/x86_64-w64-mingw32/SHA256SUMS.part
05895a7fcde672d8bd3b8dc5cd681b306c5a4cc29478db1867067447830fc625 guix-build-7a698359f5cf/output/x86_64-w64-mingw32/bitcoin-7a698359f5cf-win64-debug.zip
0fe00838108ab147f0d500f42dace8fad32c1c05952b4b13d6d56c650e8e4fc2 guix-build-7a698359f5cf/output/x86_64-w64-mingw32/bitcoin-7a698359f5cf-win64-setup-unsigned.exe
e42e839504df5e188ddc46580cd43cc8cfd4e405ff2c2337cab2792a08a416c9 guix-build-7a698359f5cf/output/x86_64-w64-mingw32/bitcoin-7a698359f5cf-win64-unsigned.tar.gz
9c60a15355ab6e228b51bfcade1e68c5c80827d4992a4f3b13383f525a7d68b8 guix-build-7a698359f5cf/output/x86_64-w64-mingw32/bitcoin-7a698359f5cf-win64.zip
hebasto
reviewed
May 7, 2022
There was a problem hiding this comment.
In e138dea "guix: compile glibc without -werror" is it possible to specify -Wno-error=... explicitly to clearly document problematic warnings?
If 15760c4 "guix: fix glibc 2.27 multiple definition warnings with GCC 10" is about warnings, why is it required after disabling -Werror in e138dea?
d309a15 "guix: use -fcommon when building glibc 2.24"
From GCC docs:
The
-fcommonplaces uninitialized global variables in a common block. This allows the linker to resolve all tentative definitions of the same variable in different compilation units to the same object, or to a non-tentative definition. This behavior is inconsistent with C++, and on many targets implies a speed and code size penalty on global variable references. It is mainly useful to enable legacy code to link without errors.
Considering this, maybe limit this patch to the powerpc hosts only?
Different (HOST) compilers may warn in different ways, and I'm not sure of the benefit of listing all these out. It'd also remove the ability to simply set
It's to fix a link issue (
I don't really want to start configuring / compiling HOST glibcs differently (RISCV is already an exception). |
|
Concept ACK |
|
No objection, but I wonder what user-facing improvement this brings. In fact, it might be slightly disadvantageous to compile an ancient tool with a modern compiler, as we are likely the first ones to do so, putting our users on an "unexplored" path. Moreover, it might actually make it harder to bump gcc at a later point, requiring even more patches to compile the ancient glibc with an even more modern compiler. Wouldn't it be better to not touch this until gcc is bumped or alternatively until glibc is removed? |
As with the majority of the changes that are merged into this repository, there may be little-to-no end-user facing improvement. The developer-facing improvement, is a simpler Guix toolchain, and improved build times.
This change shouldn't make it any harder to bump (release) GCC. The release compiler and native compiler can exist independently of each other, as they do now. If, some time into the future, we want to use GCC 13 to build our release binaries, because we need support for C++20, then we split them back out, while continuing to use GCC 10 to compile glibc.
I would like to consolidate to GCC 10, and simplify our Guix toolchain.
Do you mean until some sort of migration to musl libc? Otherwise I don't know what you mean by glibc being removed, as it is required to provide a functioning toolchain. |
OT: Heh, I hope this is not true. I consider code changes that make the code less susceptible to bugs a (minimal) user-facing improvement. |
|
Concept ACK Although I personally don't care that we're on the same GCC version everywhere, I think this change is good overall mostly because Guix has moved from |
Compiling glibc 2.24 and 2.27 with the new GCC 10 results in a number of new warnings,
i.e:
```bash
libc-tls.c: In function ‘__libc_setup_tls’:
libc-tls.c:208:30: error: array subscript 1 is outside the bounds of an interior zero-length array ‘struct dtv_slotinfo[0]’ [-Werror=zero-length-bounds]
208 | static_slotinfo.si.slotinfo[1].map = main_map;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from ../sysdeps/x86_64/ldsodefs.h:54,
from ../sysdeps/gnu/ldsodefs.h:46,
from ../sysdeps/unix/sysv/linux/ldsodefs.h:25,
from libc-tls.c:20:
../sysdeps/generic/ldsodefs.h:398:7: note: while referencing ‘slotinfo’
398 | } slotinfo[0];
| ^~~~~~~~
```
While we could try and backport all the patches required to fix these up, it would
currently seem easier to disable -Werror, which Guix uses by default when building
glibc.
The actual macro is __has_include(), not __has_include__(), using the later would result in build failures when using GCC 10. i.e: ```bash ../sysdeps/unix/sysv/linux/riscv/flush-icache.c:24:5: warning: "__has_include__" is not defined, evaluates to 0 [-Wundef] 24 | #if __has_include__ (<asm/syscalls.h>) ``` Looks like at least someone else has run into the same thing, see: http://lists.busybox.net/pipermail/buildroot/2020-July/590376.html. See also: https://gcc.gnu.org/onlinedocs/cpp/_005f_005fhas_005finclude.html https://clang.llvm.org/docs/LanguageExtensions.html#has-include
GCC 10 started using -fno-common by default, which causes issues with the powerpc builds using gibc 2.24. A patch was commited to glibc to fix the issue, 18363b4f010da9ba459b13310b113ac0647c2fcc but is non-trvial to backport, and was broken in at least one way, see the followup in commit 7650321ce037302bfc2f026aa19e0213b8d02fe6. For now, retain the legacy GCC behaviour by passing -fcommon when building glibc 2.24. https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html. https://sourceware.org/git/?p=glibc.git;a=commit;h=18363b4f010da9ba459b13310b113ac0647c2fcc https://sourceware.org/git/?p=glibc.git;a=commit;h=7650321ce037302bfc2f026aa19e0213b8d02fe6
7a69835 to
6b9d53e
Compare
|
Rebased over #25099. Will update Guix builds in PR description. |
This was referenced May 14, 2022
Guix builds
|
Guix builds on
|
Guix builds on
|
hebasto
approved these changes
May 18, 2022
There was a problem hiding this comment.
ACK 6b9d53e
Maybe reword "we'll now use GCC 10 when compiling glibc and friends..., which is the same as our release compiler" in the PR description as it does not look as a true justification, and replace it with "Guix has moved from gcc-7 to gcc-10 as the default toolchain" from #25076 (comment)?
|
I've added a sentence about Guix having swapped it's default compiler; note that Guix having swapped to GCC 10 as their default compiler was also part of the reasoning for #23778. |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
May 28, 2022
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
May 13, 2023
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
May 13, 2023
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
May 13, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Completes the migration to using a native GCC 10 toolchain for all HOSTS. This change means we'll now use GCC 10 when compiling glibc and friends (currently we use GCC 7), which is the same as our release compiler, except for macOS (Clang 10). Note that Guix has also switched it's default compiler from GCC 7 to GCC 10. See each commit for more details.
Guix build (x86_64):
Guix build (arm64):
Closes #24701.