mir-opt: Replace clone on primitives with copy

[scottmcm]

We can't do it for everything, but it would be nice to at least stop making calls to clone methods in debug from things like derived-clones.

r? @ghost

[scottmcm]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 05db2c29b137029c3db4d1dc0653fa3bd4d261bb with merge e3ab4694509bf757e2b190ca236e525005c34285...

[bors]

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

[rust-timer]

Queued e3ab4694509bf757e2b190ca236e525005c34285 with parent bafe8d0, future comparison URL.

[rust-timer]

Finished benchmarking commit (e3ab4694509bf757e2b190ca236e525005c34285): comparison url.

Summary: This benchmark run shows 1 relevant improvement 🎉 to instruction counts.

• Largest improvement in instruction counts: -2.3% on incr-patched: b9b3e592dd cherry picked builds of style-servo debug

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

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

[scottmcm]

Since this is green in perf (not even just neutral, to my surprise) and it avoids function calls for clones in debug (unlike today, where you can see things like call core::clone::impls::<impl core::clone::Clone for u32>::clone), I'd like to get one of the expert's opinions of if this is worth doing:

r? rust-lang/mir-opt

[oli-obk]

We're missing probably the largest chunk of things, and that is calling generic functions that clone their generic parameters. MIR opts will not work there, as they only see the generic function, and the Clone bounds don't allow switching to Copy in that case. This is a recurring problem with other MIR opts, too.

That said, the effects on a real world benchmark are neat though I do wonder how much of that is these benchmarks being old and not having had clippy applied to them (which will tell you to do this transformation in user space, and even on generic things).

[oli-obk]

This should probably live next to is_trivially_sized and should probably be used to speed up is_copy_modulo_regions by avoiding a query call.

[scottmcm]

I do wonder how much of that is these benchmarks being old and not having had clippy applied to them (which will tell you to do this transformation in user space, and even on generic things).

That's why the test I used here is #[derive(Clone)], since that's a case where one will regularly have clone calls on primitive types that a human can't change to a deref.

It's definitely true that this is something that LLVM will reliably inline, so only hitting the obvious things in the generic MIR is ok by me.

I'll go do the move you mentioned in the other comment
@rustbot author

[scottmcm]

Well, I tried short-circuiting in is_copy_modulo_regions, but that somehow makes it unable to prove u32: Copy in a test:

error: internal compiler error: broken MIR in DefId(0:3 ~ params_on_registers[6840]::entry_function) (NoSolution): could not prove Binder(TraitPredicate(<u32 as Copy>, polarity:Positive), [])
   |
   = note: delayed at compiler\rustc_borrowck\src\type_check\canonical.rs:150:13

So I left that out of the change. I definitely don't know enough about queries and such to have any idea why that's happening. It fails in ui\cmse-nonsecure\cmse-nonsecure-entry\params-on-stack.rs having only called my new is_trivially_pure_clone_copy on u32, not on anything else. I opened a zulip thread about it.

@rustbot ready

[bors]

📌 Commit 705b880 has been approved by oli-obk

[bors]

⌛ Testing commit 705b880 with merge a7c0d7302bbc247a044776a9fd08c0cd20a82bab...

[bors]

💔 Test failed - checks-actions

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[matthiaskrgr]

@bors retry auto (dist-apple-various didn't start

[bors]

⌛ Testing commit 705b880 with merge f8311752f15ee207d04dd52ad61c693c0d2944df...

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[bors]

💔 Test failed - checks-actions

[scottmcm]

@bors retry -- this time it's dist-x86_64-apple that didn't start

[bors]

⌛ Testing commit 705b880 with merge c5a43b8...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing c5a43b8 to master...

[rust-timer]

Finished benchmarking commit (c5a43b8): comparison url.

Summary: This benchmark run did not return any relevant results. 38 results were found to be statistically significant but too small to be relevant.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression