-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
enable the mergefunc pass #9536
Comments
This is broken, and results in poor performance due to the undefined behaviour in the LLVM IR. LLVM's `mergefunc` is a *much* better way of doing this since it merges based on the equality of the bytecode. For example, consider `std::repr`. It generates different code per type, but is not included in the type bounds of generics. The `mergefunc` pass works for most of our code but currently hits an assert on libstd. It is receiving attention upstream so it will be ready soon, but I don't think removing this broken code should wait any longer. I've opened #9536 about enabling it by default. Closes #8651 Closes #3547 Closes #2537 Closes #6971 Closes #9222
No change that I know of. |
The assert we hit before is fixed in LLVM upstream (even in the version that comes with rustc), but depending on when the pass is run, we hit a different bug due to mergefunc ignoring range metadata. A fix for that is at: http://reviews.llvm.org/D4062 |
I've built rustc with the aforementioned fix, once with MergeFunc being run early (that's what the patch for clang that comes with LLVM does), and once with MergeFunc being run late (using Build command: CPU (user) times, best of three runs:
Sizes:
So running MergeFunc early like in the clang patch isn't nearly as good as running it late. I also tried to enable usage of global aliases instead of just thunks when merging functions, but that crashes. |
Fixes #9536 --- From #9536 (comment): I've built rustc with the aforementioned fix, once with MergeFunc being run early (that's what the patch for clang that comes with LLVM does), and once with MergeFunc being run late (using `-C passes=mergefunc`). Here are some time/code size measurements I made with them: Build command: `rustc -O -o /dev/null --emit asm .../lib.rs` CPU (user) times, best of three runs: Crate | No MergeFunc | Early MergeFunc | Late MergeFunc -------------|--------------|-----------------|--------------- core | 5.380s | 5.476s | 5.364s collections | 1.884s | 1.856s | 1.892s native | 7.200s | 7.356s | 7.108s rustc | 3m23.584s | 3m28.120s | 3m21.820s std | 13.888s | 13.976s | 13.848s syntax | 48.992s | 47.752s | 48.372s Sizes: Crate | No MergeFunc | Early MergeFunc | Late MergeFunc -----------------------------------------|-------------:|--------------------:|-------------------: lib | 237037581 | 236005998 (-0.44%) | 234708744 (-0.98%) libarena-063bff73-0.11.0-pre.so | 60398 | 60393 (-0.01%) | 60394 (-0.01%) libcollections-d412c0c4-0.11.0-pre.so | 971566 | 971772 (+0.02%) | 971691 (+0.01%) libdebug-1e940314-0.11.0-pre.so | 181352 | 181514 (+0.09%) | 181363 (+0.01%) libflate-92afea7e-0.11.0-pre.so | 137837 | 137869 (+0.02%) | 137837 (+0.00%) libfmt_macros-5125f3bd-0.11.0-pre.so | 132733 | 134598 (+1.41%) | 132465 (-0.20%) libgetopts-c94737d1-0.11.0-pre.so | 158851 | 157427 (-0.90%) | 158272 (-0.36%) libgraphviz-7b3cf89d-0.11.0-pre.so | 53337 | 53178 (-0.30%) | 53337 (+0.00%) liblog-cd053230-0.11.0-pre.so | 85993 | 86017 (+0.03%) | 85780 (-0.25%) libnative-1fb5e2c0-0.11.0-pre.so | 635785 | 639352 (+0.56%) | 621184 (-2.30%) libregex-77385931-0.11.0-pre.so | 450538 | 450741 (+0.05%) | 449504 (-0.23%) librustc-d252d482-0.11.0-pre.so | 51583741 | 51227703 (-0.69%) | 50930784 (-1.27%) librustdoc-6ecbf63e-0.11.0-pre.so | 4557104 | 4501896 (-1.21%) | 4394409 (-3.57%) libserialize-0352aab7-0.11.0-pre.so | 1126096 | 1115503 (-0.94%) | 1101734 (-2.16%) libstd-59beb4f7-0.11.0-pre.so | 4499529 | 4488879 (-0.24%) | 4477710 (-0.48%) libsync-305341d2-0.11.0-pre.so | 306767 | 312211 (+1.77%) | 304086 (-0.87%) libsyntax-555559ea-0.11.0-pre.so | 6699751 | 6632291 (-1.01%) | 6596232 (-1.55%) libterm-4e4945a5-0.11.0-pre.so | 389390 | 392689 (+0.85%) | 385525 (-0.99%) libtest-a79f950d-0.11.0-pre.so | 740161 | 730673 (-1.28%) | 734534 (-0.76%) libtime-4bb3739b-0.11.0-pre.so | 131518 | 132830 (+1.00%) | 131514 (-0.00%) rustlib | 164131038 | 163594366 (-0.33%) | 162796293 (-0.81%) x86_64-unknown-linux-gnu | 164119867 | 163583195 (-0.33%) | 162785122 (-0.81%) lib | 164115771 | 163579099 (-0.33%) | 162781026 (-0.81%) liballoc-1085c790-0.11.0-pre.rlib | 1094410 | 1094444 (+0.00%) | 1094438 (+0.00%) libarena-063bff73-0.11.0-pre.rlib | 312324 | 312152 (-0.06%) | 312210 (-0.04%) libarena-063bff73-0.11.0-pre.so | 60394 | 60394 (+0.00%) | 60394 (+0.00%) libcollections-d412c0c4-0.11.0-pre.rlib | 7048646 | 7049094 (+0.01%) | 7048856 (+0.00%) libcollections-d412c0c4-0.11.0-pre.so | 971575 | 971771 (+0.02%) | 971681 (+0.01%) libcompiler-rt.a | 573802 | 573802 (+0.00%) | 573802 (+0.00%) libcore-c5ed6fb4-0.11.0-pre.rlib | 24204746 | 24209820 (+0.02%) | 24187602 (-0.07%) libdebug-1e940314-0.11.0-pre.rlib | 876616 | 878488 (+0.21%) | 876746 (+0.01%) libdebug-1e940314-0.11.0-pre.so | 181352 | 181509 (+0.09%) | 181353 (+0.00%) libflate-92afea7e-0.11.0-pre.rlib | 175062 | 175074 (+0.01%) | 175082 (+0.01%) libflate-92afea7e-0.11.0-pre.so | 137837 | 137869 (+0.02%) | 137837 (+0.00%) libfmt_macros-5125f3bd-0.11.0-pre.so | 132724 | 134599 (+1.41%) | 132469 (-0.19%) libfourcc-cc0e8bf1-0.11.0-pre.so | 125828 | 126084 (+0.20%) | 125827 (-0.00%) libgetopts-c94737d1-0.11.0-pre.rlib | 864664 | 853040 (-1.34%) | 862548 (-0.24%) libgetopts-c94737d1-0.11.0-pre.so | 158855 | 157425 (-0.90%) | 158275 (-0.37%) libglob-eafe1d22-0.11.0-pre.rlib | 951370 | 944674 (-0.70%) | 946734 (-0.49%) libglob-eafe1d22-0.11.0-pre.so | 159130 | 157385 (-1.10%) | 156791 (-1.47%) libgraphviz-7b3cf89d-0.11.0-pre.rlib | 269600 | 269062 (-0.20%) | 269560 (-0.01%) libgraphviz-7b3cf89d-0.11.0-pre.so | 53334 | 53176 (-0.30%) | 53337 (+0.01%) libgreen-ca0d0b80-0.11.0-pre.rlib | 1374120 | 1389510 (+1.12%) | 1361696 (-0.90%) libgreen-ca0d0b80-0.11.0-pre.so | 372435 | 377929 (+1.48%) | 370991 (-0.39%) libhexfloat-3b978f48-0.11.0-pre.so | 131926 | 132166 (+0.18%) | 131935 (+0.01%) liblibc-4f9a876d-0.11.0-pre.rlib | 617472 | 617472 (+0.00%) | 617472 (+0.00%) liblog-cd053230-0.11.0-pre.rlib | 371190 | 371048 (-0.04%) | 370836 (-0.10%) liblog-cd053230-0.11.0-pre.so | 85996 | 86020 (+0.03%) | 85781 (-0.25%) libmorestack.a | 1388 | 1388 (+0.00%) | 1388 (+0.00%) libnative-1fb5e2c0-0.11.0-pre.rlib | 2233070 | 2264296 (+1.40%) | 2194920 (-1.71%) libnative-1fb5e2c0-0.11.0-pre.so | 635787 | 639341 (+0.56%) | 621184 (-2.30%) libnum-ebe12db7-0.11.0-pre.rlib | 2672318 | 2675292 (+0.11%) | 2669370 (-0.11%) libnum-ebe12db7-0.11.0-pre.so | 398924 | 399357 (+0.11%) | 395821 (-0.78%) librand-2ea8f361-0.11.0-pre.rlib | 1691108 | 1691696 (+0.03%) | 1690264 (-0.05%) libregex-77385931-0.11.0-pre.rlib | 2007348 | 2006050 (-0.06%) | 2003804 (-0.18%) libregex-77385931-0.11.0-pre.so | 450520 | 450790 (+0.06%) | 449535 (-0.22%) libregex_macros-a2216dec-0.11.0-pre.so | 597208 | 569004 (-4.72%) | 568800 (-4.76%) librlibc-d1ece24e-0.11.0-pre.rlib | 12394 | 12394 (+0.00%) | 12394 (+0.00%) librustc-d252d482-0.11.0-pre.so | 51582383 | 51230320 (-0.68%) | 50930784 (-1.26%) librustdoc-6ecbf63e-0.11.0-pre.so | 4557074 | 4501877 (-1.21%) | 4394506 (-3.57%) librustuv-ede8cb89-0.11.0-pre.rlib | 4774956 | 4791366 (+0.34%) | 4732386 (-0.89%) librustuv-ede8cb89-0.11.0-pre.so | 1401710 | 1400237 (-0.11%) | 1386869 (-1.06%) libsemver-e49a2dee-0.11.0-pre.rlib | 392704 | 392434 (-0.07%) | 392940 (+0.06%) libsemver-e49a2dee-0.11.0-pre.so | 71863 | 71847 (-0.02%) | 71860 (-0.00%) libserialize-0352aab7-0.11.0-pre.rlib | 8059698 | 8033972 (-0.32%) | 7989802 (-0.87%) libserialize-0352aab7-0.11.0-pre.so | 1126099 | 1115520 (-0.94%) | 1101721 (-2.16%) libstd-59beb4f7-0.11.0-pre.rlib | 18802728 | 18780212 (-0.12%) | 18743438 (-0.32%) libstd-59beb4f7-0.11.0-pre.so | 4499534 | 4488835 (-0.24%) | 4477677 (-0.49%) libsync-305341d2-0.11.0-pre.rlib | 1377062 | 1400190 (+1.68%) | 1369498 (-0.55%) libsync-305341d2-0.11.0-pre.so | 306762 | 312212 (+1.78%) | 304095 (-0.87%) libsyntax-555559ea-0.11.0-pre.so | 6703330 | 6632254 (-1.06%) | 6596295 (-1.60%) libterm-4e4945a5-0.11.0-pre.rlib | 1503928 | 1512648 (+0.58%) | 1495932 (-0.53%) libterm-4e4945a5-0.11.0-pre.so | 389380 | 392678 (+0.85%) | 385517 (-0.99%) libtest-a79f950d-0.11.0-pre.rlib | 3606962 | 3555410 (-1.43%) | 3589258 (-0.49%) libtest-a79f950d-0.11.0-pre.so | 740255 | 728398 (-1.60%) | 734521 (-0.77%) libtime-4bb3739b-0.11.0-pre.rlib | 847036 | 847178 (+0.02%) | 847194 (+0.02%) libtime-4bb3739b-0.11.0-pre.so | 131516 | 132829 (+1.00%) | 131518 (+0.00%) liburl-b8b5640c-0.11.0-pre.rlib | 647764 | 647558 (-0.03%) | 646896 (-0.13%) liburl-b8b5640c-0.11.0-pre.so | 146616 | 146621 (+0.00%) | 146531 (-0.06%) libuuid-238d8f44-0.11.0-pre.rlib | 359732 | 359682 (-0.01%) | 358936 (-0.22%) libuuid-238d8f44-0.11.0-pre.so | 77110 | 77110 (+0.00%) | 77023 (-0.11%) total | 237081476 | 236049893 (-0.44%) | 234752639 (-0.98%) So running MergeFunc early like in the clang patch isn't nearly as good as running it late. I also tried to enable usage of global aliases instead of just thunks when merging functions, but that crashes.
…ithout_eq_to_nursery, r=xFrednet Moved derive_partial_eq_without_eq to nursery changelog: Moves: Move `derive_partial_eq_without_eq` to `nursery` (now allow-by-default) [rust-lang#9536](rust-lang/rust-clippy#9536) Closes: rust-lang/rust-clippy#9530
Replacing #9222 with this bug to stop tying it to the removal of
type_use
.This pass is almost robust enough, but still hits an assert on libstd. It does actually work on libextra already. There is work happening upstream on it right now, and one of the known issues is inability to create aggregate casts correctly (which is likely the CastInst assert this hits).
The text was updated successfully, but these errors were encountered: