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.

Sign up for GitHub

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

Jump to bottom

InstCombine: Don't optimize &mut *x into x #72820

Merged
merged 1 commit into from
Jun 3, 2020
Merged

InstCombine: Don't optimize &mut *x into x #72820

merged 1 commit into from
Jun 3, 2020

Conversation

jonas-schievink
Copy link
Contributor

Fixes #72797

@rust-highfive
Copy link
Collaborator

r? @varkor

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 31, 2020
@jonas-schievink
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented May 31, 2020

⌛ Trying commit 309661e with merge ee7813f5ca0f5a520184f18081c777da904cd1d5...

@bors
Copy link
Contributor

bors commented May 31, 2020

☀️ Try build successful - checks-azure
Build commit: ee7813f5ca0f5a520184f18081c777da904cd1d5 (ee7813f5ca0f5a520184f18081c777da904cd1d5)

@rust-timer
Copy link
Collaborator

Queued ee7813f5ca0f5a520184f18081c777da904cd1d5 with parent 4b1f86a, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit ee7813f5ca0f5a520184f18081c777da904cd1d5, comparison URL.

@jonas-schievink
Copy link
Contributor Author

It actually made things faster? How?! Is this the CGU partitioning thing?

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned varkor Jun 1, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Jun 2, 2020

Yes it is cgu partitioning: https://perf.rust-lang.org/detailed-query.html?commit=ee7813f5ca0f5a520184f18081c777da904cd1d5&base_commit=4b1f86adbe41e8dd4864ca2315f43953dd503bb5&benchmark=script-servo-opt&run_name=incr-patched:%20println

@oli-obk
Copy link
Contributor

oli-obk commented Jun 2, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jun 2, 2020

📌 Commit 309661e has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 2, 2020
@cuviper
Copy link
Member

cuviper commented Jun 2, 2020

Since beta just branched, will this need a backport?

@jonas-schievink
Copy link
Contributor Author

I don't believe this causes issues today.

@cuviper
Copy link
Member

cuviper commented Jun 2, 2020

Ok -- from #72797 I wasn't sure if #72093 caused any actual problem on its own, or just in concert with #72632 (still pending).

@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Jun 3, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 3, 2020
@bors
Auto merge of rust-lang#72935 - Dylan-DPC:rollup-60g3ab6, r=Dylan-DPC
680a4b2 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#72704 (Remote testing fixes)
 - rust-lang#72820 (InstCombine: Don't optimize `&mut *x` into `x`)
 - rust-lang#72848 (Correct generic parameter ordering in error note for E0747)
 - rust-lang#72902 (Add a test to ensure Fuse stays covariant)
 - rust-lang#72921 (Add assert to Vec with_capacity docs)

Failed merges:

r? @ghost
@bors bors merged commit 9c3ac0c into rust-lang:master Jun 3, 2020
@matthewjasper matthewjasper mentioned this pull request Jun 20, 2020
Merged
@jonas-schievink jonas-schievink deleted the instcombine-uninit branch June 22, 2020 00:58
@scottmcm scottmcm mentioned this pull request Aug 3, 2020
Closed
@jonas-schievink jonas-schievink mentioned this pull request Sep 16, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

InstCombine can incorrectly deinitialize locals that are later used
7 participants
@jonas-schievink @rust-highfive @rust-timer @bors @oli-obk @cuviper @varkor