Skip to content
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

Run branch cleanup after copy prop #59290

Merged
merged 1 commit into from Mar 19, 2019

Conversation

Projects
None yet
7 participants
@oli-obk
Copy link
Contributor

commented Mar 19, 2019

This is preliminary work for #59288 (comment) which gets rid of if in the HIR.

cc @rust-lang/wg-mir-opt @Centril

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Mar 19, 2019

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@davidtwco

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

@Centril

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

@bors r=davidtwco rollup

@bors

This comment was marked as outdated.

Copy link
Contributor

commented Mar 19, 2019

📌 Commit ab41023 has been approved by davidtwco

@davidtwco

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

@bors r+

14 seconds too late.

@bors

This comment was marked as outdated.

Copy link
Contributor

commented Mar 19, 2019

💡 This pull request was already approved, no need to approve it again.

  • There's another pull request that is currently being tested, blocking this pull request: #58805
@bors

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

📌 Commit ab41023 has been approved by davidtwco

Centril added a commit to Centril/rust that referenced this pull request Mar 19, 2019

Rollup merge of rust-lang#59290 - oli-obk:trivial_move_prop, r=davidtwco
Run branch cleanup after copy prop

This is preliminary work for rust-lang#59288 (comment) which gets rid of `if` in the HIR.

cc @rust-lang/wg-mir-opt 	@Centril

bors added a commit that referenced this pull request Mar 19, 2019

Auto merge of #59293 - Centril:rollup, r=Centril
Rollup of 11 pull requests

Successful merges:

 - #56348 (Add todo!() macro)
 - #57729 (extra testing of how NLL handles wildcard type `_`)
 - #57847 (dbg!() without parameters)
 - #58778 (Implement ExactSizeIterator for ToLowercase and ToUppercase)
 - #58812 (Clarify distinction between floor() and trunc())
 - #58939 (Fix a tiny error in documentation of std::pin.)
 - #59116 (Be more discerning on when to attempt suggesting a comma in a macro invocation)
 - #59252 (add self to mailmap)
 - #59275 (Replaced self-reflective explicit types with clearer `Self` or `Self::…` in stdlib docs)
 - #59280 (Stabilize refcell_map_split feature)
 - #59290 (Run branch cleanup after copy prop)

Failed merges:

r? @ghost
@varkor

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

Is running this pass expected to have any (negative) effect on compilation times?

@oli-obk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 19, 2019

We've been running it after "const propagation" (a pass that just lints and does not actually propagate anything) already. This is just a second run after copy prop (which is semi-expensive). The pass itself is very cheap (it just iterates over all the blocks' terminators and checks for constants, there's no larger analysis going on).

I think we'd rather end up seing build time improvements since more code is thrown away before llvm ever sees it.

@bors bors merged commit ab41023 into rust-lang:master Mar 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.