[do not merge] core::mem::replace using core::mem::swap

[GrigorenkoPV]

Brought up in and closes #154546.
Reverts #83022.
Needs a perf run.

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: @scottmcm, libs
• @scottmcm, libs expanded to 8 candidates
• Random selection from Mark-Simulacrum, scottmcm

[bjorn3]

Have you checked codegen for larger types?

[bjorn3]

Instead of removing this subtest, maybe fix the test expectation?

[GrigorenkoPV]

From what I can tell, this is somewhat of a regression. If I understand correctly, the test checks that we just do a memcpy the pointers/sizes from x into the return value, because we no longer care about x after the function returns. But this is no longer the case after the change to mem::replace. This sounds like it might be a good place to stop right there and say "this is a regression, we're not doing it", but a maybe a perf run is still worth it?

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

[GrigorenkoPV]

Have you checked codegen for larger types?

Haven't. Will.

---

@rustbot author

I also ran into a very strange failure in miri, let's see if CI gets it too.

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rust-log-analyzer]

The job x86_64-gnu-gcc failed! Check out the build log: (web) (plain enhanced) (plain)

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

+         _3 = copy _2;
+         StorageLive(_4);
+         _4 = &raw mut (*_1);
+         StorageLive(_5);
+         _5 = &raw mut _3;
+         _6 = typed_swap_nonoverlapping::<u32>(move _4, move _5) -> [return: bb1, unwind unreachable];
+     }
+ 
+     bb1: {
+         StorageDead(_5);
+         StorageDead(_4);
+         _0 = move _3;
+         StorageDead(_3);
15         return;
16     }
17 }


thread '[mir-opt] tests/mir-opt/pre-codegen/mem_replace.rs' panicked at src/tools/compiletest/src/runtest/mir_opt.rs:84:21:
Actual MIR output differs from expected MIR output /checkout/tests/mir-opt/pre-codegen/mem_replace.mem_replace.PreCodegen.after.panic-unwind.mir
stack backtrace:
   5: __rustc::rust_begin_unwind
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library/std/src/panicking.rs:689:5
   6: core::panicking::panic_fmt
             at /rustc/ad726b5063362ec9897ef3d67452fc5606ee70fa/library/core/src/panicking.rs:80:14

For more information how to resolve CI failures of this job, visit this link.

[RalfJung]

Cc @m-ou-se @scottmcm

[scottmcm]

I think #83022 is still correct here that replace should not be using a swap. Nothing material has changed since those arguments were made, AFAIK.

The only reason that using swap here helps is because I did a bunch of work on it to chunk-polymorphize by default. So maybe the answer is to replicate that chunking approach for replace too? (So that for something like [usize; 100] it does 100 replaces of usizes, with no stack space needed, rather than a couple memcpys of the arrays.)

(Or, even better, if LLVM could just notice what we're doing and optimize it for us.)