build: Bump clang version to fix non-determinism by hebasto · Pull Request #20454 · bitcoin/bitcoin

hebasto · 2020-11-22T22:43:28Z

Bumping clang from 6.0.1 to 8.0.0 in 5c2c835 (#19240) came with inherent non-determinism, that was fixed in LLVM 9.

This PR suggests the first option from:

Bump to clang9 where this is fixed

As dongcarl suggested: rewrite the some of the qt function (qt_intersect_spans in qpaintengine_raster.cpp) to avoid hitting the non-determinism

Disable the buggy behavior via cmdline option

This is an alternative to #20436, #20440 and #20447.
No patches :)

hebasto · 2020-11-22T22:44:59Z

cc @achow101 @dongcarl @fanquake @theuni

fanquake · 2020-11-23T03:52:08Z

While we will likely end up doing this for 22.0, concept NACK on switching compilers in the RC stage of a release. I'm somewhat surprised this works as is. Normally there are other minor changes required, like adjusting a path, or fixing up a .so name etc, however that's good to know.

hebasto · 2020-11-23T06:56:31Z

While we will likely end up doing this for 22.0, concept NACK on switching compilers in the RC stage of a release.

Correct. My intention is to get this merged into the master branch, not into the 0.21. For the 0.21 branch I've already ACKed #20447.

laanwj · 2020-11-23T10:03:51Z

This has been my preferred way to resolve this from the beginning.

Code review ACK 5ebb747
Did not check determinism though.

hebasto · 2020-11-23T14:44:50Z

My gitian macOS build:

Generating report
a723c3ce3816387f6f4df75b31a7cd8afe41115c37d75eb3f37ec8b19bad9e06  bitcoin-5ebb747d2a14-osx-unsigned.dmg
3ba163f8979ee9d68e19861f44d4c9757b1bd9a2af82ba902d9bc6c234ad0c47  bitcoin-5ebb747d2a14-osx-unsigned.tar.gz
e122c0511048ff7066d244277a3c83d91429b4c237a2835d17dbe96328700ebb  bitcoin-5ebb747d2a14-osx64.tar.gz
ea036d6b0e29a48a4f4776f1cee9e65e766cb74ba87130d03af2c299ff080b82  src/bitcoin-5ebb747d2a14.tar.gz
ac7c1d432f4db182cf5765bae2511881cc4a8615eccbe1c5ac6d53870789020c  bitcoin-core-osx-22-res.yml
Done.

…istic behavior in LLVM 8 8f7d1b3 Fix QPainter non-determinism on macOS (Andrew Chow) Pull request description: Aplies a patch to Qt that fixes the non-determinism by modifying Qt. The source of the non-determinism is how LLVM 8 optimizes qt_intersect_spans when compiling. The particular optimization that seems to be causing the problems is that a temp variable is being added for spans->y. For some reason, when it does this, it chooses different instructions to use when making that variable. We bypass this problem by patching qt_intersect_spans to always make and use this local variable. Potential alternative to #20436 and #20440 ACKs for top commit: hebasto: re-ACK 8f7d1b3 ~for merging into the 0.21 branch, but [not into the master](bitcoin/bitcoin#20454) branch.~ fanquake: ACK 8f7d1b3 Tree-SHA512: b0d00a77643554021736524fb64611462ef2ec849a220543c12d99edb0f52f2e8128d2cc61fa82176b7e13b294574774a92d6b649badf8b7630c6d6a7e70ce10

…eterministic behavior in LLVM 8 8f7d1b3 Fix QPainter non-determinism on macOS (Andrew Chow) Pull request description: Aplies a patch to Qt that fixes the non-determinism by modifying Qt. The source of the non-determinism is how LLVM 8 optimizes qt_intersect_spans when compiling. The particular optimization that seems to be causing the problems is that a temp variable is being added for spans->y. For some reason, when it does this, it chooses different instructions to use when making that variable. We bypass this problem by patching qt_intersect_spans to always make and use this local variable. Potential alternative to bitcoin#20436 and bitcoin#20440 ACKs for top commit: hebasto: re-ACK 8f7d1b3 ~for merging into the 0.21 branch, but [not into the master](bitcoin#20454) branch.~ fanquake: ACK 8f7d1b3 Tree-SHA512: b0d00a77643554021736524fb64611462ef2ec849a220543c12d99edb0f52f2e8128d2cc61fa82176b7e13b294574774a92d6b649badf8b7630c6d6a7e70ce10

