Skip to content

[WIP] Optimize calls to copy_nonoverlapping intrinsic to assignment #81344

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed

Conversation

wesleywiser
Copy link
Member

r? @ghost

Rebased on top of #81238

@wesleywiser wesleywiser added the A-mir-opt Area: MIR optimizations label Jan 24, 2021
@wesleywiser wesleywiser force-pushed the copy_nonoverlapping_opt branch from a528f5f to 49b2a8b Compare January 24, 2021 18:06
@wesleywiser
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@bors
Copy link
Collaborator

bors commented Jan 24, 2021

⌛ Trying commit 49b2a8b with merge 1cf2e748df357932cdf1d2cc07d6000972bf0397...

@bugadani
Copy link
Contributor

Fixes #73258 and I wonder how many more :) Nice!

locals: &LocalDecls<'tcx>,
statements: &mut Vec<Statement<'tcx>>,
) {
if let Some((src, dest, next_bb)) = self.find_copy_nonoverlapping(terminator, locals) {
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: respect optimization fuel

@bors
Copy link
Collaborator

bors commented Jan 24, 2021

☀️ Try build successful - checks-actions
Build commit: 1cf2e748df357932cdf1d2cc07d6000972bf0397 (1cf2e748df357932cdf1d2cc07d6000972bf0397)

@rust-timer
Copy link
Collaborator

Queued 1cf2e748df357932cdf1d2cc07d6000972bf0397 with parent 9a9477f, future comparison URL.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 24, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (1cf2e748df357932cdf1d2cc07d6000972bf0397): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 24, 2021
@bjorn3
Copy link
Member

bjorn3 commented Jan 24, 2021

Many improvements of up to 2.6% with a few small regressions of up to 0.8%.

@@ -25,6 +28,10 @@ impl<'tcx> MirPass<'tcx> for InstCombine {
_ => {}
}
}

if let Some(terminator) = &mut block.terminator {
Copy link
Member

Choose a reason for hiding this comment

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

You should use block.terminator_mut(). The terminator is always Some after mir building.

@nagisa
Copy link
Member

nagisa commented Jan 24, 2021

Shouldn't this be a lower_intrinsics thing, rather than a instcombine?

@nagisa
Copy link
Member

nagisa commented Jan 24, 2021

cc rust-lang/compiler-team#348 and #77511.

@wesleywiser
Copy link
Member Author

Oh, I'm sorry! I'd completely forgotten about that MCP.

Closing in favor of #77511

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-mir-opt Area: MIR optimizations 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.

8 participants