Skip to content

Commit 66fd8b1

Browse files
Rollup merge of rust-lang#140034 - RalfJung:simd_select_bitmask-padding, r=workingjubilee
simd_select_bitmask: the 'padding' bits in the mask are just ignored Fixes rust-lang#137942: we documented simd_select_bitmask to require the 'padding' bits in the mask (the mask can sometimes be longer than the vector; I am referring to these extra bits as 'padding' here) to be zero, mostly because nobody felt like doing the research for what should be done when they are non-zero. However, codegen is already perfectly happy just ignoring them, so in practice they can have any value. Some of the intrinsic wrappers in stdarch have trouble ensuring that they are zero. So let's just adjust the docs and Miri to permit non-zero 'padding' bits. Cc ```@Amanieu``` ```@workingjubilee```
2 parents 63c46b8 + 31cb737 commit 66fd8b1

File tree

5 files changed

+15
-45
lines changed

5 files changed

+15
-45
lines changed

library/core/src/intrinsics/simd.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -577,11 +577,9 @@ pub unsafe fn simd_select<M, T>(mask: M, if_true: T, if_false: T) -> T;
577577
/// For each element, if the bit in `mask` is `1`, select the element from
578578
/// `if_true`. If the corresponding bit in `mask` is `0`, select the element from
579579
/// `if_false`.
580+
/// The remaining bits of the mask are ignored.
580581
///
581582
/// The bitmask bit order matches `simd_bitmask`.
582-
///
583-
/// # Safety
584-
/// Padding bits must be all zero.
585583
#[rustc_intrinsic]
586584
#[rustc_nounwind]
587585
pub unsafe fn simd_select_bitmask<M, T>(m: M, yes: T, no: T) -> T;

src/tools/miri/src/intrinsics/simd.rs

+1-12
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
506506
};
507507

508508
let dest_len = u32::try_from(dest_len).unwrap();
509-
let bitmask_len = u32::try_from(bitmask_len).unwrap();
510509
for i in 0..dest_len {
511510
let bit_i = simd_bitmask_index(i, dest_len, this.data_layout().endian);
512511
let mask = mask & 1u64.strict_shl(bit_i);
@@ -517,17 +516,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
517516
let val = if mask != 0 { yes } else { no };
518517
this.write_immediate(*val, &dest)?;
519518
}
520-
for i in dest_len..bitmask_len {
521-
// If the mask is "padded", ensure that padding is all-zero.
522-
// This deliberately does not use `simd_bitmask_index`; these bits are outside
523-
// the bitmask. It does not matter in which order we check them.
524-
let mask = mask & 1u64.strict_shl(i);
525-
if mask != 0 {
526-
throw_ub_format!(
527-
"a SIMD bitmask less than 8 bits long must be filled with 0s for the remaining bits"
528-
);
529-
}
530-
}
519+
// The remaining bits of the mask are ignored.
531520
}
532521
// Converts a "vector of bool" into a bitmask.
533522
"bitmask" => {

src/tools/miri/tests/fail/intrinsics/simd-select-bitmask-invalid.rs

-15
This file was deleted.

src/tools/miri/tests/fail/intrinsics/simd-select-bitmask-invalid.stderr

-15
This file was deleted.

src/tools/miri/tests/pass/intrinsics/portable-simd.rs

+13
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,19 @@ fn simd_mask() {
331331
);
332332
assert_eq!(selected1, i32x4::from_array([0, 0, 0, 1]));
333333
assert_eq!(selected2, selected1);
334+
// Non-zero "padding" (the extra bits) is also allowed.
335+
let selected1 = simd_select_bitmask::<u8, _>(
336+
if cfg!(target_endian = "little") { 0b11111000 } else { 0b11110001 },
337+
i32x4::splat(1), // yes
338+
i32x4::splat(0), // no
339+
);
340+
let selected2 = simd_select_bitmask::<[u8; 1], _>(
341+
if cfg!(target_endian = "little") { [0b11111000] } else { [0b11110001] },
342+
i32x4::splat(1), // yes
343+
i32x4::splat(0), // no
344+
);
345+
assert_eq!(selected1, i32x4::from_array([0, 0, 0, 1]));
346+
assert_eq!(selected2, selected1);
334347
}
335348

336349
// Non-power-of-2 multi-byte mask.

0 commit comments

Comments
 (0)