Skip to content

Commit 4c0bc8f

Browse files
authored
Rollup merge of #147355 - sayantn:masked-loads, r=RalfJung,bjorn3
Add alignment parameter to `simd_masked_{load,store}` This PR adds an alignment parameter in `simd_masked_load` and `simd_masked_store`, in the form of a const-generic enum `core::intrinsics::simd::SimdAlign`. This represents the alignment of the `ptr` argument in these intrinsics as follows - `SimdAlign::Unaligned` - `ptr` is unaligned/1-byte aligned - `SimdAlign::Element` - `ptr` is aligned to the element type of the SIMD vector (default behavior in the old signature) - `SimdAlign::Vector` - `ptr` is aligned to the SIMD vector type The main motive for this is stdarch - most vector loads are either fully aligned (to the vector size) or unaligned (byte-aligned), so the previous signature doesn't cut it. Now, stdarch will mostly use `SimdAlign::Unaligned` and `SimdAlign::Vector`, whereas portable-simd will use `SimdAlign::Element`. - [x] `cg_llvm` - [x] `cg_clif` - [x] `miri`/`const_eval` ## Alternatives Using a const-generic/"const" `u32` parameter as alignment (and we error during codegen if this argument is not a power of two). This, although more flexible than this, has a few drawbacks - If we use an const-generic argument, then portable-simd somehow needs to pass `align_of::<T>()` as the alignment, which isn't possible without GCE - "const" function parameters are just an ugly hack, and a pain to deal with in non-LLVM backends We can remedy the problem with the const-generic `u32` parameter by adding a special rule for the element alignment case (e.g. `0` can mean "use the alignment of the element type), but I feel like this is not as expressive as the enum approach, although I am open to suggestions cc `@workingjubilee` `@RalfJung` `@BoxyUwU`
2 parents 88d08b1 + dcf69bc commit 4c0bc8f

9 files changed

+212
-5
lines changed
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#![feature(core_intrinsics, portable_simd)]
2+
3+
use std::intrinsics::simd::*;
4+
use std::simd::*;
5+
6+
fn main() {
7+
unsafe {
8+
let buf = [0u32; 5];
9+
//~v ERROR: accessing memory with alignment
10+
simd_masked_load::<_, _, _, { SimdAlign::Element }>(
11+
i32x4::splat(-1),
12+
// This is not i32-aligned
13+
buf.as_ptr().byte_offset(1),
14+
i32x4::splat(0),
15+
);
16+
}
17+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: Undefined Behavior: accessing memory with alignment ALIGN, but alignment ALIGN is required
2+
--> tests/fail/intrinsics/simd_masked_load_element_misaligned.rs:LL:CC
3+
|
4+
LL | / simd_masked_load::<_, _, _, { SimdAlign::Element }>(
5+
LL | | i32x4::splat(-1),
6+
LL | | // This is not i32-aligned
7+
LL | | buf.as_ptr().byte_offset(1),
8+
LL | | i32x4::splat(0),
9+
LL | | );
10+
| |_________^ Undefined Behavior occurred here
11+
|
12+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
13+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
14+
15+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
16+
17+
error: aborting due to 1 previous error
18+
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#![feature(core_intrinsics, portable_simd)]
2+
3+
use std::intrinsics::simd::*;
4+
use std::simd::*;
5+
6+
fn main() {
7+
unsafe {
8+
let buf = Simd::<i32, 8>::splat(0);
9+
//~v ERROR: accessing memory with alignment
10+
simd_masked_load::<_, _, _, { SimdAlign::Vector }>(
11+
i32x4::splat(-1),
12+
// This is i32-aligned but not i32x4-aligned.
13+
buf.as_array()[1..].as_ptr(),
14+
i32x4::splat(0),
15+
);
16+
}
17+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: Undefined Behavior: accessing memory with alignment ALIGN, but alignment ALIGN is required
2+
--> tests/fail/intrinsics/simd_masked_load_vector_misaligned.rs:LL:CC
3+
|
4+
LL | / simd_masked_load::<_, _, _, { SimdAlign::Vector }>(
5+
LL | | i32x4::splat(-1),
6+
LL | | // This is i32-aligned but not i32x4-aligned.
7+
LL | | buf.as_array()[1..].as_ptr(),
8+
LL | | i32x4::splat(0),
9+
LL | | );
10+
| |_________^ Undefined Behavior occurred here
11+
|
12+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
13+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
14+
15+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
16+
17+
error: aborting due to 1 previous error
18+
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#![feature(core_intrinsics, portable_simd)]
2+
3+
use std::intrinsics::simd::*;
4+
use std::simd::*;
5+
6+
fn main() {
7+
unsafe {
8+
let mut buf = [0u32; 5];
9+
//~v ERROR: accessing memory with alignment
10+
simd_masked_store::<_, _, _, { SimdAlign::Element }>(
11+
i32x4::splat(-1),
12+
// This is not i32-aligned
13+
buf.as_mut_ptr().byte_offset(1),
14+
i32x4::splat(0),
15+
);
16+
}
17+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: Undefined Behavior: accessing memory with alignment ALIGN, but alignment ALIGN is required
2+
--> tests/fail/intrinsics/simd_masked_store_element_misaligned.rs:LL:CC
3+
|
4+
LL | / simd_masked_store::<_, _, _, { SimdAlign::Element }>(
5+
LL | | i32x4::splat(-1),
6+
LL | | // This is not i32-aligned
7+
LL | | buf.as_mut_ptr().byte_offset(1),
8+
LL | | i32x4::splat(0),
9+
LL | | );
10+
| |_________^ Undefined Behavior occurred here
11+
|
12+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
13+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
14+
15+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
16+
17+
error: aborting due to 1 previous error
18+
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#![feature(core_intrinsics, portable_simd)]
2+
3+
use std::intrinsics::simd::*;
4+
use std::simd::*;
5+
6+
fn main() {
7+
unsafe {
8+
let mut buf = Simd::<i32, 8>::splat(0);
9+
//~v ERROR: accessing memory with alignment
10+
simd_masked_store::<_, _, _, { SimdAlign::Vector }>(
11+
i32x4::splat(-1),
12+
// This is i32-aligned but not i32x4-aligned.
13+
buf.as_mut_array()[1..].as_mut_ptr(),
14+
i32x4::splat(0),
15+
);
16+
}
17+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: Undefined Behavior: accessing memory with alignment ALIGN, but alignment ALIGN is required
2+
--> tests/fail/intrinsics/simd_masked_store_vector_misaligned.rs:LL:CC
3+
|
4+
LL | / simd_masked_store::<_, _, _, { SimdAlign::Vector }>(
5+
LL | | i32x4::splat(-1),
6+
LL | | // This is i32-aligned but not i32x4-aligned.
7+
LL | | buf.as_mut_array()[1..].as_mut_ptr(),
8+
LL | | i32x4::splat(0),
9+
LL | | );
10+
| |_________^ Undefined Behavior occurred here
11+
|
12+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
13+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
14+
15+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
16+
17+
error: aborting due to 1 previous error
18+

tests/pass/intrinsics/portable-simd.rs

Lines changed: 72 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -936,26 +936,93 @@ fn simd_float_intrinsics() {
936936
}
937937

938938
fn simd_masked_loadstore() {
939+
use intrinsics::*;
940+
939941
// The buffer is deliberarely too short, so reading the last element would be UB.
940942
let buf = [3i32; 3];
941943
let default = i32x4::splat(0);
942944
let mask = i32x4::from_array([!0, !0, !0, 0]);
943-
let vals = unsafe { intrinsics::simd_masked_load(mask, buf.as_ptr(), default) };
945+
let vals =
946+
unsafe { simd_masked_load::<_, _, _, { SimdAlign::Element }>(mask, buf.as_ptr(), default) };
944947
assert_eq!(vals, i32x4::from_array([3, 3, 3, 0]));
945948
// Also read in a way that the *first* element is OOB.
946949
let mask2 = i32x4::from_array([0, !0, !0, !0]);
947-
let vals =
948-
unsafe { intrinsics::simd_masked_load(mask2, buf.as_ptr().wrapping_sub(1), default) };
950+
let vals = unsafe {
951+
simd_masked_load::<_, _, _, { SimdAlign::Element }>(
952+
mask2,
953+
buf.as_ptr().wrapping_sub(1),
954+
default,
955+
)
956+
};
949957
assert_eq!(vals, i32x4::from_array([0, 3, 3, 3]));
950958

951959
// The buffer is deliberarely too short, so writing the last element would be UB.
952960
let mut buf = [42i32; 3];
953961
let vals = i32x4::from_array([1, 2, 3, 4]);
954-
unsafe { intrinsics::simd_masked_store(mask, buf.as_mut_ptr(), vals) };
962+
unsafe { simd_masked_store::<_, _, _, { SimdAlign::Element }>(mask, buf.as_mut_ptr(), vals) };
955963
assert_eq!(buf, [1, 2, 3]);
956964
// Also write in a way that the *first* element is OOB.
957-
unsafe { intrinsics::simd_masked_store(mask2, buf.as_mut_ptr().wrapping_sub(1), vals) };
965+
unsafe {
966+
simd_masked_store::<_, _, _, { SimdAlign::Element }>(
967+
mask2,
968+
buf.as_mut_ptr().wrapping_sub(1),
969+
vals,
970+
)
971+
};
958972
assert_eq!(buf, [2, 3, 4]);
973+
974+
// we use a purposely misaliged buffer to make sure Miri doesn't error in this case
975+
let buf = [0x03030303_i32; 5];
976+
let default = i32x4::splat(0);
977+
let mask = i32x4::splat(!0);
978+
let vals = unsafe {
979+
simd_masked_load::<_, _, _, { SimdAlign::Unaligned }>(
980+
mask,
981+
buf.as_ptr().byte_offset(1), // this is guaranteed to be unaligned
982+
default,
983+
)
984+
};
985+
assert_eq!(vals, i32x4::splat(0x03030303));
986+
987+
let mut buf = [0i32; 5];
988+
let mask = i32x4::splat(!0);
989+
unsafe {
990+
simd_masked_store::<_, _, _, { SimdAlign::Unaligned }>(
991+
mask,
992+
buf.as_mut_ptr().byte_offset(1), // this is guaranteed to be unaligned
993+
vals,
994+
)
995+
};
996+
assert_eq!(
997+
buf,
998+
[
999+
i32::from_ne_bytes([0, 3, 3, 3]),
1000+
0x03030303,
1001+
0x03030303,
1002+
0x03030303,
1003+
i32::from_ne_bytes([3, 0, 0, 0])
1004+
]
1005+
);
1006+
1007+
// `repr(simd)` types like `Simd<T, N>` have the correct alignment for vectors
1008+
let buf = i32x4::splat(3);
1009+
let default = i32x4::splat(0);
1010+
let mask = i32x4::splat(!0);
1011+
let vals = unsafe {
1012+
simd_masked_load::<_, _, _, { SimdAlign::Vector }>(
1013+
mask,
1014+
&raw const buf as *const i32,
1015+
default,
1016+
)
1017+
};
1018+
assert_eq!(vals, buf);
1019+
1020+
let mut buf = i32x4::splat(0);
1021+
let mask = i32x4::splat(!0);
1022+
unsafe {
1023+
simd_masked_store::<_, _, _, { SimdAlign::Vector }>(mask, &raw mut buf as *mut i32, vals)
1024+
};
1025+
assert_eq!(buf, vals);
9591026
}
9601027

9611028
fn simd_ops_non_pow2() {

0 commit comments

Comments
 (0)