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

Use ecx for const-prop local storage #62012

Merged
merged 3 commits into from
Jun 24, 2019
Merged

Use ecx for const-prop local storage #62012

merged 3 commits into from
Jun 24, 2019

Conversation

wesleywiser
Copy link
Member

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 21, 2019
Centril
Centril reviewed Jun 21, 2019
View reviewed changes
src/librustc_mir/transform/const_prop.rs Outdated Show resolved Hide resolved
src/librustc_mir/transform/const_prop.rs Outdated Show resolved Hide resolved
oli-obk
oli-obk requested changes Jun 21, 2019
View reviewed changes
Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

r=me with @Centril's and my nits addressed

src/librustc_mir/transform/const_prop.rs Outdated Show resolved Hide resolved
src/librustc_mir/transform/const_prop.rs Outdated Show resolved Hide resolved
src/librustc_mir/transform/const_prop.rs Outdated Show resolved Hide resolved
src/librustc_mir/transform/const_prop.rs Outdated Show resolved Hide resolved
@oli-obk oli-obk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 21, 2019
@oli-obk oli-obk mentioned this pull request Jun 21, 2019
Closed
@oli-obk
Copy link
Contributor

oli-obk commented Jun 21, 2019

@bors try

let's see how this impacts perf

@bors
Copy link
Contributor

bors commented Jun 21, 2019

⌛ Trying commit c686130 with merge dcd84aa...

bors added a commit that referenced this pull request Jun 21, 2019
@bors
Auto merge of #62012 - wesleywiser:const_prop_use_ecx, r=<try>
dcd84aa 
Use ecx for const-prop local storage

r? @oli-obk
@oli-obk
Copy link
Contributor

oli-obk commented Jun 21, 2019

@rust-timer build dcd84aa

@rust-timer
Copy link
Collaborator

Success: Queued dcd84aa with parent 56a12b2, comparison URL.

@bors
Copy link
Contributor

bors commented Jun 21, 2019

☀️ Try build successful - checks-travis
Build commit: dcd84aa

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit dcd84aa, comparison URL.

@wesleywiser
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Jun 22, 2019

⌛ Trying commit cef4561c9ac8017e502a040cba083973097bff2a with merge b9376e62aea7c07ac50bf6929abf4931611b10a9...

@wesleywiser
Copy link
Member Author

@rust-timer build b9376e62aea7c07ac50bf6929abf4931611b10a9

@rust-timer
Copy link
Collaborator

Success: Queued b9376e62aea7c07ac50bf6929abf4931611b10a9 with parent d4d5d67, comparison URL.

@bors
Copy link
Contributor

bors commented Jun 22, 2019

☀️ Try build successful - checks-travis
Build commit: b9376e62aea7c07ac50bf6929abf4931611b10a9

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit b9376e62aea7c07ac50bf6929abf4931611b10a9, comparison URL.

@wesleywiser
Copy link
Member Author

That didn't really help. I'm reverting that.

@wesleywiser
Copy link
Member Author

@oli-obk Are you ok with the performance?

@oli-obk
Copy link
Contributor

oli-obk commented Jun 24, 2019

Yes, I believe now that we can start reusing const eval logic, we can improve the readability of the const propagator enough to warrant the perf regression. I also think that some of these reuse-changes will improve perf or at least expose perf problems in const eval that we can then address.

@bors r+

@bors
Copy link
Contributor

bors commented Jun 24, 2019

📌 Commit c686130 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 24, 2019
@bors
Copy link
Contributor

bors commented Jun 24, 2019

⌛ Testing commit c686130 with merge 166c49d...

bors added a commit that referenced this pull request Jun 24, 2019
@bors
Auto merge of #62012 - wesleywiser:const_prop_use_ecx, r=oli-obk
166c49d 
Use ecx for const-prop local storage

r? @oli-obk
@bors
Copy link
Contributor

bors commented Jun 24, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: oli-obk
Pushing 166c49d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 24, 2019
@bors bors merged commit c686130 into rust-lang:master Jun 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Centril Centril Centril left review comments

@oli-obk oli-obk oli-obk requested changes

Assignees

@oli-obk oli-obk

Labels
merged-by-bors This PR was explicitly merged by bors. 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.
6 participants
@wesleywiser @oli-obk @bors @rust-timer @Centril @rust-highfive