depends: Add a nasty qt hack to work around clang non-determinism #20436
Conversation
Clang fixed a determinism bug in the 9.x series: llvm/llvm-project@db10186 It was never backported to 8.x, however. I have manually back-ported the fix to 8.x here: theuni/llvm-project@df4158d Credit Carl Dong for discovering the determinism problem while working on Guix. Clang 8.x emits non-deterministic code, but only with certain compile options enabled (-O2 is fine but -O3 is problematic), and only when compiling very specific code. Qt builds with -O3, so we special-case the source file that manages to trigger the bug and force it to -O2. gcc/clang honor the _last_ -O option, so appending is fine.
|
Concept ACK (in the absence of a nicer-and-reliable-yet-quick alternative). |
|
Concept ACK. I can't imagine we'd care about the performance difference of -O3 vs -O2 for one object file inside Qt... |
Thanks for the explainer and potential fix. I'm going to propose an alternative using a macOS scoped patch, which should be suiteable to merge into master and backport. |
|
As I've asked on IRC too—O3 has the nasty habit of exposing compiler issues, why not build the whole thing with O2? Concept ACK anyhow if this solves the non-determinism issue. |
|
Looks like O2 is already the default for linux ( ), but it isn't set for macos |
|
@laanwj Depends sets a default of -O2, but some things (qt obviously, boost, recently secp256k1 iirc) override it. We've never really tried fighting that, afaik. I just wanted to demonstrate the minimal determinism fix here. If you'd prefer to switch the whole thing to -O2, I'd be happy to whip that up instead. |
|
Thanks for the great detective work, and initial patch. However I'm going to close this in favour of #20447. |
…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](#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
PR'ing into 0.21 because I don't think we want a hack like this in master.
Clang fixed a determinism bug in the 9.x series: llvm/llvm-project@db10186
It was never backported to 8.x, however. I have manually back-ported the fix to 8.x here in case it's useful (with Guix, for example): theuni/llvm-project@df4158d
Credit Carl Dong for discovering the determinism problem while working on Guix.
Clang 8.x emits non-deterministic code, but only with certain compile options enabled (-O2 is fine but -O3 is problematic; I haven't tracked down the exact guilty flag), and only when compiling very specific code.
Qt builds with -O3, so we special-case the source file that manages to trigger the bug and force it to -O2. gcc/clang honor the last -O option, so appending is fine.
I suspect @fanquake will hate the fact that we're echoing into a generated file. This would be much cleaner if done at the qmake level, but I'm not sure how to do that.
This is a quick nasty work-around, so here are some other potential options:
qt_intersect_spansin qpaintengine_raster.cpp) to avoid hitting the non-determinismI've opted for the last option (Using -O2 as a large hammer), because it was the quickest to write, but the second option would be fine as well. I don't have much of a preference, this is a huge layer violation either way. :)
I'd prefer not to bump clang this late in the release process, though.