Enable New Pass Manager for Clang. #3346
Closed
Conversation
This file contains 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
Clang has a new optimization pass manager activated by -fexperimental-new-pass-manager. It's been in development for several years, and by now generally performing better than the default. It is planned to be the new default in Clang 13, but even versions before that perform better with it, and it's stable enough that major software like Firefox has been using it already: https://bugzilla.mozilla.org/show_bug.cgi?id=1619461 It's about 1% speedup for Stockfish. Result of 100 runs ================== base (...fish_clang12) = 1946851 +/- 3717 test (./stockfish ) = 1967276 +/- 3408 diff = +20425 +/- 2438 speedup = +0.0105 P(speedup > 0) = 1.0000 CPU: 4 x Intel(R) Xeon(R) CPU E3-1240 v3 @ 3.40GHz Hyperthreading: on Thanks to David Major for making me aware of this part of LLVM development.
|
With this and the ThinLTO fix clang is close to 2% faster on my hardware than GCC 10: |
|
2 long concurrent runs, both pgo builds , both with "flto". A 0.7% speed pickup for clang over gcc, total runtime over 3 minutes, fwiw, I ran a number of shorter concurrent runs and clang was faster everytime with the pass manager flag noted in this PR. OS Sys Windows 10 Pro |
|
I did a comparison, just with master, in my case clang is 3.5% slower (AMD Ryzen 9 3950X): I would say both compilers are pretty equivalent in speed nevertheless. |
|
Hi Joost, there's some benchmarking in issue #3341. It does look like they're close (at least with clang-12), but when GCC is faster it tends to be a lot faster, so I withdrew the issue to default clang. But the clang settings change here should be beneficial though, for people that choose to use clang. |
|
Do you know what older versions of clang++ support the flag -fexperimental-new-pass-manager flag, and how future versions of clang will deal with the flag? That is, should we be concerned about users of earlier / newer versions of clang where this might have side effects (bugs). |
|
Newer versions will Just Work - they default it on internally, and they add a flag to activate the old behavior (legacy pass manager). At some point they may remove the flag, by which point we could remove it here as it will be the default anyway. The flag has existed since 2016 I think (https://reviews.llvm.org/D28077), so every clang that's newer than 4.0 (or thereabouts) should handle it. Given that we don't default clang, I think that's more than enough margin. |
|
OK, both PRs combined give a good speedup here: |
BM123499
pushed a commit
to BM123499/Stockfish
that referenced
this pull request
Feb 22, 2021
It's about 1% speedup for Stockfish. Result of 100 runs ================== base (...fish_clang12) = 1946851 +/- 3717 test (./stockfish ) = 1967276 +/- 3408 diff = +20425 +/- 2438 speedup = +0.0105 P(speedup > 0) = 1.0000 Thanks to David Major for making me aware of this part of LLVM development. closes official-stockfish#3346 No functional change
joergoster
pushed a commit
to joergoster/Stockfish-old
that referenced
this pull request
Feb 27, 2021
It's about 1% speedup for Stockfish. Result of 100 runs ================== base (...fish_clang12) = 1946851 +/- 3717 test (./stockfish ) = 1967276 +/- 3408 diff = +20425 +/- 2438 speedup = +0.0105 P(speedup > 0) = 1.0000 Thanks to David Major for making me aware of this part of LLVM development. closes official-stockfish/Stockfish#3346 No functional change
Fanael
pushed a commit
to Fanael/Stockfish
that referenced
this pull request
Mar 7, 2021
It's about 1% speedup for Stockfish. Result of 100 runs ================== base (...fish_clang12) = 1946851 +/- 3717 test (./stockfish ) = 1967276 +/- 3408 diff = +20425 +/- 2438 speedup = +0.0105 P(speedup > 0) = 1.0000 Thanks to David Major for making me aware of this part of LLVM development. closes official-stockfish#3346 No functional change
dav1312
pushed a commit
to dav1312/Stockfish
that referenced
this pull request
Nov 25, 2022
Passed cutechess STC: Score of RF 14 vs RF 3: 1124 - 536 - 1130 [0.605] 2790 ... RF 14 playing White: 620 - 246 - 530 [0.634] 1396 ... RF 14 playing Black: 504 - 290 - 600 [0.577] 1394 ... White vs Black: 910 - 750 - 1130 [0.529] 2790 Elo difference: 74.3 +/- 10.0, LOS: 100.0 %, DrawRatio: 40.5 % SPRT: llr 2.95 (100.2%), lbound -2.94, ubound 2.94 - H1 was accepted Passed cutechess LTC: Score of RF 14 vs RF 3: 875 - 355 - 1062 [0.613] 2292 ... RF 14 playing White: 519 - 153 - 475 [0.660] 1147 ... RF 14 playing Black: 356 - 202 - 587 [0.567] 1145 ... White vs Black: 721 - 509 - 1062 [0.546] 2292 Elo difference: 80.2 +/- 10.4, LOS: 100.0 %, DrawRatio: 46.3 % SPRT: llr 2.95 (100.0%), lbound -2.94, ubound 2.94 - H1 was accepted Bench: 4918790 (+24 squashed commit) Squashed commit: [5118c15] Use Bitboard over Square in movegen It uses pos.checkers() on target when movegen is the type of EVASION. It simplify the code. And it's also expected a slightly speed up, because Bitboard is more direct when doing bitwise. Passed STC: LLR: 2.93 (-2.94,2.94) {-1.25,0.25} Total: 28176 W: 2506 L: 2437 D: 23233 Ptnml(0-2): 80, 1904, 10063, 1949, 92 https://tests.stockfishchess.org/tests/view/60421d18ddcba5f0627bb6a9 Passed LTC: LLR: 2.93 (-2.94,2.94) {-0.75,0.25} Total: 9704 W: 402 L: 341 D: 8961 Ptnml(0-2): 3, 279, 4230, 334, 6 https://tests.stockfishchess.org/tests/view/60422823ddcba5f0627bb6ae closes official-stockfish#3383 No functional change [42b44ee] Deal with commented lines in UCI input commands starting with '#' as the first character will be ignored closes official-stockfish#3378 No functional change [550d3d8] Add Stockfish namespace. fixes official-stockfish#3350 and is a small cleanup that might make it easier to use SF in separate projects, like a NNUE trainer or similar. closes official-stockfish#3370 No functional change. [6ccec01] Clean functions returning by const values The codebase contains multiple functions returning by const-value. This patch is a small cleanup making those function returns by value instead, removing the const specifier. closes official-stockfish#3328 No functional change [5801707] Import author list and copyright years Easier to do it this way than track all the cherry picks one by one. [fd5fc27] Use correct chess terms + fix spelling. - "discovered check" (instead of "discovery check") - "en passant" (instead of "en-passant") - "pseudo-legal" before a noun (instead of "pseudo legal") - "3-fold" (instead of "3fold") closes official-stockfish#3294 No functional change. [3d8a301] Better code for hash table generation This patch removes some magic numbers in TT bit management and introduce proper constants in the code, to improve documentation and ease further modifications. No function change [f314344] Allow TT entries with key16==0 to be fetched Fix the issue where a TT entry with key16==0 would always be reported as a miss. Instead, we'll use depth8 to detect whether the TT entry is occupied. In order to do that, we'll change DEPTH_OFFSET to -7 (depth8==0) to distinguish between an unoccupied entry and the otherwise lowest possible depth, i.e., DEPTH_NONE (depth8==1). To prevent a performance regression, we'll reorder the TT entry fields by the access order of TranspositionTable::probe(). Memory in general works fastest when accessed in sequential order. We'll also match the store order in TTEntry::save() with the entry field order, and re-order the 'if-or' expressions in TTEntry::save() from the cheapest to the most expensive. Finally, as we now have a proper TT entry occupancy test, we'll fix a minor corner case with hashfull reporting. To reproduce: - Use a big hash - Either: a. Start 31 very quick searches (this wraparounds generation to 0); or b. Force generation of the first search to 0. - go depth infinite Before the fix, hashfull would incorrectly report nearly full hash immediately after the search start, since TranspositionTable::hashfull() used to consider only the entry generation and not whether the entry was actually occupied. STC: LLR: 2.95 (-2.94,2.94) {-0.25,1.25} Total: 36848 W: 4091 L: 3898 D: 28859 Ptnml(0-2): 158, 2996, 11972, 3091, 207 https://tests.stockfishchess.org/tests/view/5f3f98d5dc02a01a0c2881f7 LTC: LLR: 2.95 (-2.94,2.94) {0.25,1.25} Total: 32280 W: 1828 L: 1653 D: 28799 Ptnml(0-2): 34, 1428, 13051, 1583, 44 https://tests.stockfishchess.org/tests/view/5f3fe77a87a5c3c63d8f5332 closes official-stockfish#3048 Bench: 3742162 [5b421ab] Enable New Pass Manager for Clang. It's about 1% speedup for Stockfish. Result of 100 runs ================== base (...fish_clang12) = 1946851 +/- 3717 test (./stockfish ) = 1967276 +/- 3408 diff = +20425 +/- 2438 speedup = +0.0105 P(speedup > 0) = 1.0000 Thanks to David Major for making me aware of this part of LLVM development. closes official-stockfish#3346 No functional change [f7f7e38] Disable ThinLTO when using Clang. Benchmarking with current Clang 12 shows that and ThinLTO is a pessimization, see issue official-stockfish#3341. closes official-stockfish#3345 No functional change. [6373f89] Simplify Chess 960 castling a little cleanup, and small speedup (about 0.3%) for Chess 960. Verified with perft on a large set of chess960 positions. Closes official-stockfish#3317 No functional change [3f4d84c] Speed Up Perft Search It speeds up generate<LEGAL>, and thus perft, roughly by 2-3%. closes official-stockfish#3312 No functional change [88974f5] Clean Up Castling in gives_check There is no need to add rto or kto on the Bitboard which represents the pieces. STC: LLR: 2.93 (-2.94,2.94) {-1.25,0.25} Total: 57064 W: 5096 L: 5067 D: 46901 Ptnml(0-2): 202, 3862, 20355, 3931, 182 https://tests.stockfishchess.org/tests/view/6005ea2c6019e097de3efa55 LTC: LLR: 2.92 (-2.94,2.94) {-0.75,0.25} Total: 30088 W: 1094 L: 1052 D: 27942 Ptnml(0-2): 10, 882, 13217, 926, 9 https://tests.stockfishchess.org/tests/view/6006115a6019e097de3efa6e closes official-stockfish#3311 No functional change. [2129ec9] Avoid more expensive legality check speedup of the code, enough to pass STC, failed LTC. Passed STC: LLR: 2.93 (-2.94,2.94) {-0.25,1.25} Total: 68928 W: 6334 L: 6122 D: 56472 Ptnml(0-2): 233, 4701, 24369, 4943, 218 https://tests.stockfishchess.org/tests/view/6002747f6019e097de3ef8dc Failed LTC: LLR: -2.96 (-2.94,2.94) {0.25,1.25} Total: 44560 W: 1702 L: 1675 D: 41183 Ptnml(0-2): 25, 1383, 19438, 1408, 26 https://tests.stockfishchess.org/tests/view/6002a4836019e097de3ef8e3 About 1% speedup: Result of 50 runs ================== base (...kfish.master) = 2237500 +/- 7428 test (...ckfish.patch) = 2267003 +/- 7017 diff = +29503 +/- 4774 speedup = +0.0132 P(speedup > 0) = 1.0000 closes official-stockfish#3304 No functional change. [289fbcc] Use stable sort to make sure bench with TB yields same results everywhere. std::sort() is not stable so different implementations can produce different results: use the stable version instead. Observed for '8/6k1/5r2/8/8/8/1K6/Q7 w - - 0 1' yielding different bench results for gcc and MSVC and 3-4-5 syzygy TB prior to this patch. closes official-stockfish#3083 No functional change. [5a344f1] Fix parallel LTO issues on Windows This adds -save-temps to the linker flags when parallel LTO is used on MinGW/MSYS. fixes official-stockfish#2977 closes official-stockfish#2978 No functional change. [5da0f55] Parallelize Link Time Optimization for GCC, CLANG and MINGW This patch tries to run multiple LTO threads in parallel, speeding up the build process of optimized builds if the -j make parameter is used. This mitigates the longer linking times of optimized builds since the integration of the NNUE code. Roughly 2x build speedup. I've tried a similar patch some two years ago but it ran into trouble with old compiler versions then. Since we're on the C++17 standard now these old compilers should be obsolete. closes official-stockfish#2943 No functional change. [630995a] Remove unnecessay legality check Possible after the recent reording pos.legal(move) check official-stockfish#2941 No functional change. [6f512d9] Do move legality check before pruning. This alllows to simplify the code because the move counter haven't to be decremented later if a move isn't legal. As a side effect now illegal pruned moves doesn't included anymore in move counter. So slightly less pruning and reductions are done. STC: LLR: 2.94 (-2.94,2.94) {-1.50,0.50} Total: 111016 W: 21106 L: 21077 D: 68833 Ptnml(0-2): 1830, 13083, 25736, 12946, 1913 https://tests.stockfishchess.org/tests/view/5f28816fa5abc164f05e4c26 LTC: LLR: 2.94 (-2.94,2.94) {-1.50,0.50} Total: 39264 W: 4909 L: 4843 D: 29512 Ptnml(0-2): 263, 3601, 11854, 3635, 279 https://tests.stockfishchess.org/tests/view/5f297902a5abc164f05e4c8e closes official-stockfish#2906 Bench: 3795876 [7f6f1b8] Remove pawn tables as well, they're unused Bench: 3865928 [c7fa058] We don't need specialized endgame eval where we're going [2e3cd4d] Remove piece lists This patch removes the incrementally updated piece lists from the Position object. This has been tried before but always failed. My reasons for trying again are: * 32-bit systems (including phones) are now much less important than they were some years ago (and are absent from fishtest); * NNUE may have made SF less finely tuned to the order in which moves were generated. STC: LLR: 2.94 (-2.94,2.94) {-1.25,0.25} Total: 55272 W: 5260 L: 5216 D: 44796 Ptnml(0-2): 208, 4147, 18898, 4159, 224 https://tests.stockfishchess.org/tests/view/5fc2986a42a050a89f02c926 LTC: LLR: 2.96 (-2.94,2.94) {-0.75,0.25} Total: 16600 W: 673 L: 608 D: 15319 Ptnml(0-2): 14, 533, 7138, 604, 11 https://tests.stockfishchess.org/tests/view/5fc2f98342a050a89f02c95c closes official-stockfish#3247 Bench: 3940967 [693c459] We don't need PSQTs where we're going [c6c710d] Initial import of simple eval
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Clang has a new optimization pass manager activated by
-fexperimental-new-pass-manager. It's been in development
for several years, and by now generally performing better
than the default. It is planned to be the new default in
Clang 13, but even versions before that perform better with it,
and it's stable enough that major software like Firefox has
been using it already:
https://bugzilla.mozilla.org/show_bug.cgi?id=1619461
It's about 1% speedup for Stockfish.
Result of 100 runs
base (...fish_clang12) = 1946851 +/- 3717
test (./stockfish ) = 1967276 +/- 3408
diff = +20425 +/- 2438
speedup = +0.0105
P(speedup > 0) = 1.0000
CPU: 4 x Intel(R) Xeon(R) CPU E3-1240 v3 @ 3.40GHz
Hyperthreading: on
Thanks to David Major for making me aware of this part
of LLVM development.