Auto merge of #3206 - RalfJung:simd-bitmask, r=RalfJung

SIMD bitmasks: use 'round up to multiple of 8' rather than 'clamp to at least 8'

This should prepare us better for a future with non-power-of-2 vectors, if they ever happen.

src/helpers.rs

@@ -1188,3 +1188,11 @@ pub(crate) fn simd_element_to_bool(elem: ImmTy<'_, Provenance>) -> InterpResult<
         _ => throw_ub_format!("each element of a SIMD mask must be all-0-bits or all-1-bits"),
     })
 }
+
+// This looks like something that would be nice to have in the standard library...
+pub(crate) fn round_to_next_multiple_of(x: u64, divisor: u64) -> u64 {
+    assert_ne!(divisor, 0);
+    // divisor is nonzero; multiplication cannot overflow since we just divided
+    #[allow(clippy::arithmetic_side_effects)]
+    return (x.checked_add(divisor - 1).unwrap() / divisor) * divisor;
+}

src/machine.rs

@@ -768,11 +768,6 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
         drop(self.profiler.take());
     }
 
-    pub(crate) fn round_up_to_multiple_of_page_size(&self, length: u64) -> Option<u64> {
-        #[allow(clippy::arithmetic_side_effects)] // page size is nonzero
-        (length.checked_add(self.page_size - 1)? / self.page_size).checked_mul(self.page_size)
-    }
-
     pub(crate) fn page_align(&self) -> Align {
         Align::from_bytes(self.page_size).unwrap()
     }

src/shims/intrinsics/simd.rs

@@ -4,8 +4,10 @@ use rustc_middle::{mir, ty, ty::FloatTy};
 use rustc_span::{sym, Symbol};
 use rustc_target::abi::{Endian, HasDataLayout};
 
+use crate::helpers::{
+    bool_to_simd_element, check_arg_count, round_to_next_multiple_of, simd_element_to_bool,
+};
 use crate::*;
-use helpers::{bool_to_simd_element, check_arg_count, simd_element_to_bool};
 
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
@@ -411,7 +413,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let (yes, yes_len) = this.operand_to_simd(yes)?;
                 let (no, no_len) = this.operand_to_simd(no)?;
                 let (dest, dest_len) = this.place_to_simd(dest)?;
-                let bitmask_len = dest_len.max(8);
+                let bitmask_len = round_to_next_multiple_of(dest_len, 8);
 
                 // The mask must be an integer or an array.
                 assert!(
@@ -457,7 +459,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             "bitmask" => {
                 let [op] = check_arg_count(args)?;
                 let (op, op_len) = this.operand_to_simd(op)?;
-                let bitmask_len = op_len.max(8);
+                let bitmask_len = round_to_next_multiple_of(op_len, 8);
 
                 // Returns either an unsigned integer or array of `u8`.
                 assert!(

src/shims/unix/mem.rs

@@ -7,7 +7,7 @@
 //! equivalent. That is the only part we support: no MAP_FIXED or MAP_SHARED or anything
 //! else that goes beyond a basic allocation API.
 
-use crate::*;
+use crate::{helpers::round_to_next_multiple_of, *};
 use rustc_target::abi::Size;
 
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
@@ -89,7 +89,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         }
 
         let align = this.machine.page_align();
-        let map_length = this.machine.round_up_to_multiple_of_page_size(length).unwrap_or(u64::MAX);
+        let map_length = round_to_next_multiple_of(length, this.machine.page_size);
 
         let ptr =
             this.allocate_ptr(Size::from_bytes(map_length), align, MiriMemoryKind::Mmap.into())?;
@@ -123,7 +123,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             return Ok(Scalar::from_i32(-1));
         }
 
-        let length = this.machine.round_up_to_multiple_of_page_size(length).unwrap_or(u64::MAX);
+        let length = round_to_next_multiple_of(length, this.machine.page_size);
 
         let ptr = Machine::ptr_from_addr_cast(this, addr)?;
 

tests/pass/portable-simd.rs

@@ -253,8 +253,6 @@ fn simd_mask() {
     let bitmask = mask.to_bitmask();
     assert_eq!(bitmask, 0b1000);
     assert_eq!(Mask::<i64, 4>::from_bitmask(bitmask), mask);
-
-    // Also directly call intrinsic, to test both kinds of return types.
     unsafe {
         let bitmask1: u8 = simd_bitmask(mask.to_int());
         let bitmask2: [u8; 1] = simd_bitmask(mask.to_int());