core::mem::swap used to work, then broke (and later accidentally got unbroken again).

[hatoo]

Expected Behaviour

Codes that uses core::mem::swap

#![cfg_attr(
    target_arch = "spirv",
    no_std,
    feature(register_attr),
    register_attr(spirv)
)]

#[cfg(not(target_arch = "spirv"))]
use spirv_std::macros::spirv;

use spirv_std as _;

#[spirv(vertex)]
pub fn test_vs() {
    let mut x1 = 0.0f32;
    let mut x2 = 1.0f32;

    core::mem::swap(&mut x1, &mut x2);
}

will compile. it's OK in previous version at least b9867d0.

Example & Steps To Reproduce

1. Clone https://github.com/hatoo/rust-gpu-issue/tree/swap-fail (note: swap-fail branch)
2. cargo build and error

> cargo build
   Compiling builder v0.1.0 (C:\Users\hato2\Desktop\rust-gpu-issue\builder)
error: failed to run custom build command for `builder v0.1.0 (C:\Users\hato2\Desktop\rust-gpu-issue\builder)`

Caused by:
  process didn't exit successfully: `C:\Users\hato2\Desktop\rust-gpu-issue\target\debug\build\builder-caf084bc583adeed\build-script-build` (exit code: 1)
  --- stderr
     Compiling shader v0.1.0 (C:\Users\hato2\Desktop\rust-gpu-issue\shader)
  error: Cannot memcpy dynamically sized data
      --> C:\Users\hato2\.rustup\toolchains\nightly-2021-10-26-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\intrinsics.rs:2057:14
       |
  2057 |     unsafe { copy_nonoverlapping(src, dst, count) }
       |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  error: could not compile `shader` due to previous error
  Error: BuildFailed

It's OK in the previous version. Please see https://github.com/hatoo/rust-gpu-issue/tree/swap-succ.

System Info

• Rust: rustc 1.58.0-nightly (29b124802 2021-10-25)
• OS: Windows 11
• GPU: RTX2080ti
• SPIR-V: SPIRV-Tools v2021.3 v2021.3

Backtrace

Backtrace

<backtrace>

[eddyb]

In #716 we updated to a version that supports mem::replace again, thanks to rust-lang/rust#87827, but mem::swap indeed did not get fixed the same way - if someone wants to try and land upstream something like that PR but for mem::swap, feel free to.

I gave up on it because of how rare mem::swap is compared to mem::replace (which gets used everywhere in core, e.g. Option::take(self) is mem::replace(self, None) and range iterators also depend on it).

There's a more general fix at rust-lang/rust#86699 but it requires a bunch of unstable feature-gating stuff I haven't gone back to for.

[oisyn]

Closing as won't fix, it's very unlikely we will address this in the forseeable future.

[eddyb]

This actually works in Rust-GPU 0.4, and more specifically since nightly-2022-02-28, thanks to:

• For MIRI, cfg out the swap vectorization logic from 94212 rust-lang/rust#94412
  _{(the function being modified there, swap_simple, is always used on SPIR-V, and the change that actually unblocked it was ptr::copy_nonoverlapping(y, x, 1) being replaced with a ptr::{read,write} pair)}

But at the same time, I wanted to propose to upstream that they can remove the target_arch = "spirv" hack, as it got brought up in this issue:

• What guarantees are provided about whether the reference/stdlib docs are valid on target architectures? rust-lang/unsafe-code-guidelines#461

Now I'm worried that people may be relying on it, since it works, but I'm not sure how to check.
core itself seems to mostly use for sorting/partitioning-like tasks.

Oh, according to sourcegraph, it's definitely gotten used, oh well.

So we might want to do nothing for now, and only remove the upstream hack if qptr can supersede it.