Are the wasm simd shl/shr intrinsics sound?

[RalfJung]

I found this in stdarch:

/// Shifts each lane to the left by the specified number of bits.
///
/// Only the low bits of the shift amount are used if the shift amount is
/// greater than the lane width.
#[inline]
#[cfg_attr(test, assert_instr(i16x8.shl))]
#[target_feature(enable = "simd128")]
#[doc(alias("i16x8.shl"))]
#[stable(feature = "wasm_simd", since = "1.54.0")]
pub fn i16x8_shl(a: v128, amt: u32) -> v128 {
    unsafe { simd_shl(a.as_i16x8(), simd::i16x8::splat(amt as i16)).v128() }
}

However, simd_shl is UB when the shift amount exceeds the bitwidth. So the implementation does not seem to match what the doc comment says?

Cc @alexcrichton @Amanieu @workingjubilee

[hanna-kruppe]

Yes, this is a plain bug, it needs to mask amt to be sound (if LLVM doesn’t exploit the UB then the lowering to wasm opcodes will behave as expected). Similar problem for the right-shift intrinsics using simd_shr.

[alexcrichton]

Yes the documentation reflects the wasm intrinsic, and agreed it's a bug that should be fixed. If codegen regresses then that would ideally lead to an issue to improve it on LLVM's side, but the definition here should nevertheless change even if it regresses slightly.

[alexcrichton]

Also, to dispel any doubt since this is simd-related, this is indeed a problem where with today's nightly:

use core::arch::wasm32::*;

#[no_mangle]
pub extern "C" fn foo(a: v128) -> v128 {
    i8x16_shl(a, 100)
}

compiles to

define dso_local <4 x i32> @foo(<4 x i32> %a) unnamed_addr #0 !dbg !5 {
  ret <4 x i32> undef, !dbg !26
}

[alexcrichton]

(sorry for multiple comments)

I've posted a fix to rust-lang/stdarch#1737 and additionally confirmed that adding this shift doesn't actually affect codegen, so once that's merged and ready I think this can be closed on the update of rust-lang/rust's stdarch submodule.