miri no longer builds after rust-lang/rust#94469

[rust-highfive]

Hello, this is your friendly neighborhood mergebot.
After merging PR #94469, I observed that the tool miri has failing tests.
A follow-up PR to the repository https://github.com/rust-lang/miri is needed to fix the fallout.

cc @Dylan-DPC, do you think you would have time to do the follow-up work?
If so, that would be great!

[RalfJung]

Ah, now division needs simd_select:

error: unsupported operation: unimplemented intrinsic: simd_select
  --> /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/../../portable-simd/crates/core_simd/src/select.rs:37:18
   |
37 |         unsafe { intrinsics::simd_select(self.to_int(), true_values, false_values) }
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unimplemented intrinsic: simd_select
   |
   = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
           
   = note: inside `core::core_simd::select::<impl std::simd::Mask<i32, 4_usize>>::select::<i32>` at /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/../../portable-simd/crates/core_simd/src/select.rs:37:18
   = note: inside `core::core_simd::ops::<impl std::ops::Div for std::simd::Simd<i32, 4_usize>>::div` at /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/../../portable-simd/crates/core_simd/src/ops.rs:85:17
note: inside `simd_ops_i32` at $DIR/portable-simd.rs:21:16
  --> $DIR/portable-simd.rs:21:16
   |
21 |     assert_eq!(a / b, i32x4::from_array([10, 5, 3, 2]));
   |                ^^^^^
note: inside `main` at $DIR/portable-simd.rs:50:5
  --> $DIR/portable-simd.rs:50:5
   |
50 |     simd_ops_i32();
   |     ^^^^^^^^^^^^^^

@workingjubilee in the docs it says this is equivalent (yes & m) | (no & (m^-1) -- so, both yes and no need to be fully initialized? That's different from the select LLVM operation so I am slightly surprised. (OTOH this intrinsic is by-value and its type would not allow uninit data anyway so I guess the question is somewhat moot.)

[workingjubilee]

Whoops.
Yes, that's the intent, it's also called a "blend" in SIMD jargon. It not having much in common is due to it merely having a shared name from outside the LLVM terminology.

[workingjubilee]

I don't think it strictly has to have both being init, though... for this purpose, it is sufficient to assume that the stated equivalence holds in the case of uninit data only if both sides are also considered implicitly frozen by this operation. One of the purposes IS to allow composing a partially initialized vector with another that fills in the blanks in order to get a fully initialized vector.

The important detail is that the operation is fully branchless: it may be valid to execute it using branches, but it does not need any to complete.

[RalfJung]

Well, it is still quite relevant whether calling the intrinsic with uninit data in a "dead" element is UB or not.

But for now I will simply make it UB, and then if that is ever a problem we can see what we want to do.

[workingjubilee]

It should not be, though I think that the resulting vector from a simd_select should always be fully initialized as a postcondition, which means it may be UB if both sides of a selection are uninitialized data (and the mask itself has to be initialized). We don't have MaybeUninit vectors yet though.

[RalfJung]

It should not be

In that case, at least on the LLVM side, equivalence with (yes & m) | (no & (m^-1) does not hold -- under that equivalence, if any input is poison, the output will be poison.

I think that the resulting vector from a simd_select should always be fully initialized as a postcondition, which means it may be UB if both sides of a selection are uninitialized data

That seems like an odd requirement, what would motivate this?

(and the mask itself has to be initialized)

Definitely.

We don't have MaybeUninit vectors yet though.

Yeah so in a sense the discussion is rather moot currently and maybe I shouldn't have started it. (But by default Miri does not check integer initialization, so this still makes a difference for that case.)

[workingjubilee]

That seems like an odd requirement, what would motivate this?

Because ultimately, simd_select is "really" trying to be
https://www.felixcloutier.com/x86/blendps

And there's a suite of separately-defined LLVM intrinsics... the llvm.vp intrinsics... that we will probably introduce Rust equivalents of if we get around to working with MaybeUninit vectors. And all of those will probably use masked_ or something in their nomenclature.

[RalfJung]

I don't understand how that motivates introducing UB when "forwarding" uninit data.

I expected the type of simd_select with MaybeUninit to be something like simd_select(mask: Mask<N>, yes: MaybeUninitVec<N>, no: MaybeUninitVec<N>) -> MaybeUninitVec<N>. Such an intrinsic could even be safe to call, but requiring the "selected" values to be initialized would prevent that, and I do not understand the benefit of that.

[workingjubilee]

Hmm, perhaps you are right then.