laanwj · 2021-03-04T08:41:20Z

I think we still need to do this?
@dongcarl @fanquake

LLVM 8 has inherent nondeterminism in the compiler, fixed in LLVM 9.

This reverts commit 8f7d1b3.

hebasto · 2021-03-11T11:53:17Z

Rebased on top of the #21376.

hebasto · 2021-03-11T12:59:26Z

Gitian builds:

Generating report
df3ab808f69e38bf263981e202247c253191ebba6f1e5a2d847cf1f493a73cdd  bitcoin-8df117089569-osx-unsigned.dmg
e9f3ef47f067a0145b013ce502175ccdd8465f2a1f00277e2abb2eb8e90f860d  bitcoin-8df117089569-osx-unsigned.tar.gz
0b1b52aa8c1b20bcd1e9bc75053acfdcc436de8ce382f26068ac7f7c9483d1b9  bitcoin-8df117089569-osx64.tar.gz
3196530732e8ff0388a59a30c7bad17ecb7496f844af29a3813404d6523c6284  src/bitcoin-8df117089569.tar.gz
d7f9d053360ba8ad6ba8853059b22519b4d593bbcda91071ff0eae8e5bb7d346  bitcoin-core-osx-22-res.yml
Done.

# cache was not cleared
Generating report
df3ab808f69e38bf263981e202247c253191ebba6f1e5a2d847cf1f493a73cdd  bitcoin-8df117089569-osx-unsigned.dmg
e9f3ef47f067a0145b013ce502175ccdd8465f2a1f00277e2abb2eb8e90f860d  bitcoin-8df117089569-osx-unsigned.tar.gz
0b1b52aa8c1b20bcd1e9bc75053acfdcc436de8ce382f26068ac7f7c9483d1b9  bitcoin-8df117089569-osx64.tar.gz
3196530732e8ff0388a59a30c7bad17ecb7496f844af29a3813404d6523c6284  src/bitcoin-8df117089569.tar.gz
d7f9d053360ba8ad6ba8853059b22519b4d593bbcda91071ff0eae8e5bb7d346  bitcoin-core-osx-22-res.yml
Done.

# cache was cleared
Generating report
df3ab808f69e38bf263981e202247c253191ebba6f1e5a2d847cf1f493a73cdd  bitcoin-8df117089569-osx-unsigned.dmg
e9f3ef47f067a0145b013ce502175ccdd8465f2a1f00277e2abb2eb8e90f860d  bitcoin-8df117089569-osx-unsigned.tar.gz
0b1b52aa8c1b20bcd1e9bc75053acfdcc436de8ce382f26068ac7f7c9483d1b9  bitcoin-8df117089569-osx64.tar.gz
3196530732e8ff0388a59a30c7bad17ecb7496f844af29a3813404d6523c6284  src/bitcoin-8df117089569.tar.gz
55c5c0a8f8863508f7544568d5a0c8214529f9aa57200a2babcd92486c9ec334  bitcoin-core-osx-22-res.yml
Done.

Guix build:

$ find output -type f -name *$(git rev-parse --short HEAD)*.* -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
71fb6c229b472a34a80c8efa62c8926078ca66c47ec4eceac4dcb1689879228a  output/bitcoin-8df117089569-osx-unsigned.dmg
d86bb1f041ae104eeb524f346349a3568f3549919618f36a1c0d72186fa763e7  output/bitcoin-8df117089569-osx-unsigned.tar.gz
7b5a0f09c8c21df67aca61fcf5cb9a18c2944a71e769f50d87beb1ce1861cfaa  output/bitcoin-8df117089569-osx64.tar.gz
3196530732e8ff0388a59a30c7bad17ecb7496f844af29a3813404d6523c6284  output/src/bitcoin-8df117089569.tar.gz

jarolrod

Contributing GUIX hashes, mine line up with hebasto's

find output -type f -name *$(git rev-parse --short HEAD)*.* -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum

71fb6c229b472a34a80c8efa62c8926078ca66c47ec4eceac4dcb1689879228a  output/bitcoin-8df117089569-osx-unsigned.dmg
d86bb1f041ae104eeb524f346349a3568f3549919618f36a1c0d72186fa763e7  output/bitcoin-8df117089569-osx-unsigned.tar.gz
7b5a0f09c8c21df67aca61fcf5cb9a18c2944a71e769f50d87beb1ce1861cfaa  output/bitcoin-8df117089569-osx64.tar.gz
3196530732e8ff0388a59a30c7bad17ecb7496f844af29a3813404d6523c6284  output/src/bitcoin-8df117089569.tar.gz

