Skip to content

rustc_mir: don't build unused unwind cleanup blocks #43576

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

Merged
merged 5 commits into from
Aug 2, 2017

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Jul 31, 2017

When building a scope exit, don't build unwind cleanup blocks unless they will actually be used by the unwind path of a drop - the unused blocks are removed by SimplifyCfg, but they can cause a significant performance slowdown before they are removed. That fixes #43511.

Also a few other small MIR cleanups & optimizations.

r? @eddyb

arielb1 added 4 commits August 1, 2017 00:12
The unused blocks are removed by SimplifyCfg, but they can cause a
significant performance slowdown before they are removed.
I saw MIR cache invalidation somewhat hot on my profiler when per-BB
indexin was used. That shouldn't matter much, but there is no good
reason not to use an iterator.
Removing nops can allow more basic blocks to be merged, but merging
basic blocks can't allow for more nops to be removed, so we should
remove nops first.

This doesn't matter *that* much, because normally we run SimplifyCfg
several times, but there's no reason not to do it.
@eddyb
Copy link
Member

eddyb commented Aug 1, 2017

r=me with the Travis build fixed

// If we are emitting a `drop` statement, we need to have the cached
// diverge cleanup pads ready in case that drop panics.
let may_panic =
self.scopes[(len - scope_count)..].iter().any(|s| s.drops.iter().any(|s| s.kind.may_panic()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is >100 chars, failing tidy. (I consider a for-loop + break is easier to read here.)

[00:03:50] tidy error: /checkout/src/librustc_mir/build/scope.rs:390: line longer than 100 chars
[00:03:51] some tidy checks failed

@arielb1
Copy link
Contributor Author

arielb1 commented Aug 1, 2017

@bors r=eddyb

@bors
Copy link
Collaborator

bors commented Aug 1, 2017

📌 Commit ce0ca76 has been approved by eddyb

@bors
Copy link
Collaborator

bors commented Aug 1, 2017

⌛ Testing commit ce0ca76 with merge 640cfc8...

bors added a commit that referenced this pull request Aug 1, 2017
rustc_mir: don't build unused unwind cleanup blocks

When building a scope exit, don't build unwind cleanup blocks unless they will actually be used by the unwind path of a drop - the unused blocks are removed by SimplifyCfg, but they can cause a significant performance slowdown before they are removed. That fixes #43511.

Also a few other small MIR cleanups & optimizations.

r? @eddyb
@bors
Copy link
Collaborator

bors commented Aug 2, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing 640cfc8 to master...

@bors bors merged commit ce0ca76 into rust-lang:master Aug 2, 2017
@bors bors mentioned this pull request Aug 2, 2017
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

25% regression in compile time of issue-20936-deep-vector benchmark
4 participants