-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Rollup of 9 pull requests #33632
Merged
Merged
Rollup of 9 pull requests #33632
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
Member
Manishearth
commented
May 14, 2016
- Successful merges: Only break critical edges where actually needed #33544, [MIR] Enhance the SimplifyCfg pass to merge consecutive blocks #33552, Don't use env::current_exe with libbacktrace #33554, Remove unification despite ambiguity in projection #33555, Use symlink_metadata in tidy to avoid panicking on broken symlinks. #33560, [MIR trans] Optimize trans for biased switches #33566, Support references to outer type params for assoc consts #33572, trans-collector: Assorted fixes and refactorings needed for making trans collector-driven. #33574, Plumb inference obligations through selection, take 2 #33576
- Failed merges:
Currently, to prepare for MIR trans, we break _all_ critical edges, although we only actually need to do this for edges originating from a call that gets translated to an invoke instruction in LLVM. This has the unfortunate effect of undoing a bunch of the things that SimplifyCfg has done. A particularly bad case arises when you have a C-like enum with N variants and a derived PartialEq implementation. In that case, the match on the (&lhs, &rhs) tuple gets translated into nested matches with N arms each and a basic block each, resulting in N² basic blocks. SimplifyCfg reduces that to roughly 2*N basic blocks, but breaking the critical edges means that we go back to N². In nickel.rs, there is such an enum with roughly N=800. So we get about 640K basic blocks or 2.5M lines of LLVM IR. LLVM takes a while to reduce that to the final "disr_a == disr_b". So before this patch, we had 2.5M lines of IR with 640K basic blocks, which took about about 3.6s in LLVM to get optimized and translated. After this patch, we get about 650K lines with about 1.6K basic blocks and spent a little less than 0.2s in LLVM. cc rust-lang#33111
Currently, all switches in MIR are exhausitive, meaning that we can have a lot of arms that all go to the same basic block, the extreme case being an if-let expression which results in just 2 possible cases, be might end up with hundreds of arms for large enums. To improve this situation and give LLVM less code to chew on, we can detect whether there's a pre-dominant target basic block in a switch and then promote this to be the default target, not translating the corresponding arms at all. In combination with rust-lang#33544 this makes unoptimized MIR trans of nickel.rs as fast as using old trans and greatly improves the times for optimized builds, which are only 30-40% slower instead of ~300%. cc rust-lang#33111
…:empty() where appropriate.
If the path we give to libbacktrace doesn't actually correspond to the current process, libbacktrace will segfault *at best*. cc rust-lang#21889
… r=Aatch Only break critical edges where actually needed Currently, to prepare for MIR trans, we break _all_ critical edges, although we only actually need to do this for edges originating from a call that gets translated to an invoke instruction in LLVM. This has the unfortunate effect of undoing a bunch of the things that SimplifyCfg has done. A particularly bad case arises when you have a C-like enum with N variants and a derived PartialEq implementation. In that case, the match on the (&lhs, &rhs) tuple gets translated into nested matches with N arms each and a basic block each, resulting in N² basic blocks. SimplifyCfg reduces that to roughly 2*N basic blocks, but breaking the critical edges means that we go back to N². In nickel.rs, there is such an enum with roughly N=800. So we get about 640K basic blocks or 2.5M lines of LLVM IR. LLVM takes a while to reduce that to the final "disr_a == disr_b". So before this patch, we had 2.5M lines of IR with 640K basic blocks, which took about about 3.6s in LLVM to get optimized and translated. After this patch, we get about 650K lines with about 1.6K basic blocks and spent a little less than 0.2s in LLVM. cc rust-lang#33111 r? @Aatch
[MIR] Enhance the SimplifyCfg pass to merge consecutive blocks Updated from rust-lang#30238, including the changes suggested by @Aatch.
…hton Don't use env::current_exe with libbacktrace If the path we give to libbacktrace doesn't actually correspond to the current process, libbacktrace will segfault *at best*. cc rust-lang#21889 r? @alexcrichton cc @semarie
…sakis Remove unification despite ambiguity in projection Turns out that closures aren't explicitly considered in `project.rs`, so the ambiguity handling w.r.t. closures can just be removed as the change done in `select.rs` covers it. r? @nikomatsakis
Use symlink_metadata in tidy to avoid panicking on broken symlinks. r? @alexcrichton
[MIR trans] Optimize trans for biased switches Currently, all switches in MIR are exhausitive, meaning that we can have a lot of arms that all go to the same basic block, the extreme case being an if-let expression which results in just 2 possible cases, be might end up with hundreds of arms for large enums. To improve this situation and give LLVM less code to chew on, we can detect whether there's a pre-dominant target basic block in a switch and then promote this to be the default target, not translating the corresponding arms at all. In combination with rust-lang#33544 this makes unoptimized MIR trans of nickel.rs as fast as using old trans and greatly improves the times for optimized builds, which are only 30-40% slower instead of ~300%. cc rust-lang#33111
Support references to outer type params for assoc consts Fixes rust-lang#28809 r? @eddyb
… r=nikomatsakis trans-collector: Assorted fixes and refactorings needed for making trans collector-driven. As the title says. The messages on the individual commits should do a good job of explaining what they are about. r? @nikomatsakis
Plumb inference obligations through selection, take 2 Using a `SnapshotVec` and dumping inferred obligations into `Vtable` variants. r? @nikomatsakis
(rust_highfive has picked a reviewer for you, use r? to override) |
@bors r+ p=10 |
📌 Commit 61d87f0 has been approved by |
bors
added a commit
that referenced
this pull request
May 14, 2016
This was referenced May 14, 2016
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.