Repository navigation
Index argument for simd_insert no longer a const #85105
Description
I tried this code:
let a = _mm_setr_epi16(0, 1, 2, 3, 4, 5, 6, 7);
let r = _mm_insert_epi16(a, 9, 0);
let e = _mm_setr_epi16(9, 1, 2, 3, 4, 5, 6, 7);
assert_eq_m128i(r, e);I expected to see this happen: The index argument 9 is given as direct constant to the simd_insert intrinsic.
Instead, this happened: The cast of the index argument to u32 is stored in a local and then this local is passed to simd_insert. This causes cg_clif to error out as it requires a constant.
#[rustc_legacy_const_generics(2)]
pub unsafe fn _mm_insert_epi16<const IMM8: i32>(a: __m128i, i: i32) -> __m128i {
static_assert_imm3!(IMM8);
transmute(simd_insert(a.as_i16x8(), IMM8 as u32, i as i16))
}[src/constant.rs:450] fx.mir = Body {
basic_blocks: [
BasicBlockData {
statements: [
StorageLive(_3),
StorageLive(_4),
StorageLive(_5),
_5 = _1,
StorageLive(_9),
StorageLive(_10),
_10 = move _5,
_9 = _10,
StorageDead(_10),
],
terminator: Some(
Terminator {
source_info: // ...
kind: _4 = std::intrinsics::transmute::<std::arch::x86_64::__m128i, core::core_arch::simd::i16x8>(move _9) -> bb3,
},
),
is_cleanup: false,
},
BasicBlockData {
statements: [
StorageDead(_7),
StorageDead(_6),
StorageDead(_4),
],
terminator: Some(
Terminator {
source_info: // ...
kind: _0 = std::intrinsics::transmute::<core::core_arch::simd::i16x8, std::arch::x86_64::__m128i>(move _3) -> bb2,
},
),
is_cleanup: false,
},
BasicBlockData {
statements: [
StorageDead(_3),
],
terminator: Some(
Terminator {
source_info: // ...
kind: return,
},
),
is_cleanup: false,
},
BasicBlockData {
statements: [
StorageDead(_9),
StorageDead(_5),
StorageLive(_6),
_6 = const IMM8 as u32 (Misc),
StorageLive(_7),
StorageLive(_8),
_8 = _2,
_7 = move _8 as i16 (Misc),
StorageDead(_8),
],
terminator: Some(
Terminator {
source_info: // ...
kind: _3 = core::core_arch::simd_llvm::simd_insert::<core::core_arch::simd::i16x8, i16>(move _4, move _6, move _7) -> bb1,
},
),
is_cleanup: false,
},
],
phase: Optimization,
source: MirSource {
instance: Item(
WithOptConstParam {
did: DefId(2:13246 ~ core[8231]::core_arch::x86::sse2::_mm_insert_epi16),
const_param_did: None,
},
),
promoted: None,
},
local_decls: [
LocalDecl {
ty: std::arch::x86_64::__m128i,
// ...
},
LocalDecl {
ty: std::arch::x86_64::__m128i,
// ...
},
LocalDecl {
ty: i32,
// ...
},
LocalDecl {
ty: core::core_arch::simd::i16x8,
// ...
},
LocalDecl {
ty: core::core_arch::simd::i16x8,
// ...
},
LocalDecl {
ty: std::arch::x86_64::__m128i,
// ...
},
LocalDecl {
ty: u32,
// ...
},
LocalDecl {
ty: i16,
// ...
},
LocalDecl {
ty: i32,
// ...
},
LocalDecl {
ty: std::arch::x86_64::__m128i,
// ...
},
LocalDecl {
ty: std::arch::x86_64::__m128i,
// ...
},
],
var_debug_info: [
VarDebugInfo {
name: "a",
source_info: // ...
value: _1,
},
VarDebugInfo {
name: "i",
source_info: // ...
value: _2,
},
VarDebugInfo {
name: "self",
source_info: // ...
value: _5,
},
VarDebugInfo {
name: "self",
source_info: // ...
value: _10,
},
],
required_consts: [
const core::core_arch::macros::ValidateConstImm::<IMM8, 0_i32, 7_i32>::VALID,
],
// ...
}
[src/constant.rs:451] operand = move _6Meta
rustc --version --verbose:
rustc 1.54.0-nightly (881c1ac40 2021-05-08)
binary: rustc
commit-hash: 881c1ac408d93bb7adaa3a51dabab9266e82eee8
commit-date: 2021-05-08
host: x86_64-unknown-linux-gnu
release: 1.54.0-nightly
LLVM version: 12.0.0
Activity
bjorn3 commentedon May 9, 2021
I worked around this in bjorn3/rustc_codegen_cranelift@bcc68c2 by walking the MIR to find a single defining use of the local that should contain the constant argument.
oli-obk commentedon Jun 8, 2021
I guess we should give simd_insert some generic const arguments? So instead of
fn simd_insert<T, E>(x: T, idx: u32, y: E) -> T;we would have
fn simd_insert<T, E, const IDX: u32>(x: T, y: E) -> T;does that fit how that index works? cc @lqd does SIMD have a team to ping?
lqd commentedon Jun 8, 2021
There is a zulip stream for stdarch but it's pretty much inactive (opening issues in https://github.com/rust-lang/stdarch does work).
For pings, I believe @Amanieu would be the main person to contact for
stdarch.For this issue in particular, I do wonder if
simd_insertmay need a similar treatment to what Ralf did withsimd_shufflerecently (but simpler; and for different reasons obviously) if this needs to be fixed ?Nugine commentedon Nov 19, 2022
Using
inline_constmay fix this issue.oli-obk commentedon Nov 21, 2022
I think we should still enforce this at the signature level by making it a const generic parameter. If anyone wants to be mentored on this, I'd be happy to do that.
We'd roll it out with minimum annoyance by creating a
that we can then add incrementally to all use sites (mostly the stdsimd submodule). Updating the submodule at the same time as the intrinsic would be super annoying to do, so let's not ^^
lqd commentedon Nov 21, 2022
@oli-obk will it be possible to cast the
i32const generic parameter that the intrinsics use, to thissimd_insert_new'su32const generic parameter without locals ? (would it require adding post-monomorphized conversion at eachsimd_insertcall site, orgeneric_const_exprs?)oli-obk commentedon Nov 21, 2022
we could just make the
const N: u32bei32instead, would that work? or are both signed and unsigned versions needed?lqd commentedon Nov 21, 2022
I'm not sure, I thought
simd_insertwas one of the LLVM extern C intrinsics that stdarch calls directly, but it's a"platform-intrinsic"so we may have more leeway about its signature.oli-obk commentedon Nov 21, 2022
there is no direct calls anywhere. We are generating the llvm intrinsic call during codegen. We can pick whatever signature we need from the Rust side and translate during LLVM codegen.
bjorn3 commentedon Jan 10, 2025
For all
simd_*intrinsics this has been fixed now, though it is not enforced in any backend other than cg_clif. For LLVM intrinsics there are still some cases remaining however. (search formir_operand_get_const_valin cg_clif's source)