Skip to content

Test the performance effects of LLVM D80707 #72703

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

Closed
wants to merge 1 commit into from

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented May 28, 2020

Proposed LLVM change: https://reviews.llvm.org/D80707

@rust-highfive
Copy link
Contributor

@cuviper: no appropriate reviewer found, use r? to override

@rust-highfive
Copy link
Contributor

⚠️ Warning ⚠️

  • These commits modify submodules.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 28, 2020
@cuviper
Copy link
Member Author

cuviper commented May 28, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Collaborator

bors commented May 28, 2020

⌛ Trying commit 7bb8b6f with merge 5f884a00ab7292844b781231b742372b81bcd4ef...

@cuviper
Copy link
Member Author

cuviper commented May 28, 2020

I couldn't reproduce the initial 30% (!) performance claim in that change, and neither could Serge when he tried again, so he has now updated the numbers with just a modest 1% reduction in instruction count. Still, if that little bit shows in rustc too, it may be worthwhile to apply.

@bors
Copy link
Collaborator

bors commented May 28, 2020

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

@rust-timer
Copy link
Collaborator

Queued 5f884a00ab7292844b781231b742372b81bcd4ef with parent 4512721, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 5f884a00ab7292844b781231b742372b81bcd4ef, comparison URL.

@cuviper
Copy link
Member Author

cuviper commented Jun 19, 2020

Serge has found that a lot of passes don't actually set that "changed" flag correctly. While he is chasing those down, the cumulative fixes will probably be too much to backport. So, we'll await the change in some future LLVM rebase. :)

@cuviper cuviper closed this Jun 19, 2020
@cuviper cuviper deleted the llvm-D80707 branch October 15, 2022 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants