Skip to content

Commit 496728e

Browse files
Rollup merge of #154043 - RalfJung:simd-min-max, r=Amanieu,calebzulawski,antoyo
simd_fmin/fmax: make semantics and name consistent with scalar intrinsics This is the SIMD version of rust-lang/rust#153343: change the documented semantics of the SIMD float min/max intrinsics to that of the scalar intrinsics, and also make the name consistent. The overall semantic change this amounts to is that we restrict the non-determinism: the old semantics effectively mean "when one input is an SNaN, the result non-deterministically is a NaN or the other input"; the new semantics say that in this case the other input must be returned. For all other cases, old and new semantics are equivalent. This means all users of these intrinsics that were correct with the old semantics are still correct: the overall set of possible behaviors has become smaller, no new possible behaviors are being added. In terms of providers of this API: - Miri, GCC, and cranelift already implement the new semantics, so no changes are needed. - LLVM is adjusted to use `minimumnum nsz` instead of `minnum`, thus giving us the new semantics. In terms of consumers of this API: - Portable SIMD almost certainly wants to match the scalar behavior, so this is strictly a bugfix here. - Stdarch mostly stopped using the intrinsic, except on nvptx, where arguably the new semantics are closer to what we actually want than the old semantics (rust-lang/stdarch#2056). Q: Should there be an `f` in the intrinsic name to indicate that it is for floats? E.g., `simd_fminimum_number_nsz`? Also see rust-lang/rust#153395.
2 parents c8f40f8 + a3741b2 commit 496728e

3 files changed

Lines changed: 17 additions & 13 deletions

File tree

example/float-minmax-pass.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,24 +33,24 @@ fn main() {
3333
let n = f32x4([nan, nan, nan, nan]);
3434

3535
unsafe {
36-
let min0 = simd_fmin(x, y);
37-
let min1 = simd_fmin(y, x);
36+
let min0 = simd_minimum_number_nsz(x, y);
37+
let min1 = simd_minimum_number_nsz(y, x);
3838
assert_eq!(min0.into_array(), min1.into_array());
3939
let e = f32x4([1.0, 1.0, 3.0, 3.0]);
4040
assert_eq!(min0.into_array(), e.into_array());
41-
let minn = simd_fmin(x, n);
41+
let minn = simd_minimum_number_nsz(x, n);
4242
assert_eq!(minn.into_array(), x.into_array());
43-
let minn = simd_fmin(y, n);
43+
let minn = simd_minimum_number_nsz(y, n);
4444
assert_eq!(minn.into_array(), y.into_array());
4545

46-
let max0 = simd_fmax(x, y);
47-
let max1 = simd_fmax(y, x);
46+
let max0 = simd_maximum_number_nsz(x, y);
47+
let max1 = simd_maximum_number_nsz(y, x);
4848
assert_eq!(max0.into_array(), max1.into_array());
4949
let e = f32x4([2.0, 2.0, 4.0, 4.0]);
5050
assert_eq!(max0.into_array(), e.into_array());
51-
let maxn = simd_fmax(x, n);
51+
let maxn = simd_maximum_number_nsz(x, n);
5252
assert_eq!(maxn.into_array(), x.into_array());
53-
let maxn = simd_fmax(y, n);
53+
let maxn = simd_maximum_number_nsz(y, n);
5454
assert_eq!(maxn.into_array(), y.into_array());
5555
}
5656
}

src/intrinsics/simd.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,7 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
493493
}
494494
}
495495

496-
sym::simd_fmin | sym::simd_fmax => {
496+
sym::simd_minimum_number_nsz | sym::simd_maximum_number_nsz => {
497497
intrinsic_args!(fx, args => (x, y); intrinsic);
498498

499499
if !x.layout().ty.is_simd() {
@@ -508,8 +508,12 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
508508
_ => unreachable!("{:?}", lane_ty),
509509
}
510510
match intrinsic {
511-
sym::simd_fmin => crate::num::codegen_float_min(fx, x_lane, y_lane),
512-
sym::simd_fmax => crate::num::codegen_float_max(fx, x_lane, y_lane),
511+
sym::simd_minimum_number_nsz => {
512+
crate::num::codegen_float_min(fx, x_lane, y_lane)
513+
}
514+
sym::simd_maximum_number_nsz => {
515+
crate::num::codegen_float_max(fx, x_lane, y_lane)
516+
}
513517
_ => unreachable!(),
514518
}
515519
});

src/num.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -500,8 +500,8 @@ fn codegen_ptr_binop<'tcx>(
500500

501501
// In Rust floating point min and max don't propagate NaN (not even SNaN). In Cranelift they do
502502
// however. For this reason it is necessary to use `a.is_nan() ? b : (a >= b ? b : a)` for
503-
// `minnumf*` and `a.is_nan() ? b : (a <= b ? b : a)` for `maxnumf*`. NaN checks are done by
504-
// comparing a float against itself. Only in case of NaN is it not equal to itself.
503+
// `minimum_number_nsz` and `a.is_nan() ? b : (a <= b ? b : a)` for `maximum_number_nsz`. NaN checks
504+
// are done by comparing a float against itself. Only in case of NaN is it not equal to itself.
505505
pub(crate) fn codegen_float_min(fx: &mut FunctionCx<'_, '_, '_>, a: Value, b: Value) -> Value {
506506
// FIXME(bytecodealliance/wasmtime#8312): Replace with Cranelift `fcmp` once
507507
// `f16`/`f128` backend lowerings have been added to Cranelift.

0 commit comments

Comments
 (0)