DrahtBot · 2021-03-12T08:09:23Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

guix: Add guix-{attest,verify} scripts #21462 (guix: Add guix-{attest,verify} scripts by dongcarl)
build: split libtapi and clang out of native_cctools #21457 (build: split libtapi and clang out of native_cctools by fanquake)
build, qt: No longer need to patch translation.pro #21420 (build, qt: No longer need to patch translation.pro by hebasto)
guix: Misc feedback-based fixes + hier restructuring #21375 (guix: Misc feedback-based fixes + hier restructuring by dongcarl)
guix: Add guix-clean script + establish gc-root for container profiles #21304 (guix: Add guix-clean script + establish gc-root for container profiles by dongcarl)

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.

DrahtBot · 2021-03-17T14:35:01Z

Guix builds

File	commit `e0bc27a` (master)	commit 292695d5402a528532f8f19c7d1955586b7b532a (master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`3b30f4198b68fadf...`	`fbd9cb3b0f69a69d...`
*-aarch64-linux-gnu.tar.gz	`176d561bd8a2aa0d...`	`a9b968b656d7712e...`
*-arm-linux-gnueabihf-debug.tar.gz	`ea017c7dff2b99be...`	`2fdbb8f2d6436870...`
*-arm-linux-gnueabihf.tar.gz	`3ce0b418bce0f580...`	`6d8015f2beecb429...`
*-osx-unsigned.dmg	`772d2aa9989e8d40...`	`ae6b831da7906043...`
*-osx-unsigned.tar.gz	`0d8003ee2c879e13...`	`d30926a1853c162e...`
*-osx64.tar.gz	`135d20c9d45b8193...`	`38aa5adb89864d4e...`
*-powerpc64-linux-gnu-debug.tar.gz	`0f86fbb3da88d75b...`	`d6c25a4366e2d466...`
*-powerpc64-linux-gnu.tar.gz	`8fffb547da793622...`	`69c37461a0d145b9...`
*-powerpc64le-linux-gnu-debug.tar.gz	`25bc9f40d7593a0e...`	`1d00255a50763a3e...`
*-powerpc64le-linux-gnu.tar.gz	`00ffc2de444f9f45...`	`85f59c2c327b6c6f...`
*-riscv64-linux-gnu-debug.tar.gz	`13259ba2232c7908...`	`2c0490e4fa7a6627...`
*-riscv64-linux-gnu.tar.gz	`277fa4286cd1acc3...`	`c4f2f1a3649d305d...`
*-win-unsigned.tar.gz	`586f395e11db7d10...`	`8134e2c6173da988...`
*-win64-debug.zip	`a47083334cbdcfa3...`	`ecbcf562502612bf...`
*-win64-setup-unsigned.exe	`926f391d3ea1f345...`	`43177ebda8aa3611...`
*-win64.zip	`cbfc8629325db892...`	`e69e51bbda561174...`
*-x86_64-linux-gnu-debug.tar.gz	`ed183a137ae13cd4...`	`ab1188a63ee0bd68...`
*-x86_64-linux-gnu.tar.gz	`f24558a571c69c68...`	`22157c0fc560511d...`
*.tar.gz	`86c28332470cb476...`	`21d580fcb713968c...`
guix_build.log	`456643eedd9954da...`	`8c8ae81656e1e9e7...`
guix_build.log.diff		`beff8b72b7e6ca52...`

fanquake · 2021-03-18T02:48:37Z

I've now included this in #19817, as part of a toolchain bump, which also means we'd move to Clang 10.

765e0be build: split native_cctools (fanquake) Pull request description: This splits our native cctools package into two additional packages: `native_clang` and `native_libtapi`. This is in an effort to not only make our mac toolchain more understandable, but also to reduce duplication, and as a nice side-effect, fix the issue mentioned [here](#19817 (comment)). Everything about the current build process should remain the same. For gitian, that is: * Download LLVM Clang 8.0.0 binary. * Build libtapi using downloaded Clang. * Build cctools using downloaded Clang and libtapi. * Build the rest of depends.. For Guix (`FORCE_USE_SYSTEM_CLANG=1`): * Build libtapi using using system Clang. * Build cctools using system Clang and libtapi. * Build the rest of depends.. After this has been merged, my plan is to combine a modified version of #20454 and #21414 with #19817, and from what I understand that will be enough to support Apple Arm cross compilation. Builds at 477ed59: Guix: ```bash find output -type f -name *$(git rev-parse --short HEAD)*.* -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum 7e57b9e5a2109d1a35f0091d86f975c1b1d73ac70ac2609cefbe1134efbf2c87 output/bitcoin-477ed59f49f3-osx-unsigned.dmg dd11e71c2634ac2fa883d1e45cbd6de194fad37624bb56b8b8a6213fd40d6050 output/bitcoin-477ed59f49f3-osx-unsigned.tar.gz 64384eaa2fd9768992d86a06a1414c9e92e84ba21a875696483df2bb5828e2a2 output/bitcoin-477ed59f49f3-osx64.tar.gz 8a889e88db952d2c82ef44713c04aba95b777441f578738ff6d8a0d251e51da3 output/src/bitcoin-477ed59f49f3.tar.gz ``` Gitian: ```bash d0eee8542d5f3d662555ad7218d2dc9f3f862656e65bcb2f01f256bfa0deead2 bitcoin-477ed59f49f3-osx-unsigned.dmg ba7bc94897e42e7a037e352c4e4e1730f181c6d76b6d6a2785bbd7bf85614c83 bitcoin-477ed59f49f3-osx-unsigned.tar.gz c4426d1d310a2fbffcaf2b7df0da4ec97bd11aab07085006dae68777b03f6372 bitcoin-477ed59f49f3-osx64.tar.gz 8a889e88db952d2c82ef44713c04aba95b777441f578738ff6d8a0d251e51da3 src/bitcoin-477ed59f49f3.tar.gz a746831467dc8ff17ec5df06fc9288a859c1961d8c0b632d97b42f080dbd825d bitcoin-core-osx-22-res.yml ``` ACKs for top commit: dongcarl: reACK 765e0be hebasto: ACK 765e0be, verified building of the `native_cctools` package step-by-step. Tree-SHA512: 61cf2b092fb8b9724adda1084e0cac9db889cd5e391914b43592aebc470fae3c1cbabc8b59a0abce6e7bad8c646694fe927f26f4deb18b60d7fd92f374f62feb

…ools 765e0be build: split native_cctools (fanquake) Pull request description: This splits our native cctools package into two additional packages: `native_clang` and `native_libtapi`. This is in an effort to not only make our mac toolchain more understandable, but also to reduce duplication, and as a nice side-effect, fix the issue mentioned [here](bitcoin#19817 (comment)). Everything about the current build process should remain the same. For gitian, that is: * Download LLVM Clang 8.0.0 binary. * Build libtapi using downloaded Clang. * Build cctools using downloaded Clang and libtapi. * Build the rest of depends.. For Guix (`FORCE_USE_SYSTEM_CLANG=1`): * Build libtapi using using system Clang. * Build cctools using system Clang and libtapi. * Build the rest of depends.. After this has been merged, my plan is to combine a modified version of bitcoin#20454 and bitcoin#21414 with bitcoin#19817, and from what I understand that will be enough to support Apple Arm cross compilation. Builds at 477ed59: Guix: ```bash find output -type f -name *$(git rev-parse --short HEAD)*.* -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum 7e57b9e5a2109d1a35f0091d86f975c1b1d73ac70ac2609cefbe1134efbf2c87 output/bitcoin-477ed59f49f3-osx-unsigned.dmg dd11e71c2634ac2fa883d1e45cbd6de194fad37624bb56b8b8a6213fd40d6050 output/bitcoin-477ed59f49f3-osx-unsigned.tar.gz 64384eaa2fd9768992d86a06a1414c9e92e84ba21a875696483df2bb5828e2a2 output/bitcoin-477ed59f49f3-osx64.tar.gz 8a889e88db952d2c82ef44713c04aba95b777441f578738ff6d8a0d251e51da3 output/src/bitcoin-477ed59f49f3.tar.gz ``` Gitian: ```bash d0eee8542d5f3d662555ad7218d2dc9f3f862656e65bcb2f01f256bfa0deead2 bitcoin-477ed59f49f3-osx-unsigned.dmg ba7bc94897e42e7a037e352c4e4e1730f181c6d76b6d6a2785bbd7bf85614c83 bitcoin-477ed59f49f3-osx-unsigned.tar.gz c4426d1d310a2fbffcaf2b7df0da4ec97bd11aab07085006dae68777b03f6372 bitcoin-477ed59f49f3-osx64.tar.gz 8a889e88db952d2c82ef44713c04aba95b777441f578738ff6d8a0d251e51da3 src/bitcoin-477ed59f49f3.tar.gz a746831467dc8ff17ec5df06fc9288a859c1961d8c0b632d97b42f080dbd825d bitcoin-core-osx-22-res.yml ``` ACKs for top commit: dongcarl: reACK 765e0be hebasto: ACK 765e0be, verified building of the `native_cctools` package step-by-step. Tree-SHA512: 61cf2b092fb8b9724adda1084e0cac9db889cd5e391914b43592aebc470fae3c1cbabc8b59a0abce6e7bad8c646694fe927f26f4deb18b60d7fd92f374f62feb

DrahtBot added the Build system label Nov 23, 2020

maflcko added Needs gitian build DrahtBot Guix build requested labels Nov 23, 2020

DrahtBot removed DrahtBot Guix build requested Needs gitian build labels Nov 27, 2020

hebasto added 2 commits March 11, 2021 13:34

build: Bump clang version to fix non-determinism

ac5e771

LLVM 8 has inherent nondeterminism in the compiler, fixed in LLVM 9.

Revert "Fix QPainter non-determinism on macOS"

8df1170

This reverts commit 8f7d1b3.

hebasto force-pushed the 201122-clang branch from 5ebb747 to 8df1170 Compare March 11, 2021 11:51

jarolrod reviewed Mar 11, 2021

View reviewed changes

bitcoin deleted a comment from DrahtBot Mar 11, 2021

maflcko added Needs gitian build DrahtBot Guix build requested labels Mar 11, 2021

DrahtBot mentioned this pull request Mar 12, 2021

build, qt: No longer need to patch translation.pro #21420

Closed

fanquake mentioned this pull request Mar 17, 2021

build: split libtapi and clang out of native_cctools #21457

Merged

DrahtBot removed the DrahtBot Guix build requested label Mar 17, 2021

This was referenced Mar 17, 2021

guix: Add guix-{attest,verify} scripts #21462

Merged

guix: Misc feedback-based fixes + hier restructuring #21375

Merged

DrahtBot mentioned this pull request Mar 17, 2021

guix: Add guix-clean script + establish gc-root for container profiles #21304

Merged

fanquake closed this Mar 18, 2021

maflcko removed the Needs gitian build label Mar 20, 2021

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Bump clang version to fix non-determinism#20454

build: Bump clang version to fix non-determinism#20454
hebasto wants to merge 2 commits intobitcoin:masterfrom
hebasto:201122-clang

hebasto commented Nov 22, 2020 •

edited

Loading

Uh oh!

hebasto commented Nov 22, 2020

Uh oh!

fanquake commented Nov 23, 2020

Uh oh!

hebasto commented Nov 23, 2020

Uh oh!

laanwj commented Nov 23, 2020 •

edited

Loading

Uh oh!

hebasto commented Nov 23, 2020

Uh oh!

laanwj commented Mar 4, 2021

Uh oh!

hebasto commented Mar 11, 2021

Uh oh!

hebasto commented Mar 11, 2021

Uh oh!

jarolrod left a comment

Uh oh!

DrahtBot commented Mar 12, 2021 •

edited

Loading

Uh oh!

DrahtBot commented Mar 17, 2021

Uh oh!

fanquake commented Mar 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

hebasto commented Nov 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Nov 22, 2020

Uh oh!

fanquake commented Nov 23, 2020

Uh oh!

hebasto commented Nov 23, 2020

Uh oh!

laanwj commented Nov 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Nov 23, 2020

Uh oh!

laanwj commented Mar 4, 2021

Uh oh!

hebasto commented Mar 11, 2021

Uh oh!

hebasto commented Mar 11, 2021

Uh oh!

jarolrod left a comment

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Mar 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

DrahtBot commented Mar 17, 2021

Guix builds

Uh oh!

fanquake commented Mar 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

hebasto commented Nov 22, 2020 •

edited

Loading

laanwj commented Nov 23, 2020 •

edited

Loading

DrahtBot commented Mar 12, 2021 •

edited

Loading