rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
99.2k stars 12.81k forks source link

Confusing error when using `simd_masked_load` with the wrong type #119210

Open RalfJung opened 11 months ago

RalfJung commented 11 months ago

I tried this code:

#![feature(portable_simd, platform_intrinsics, adt_const_params, core_intrinsics)]
#![allow(incomplete_features, internal_features)]
use std::intrinsics::simd as intrinsics;
use std::ptr;
use std::simd::prelude::*;

pub fn simd_masked_loadstore() {
    let val = 42u8;
    let ptrs: Simd<*const u8, 4> =
        Simd::from_array([ptr::null(), ptr::addr_of!(val), ptr::addr_of!(val), ptr::addr_of!(val)]);
    let default = i8x4::splat(0);
    let mask = i8x4::from_array([0, !0, 0, !0]);
    let vals = unsafe { intrinsics::simd_masked_load(mask, ptrs, default) };
    assert_eq!(vals, i8x4::from_array([0, 42, 0, 42]),);
}

This errors, saying:

error[E0511]: invalid monomorphization of `simd_masked_load` intrinsic:
expected element type `i8` of second argument `std::simd::Simd<*const u8, 4>`
to be a pointer to the element type `i8` of the first argument `std::simd::Simd<i8, 4>`, found `i8` != `*_ i8`

However, the second argument has element type *const u8, even though the error says it has element type i8.

Actually the second argument should be a pointer. So talking about its "element type" doesn't even make a ton of sense.

Cc @farnoy

farnoy commented 11 months ago

I think there are many problems like this with multiple simd_ intrinsics and that's why it wasn't seen as a blocker for introducing these two new intrinsics recently.

We could use this issue to track improvements in this area. IMO, it would make sense to lift the validations that we're doing in the LLVM codegen crate, like these, into HIR, which is currently very basic and doesn't catch these monomorphization issues. I don't know exactly what would be involved though.

cc @calebzulawski @workingjubilee

calebzulawski commented 11 months ago

I support lifting all of the type checking, because I really don't like writing the type checking in the codegen crate.

workingjubilee commented 11 months ago

Yeah, I frankly have no idea why this wasn't done long ago, before I even started participating.