Skip to content

Commit 67ab3ac

Browse files
GuillaumeGomezGuillaumeGomez
authored
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 #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 #153395.
2 parents e6f17e4 + 986a280 commit 67ab3ac

18 files changed

Lines changed: 95 additions & 79 deletions

File tree

‎compiler/rustc_codegen_cranelift/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
}

‎compiler/rustc_codegen_cranelift/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
});

‎compiler/rustc_codegen_cranelift/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.

‎compiler/rustc_codegen_gcc/src/builder.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2278,6 +2278,8 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> {
22782278
})
22792279
}
22802280

2281+
/// Emits a SIMD min/max operation for floats. The semantics for each lane are: if one
2282+
/// side is NaN (QNaN or SNaN), the other side is returned.
22812283
fn vector_extremum(
22822284
&mut self,
22832285
a: RValue<'gcc>,
@@ -2286,8 +2288,9 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> {
22862288
) -> RValue<'gcc> {
22872289
let vector_type = a.get_type();
22882290

2289-
// mask out the NaNs in b and replace them with the corresponding lane in a, so when a and
2290-
// b get compared & spliced together, we get the numeric values instead of NaNs.
2291+
// Mask out the NaNs (both QNaN and SNaN) in b and replace them with the corresponding lane
2292+
// in a, so when a and b get compared & spliced together, we get the numeric values instead
2293+
// of NaNs.
22912294
let b_nan_mask = self.context.new_comparison(self.location, ComparisonOp::NotEquals, b, b);
22922295
let mask_type = b_nan_mask.get_type();
22932296
let b_nan_mask_inverted =
@@ -2309,7 +2312,7 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> {
23092312
self.context.new_bitcast(self.location, res, vector_type)
23102313
}
23112314

2312-
pub fn vector_fmin(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
2315+
pub fn vector_minimum_number_nsz(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
23132316
self.vector_extremum(a, b, ExtremumOperation::Min)
23142317
}
23152318

@@ -2341,7 +2344,7 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> {
23412344
unimplemented!();
23422345
}
23432346

2344-
pub fn vector_fmax(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
2347+
pub fn vector_maximum_number_nsz(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
23452348
self.vector_extremum(a, b, ExtremumOperation::Max)
23462349
}
23472350

‎compiler/rustc_codegen_gcc/src/intrinsic/simd.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,8 +1222,8 @@ pub fn generic_simd_intrinsic<'a, 'gcc, 'tcx>(
12221222
simd_and: Uint, Int => and;
12231223
simd_or: Uint, Int => or; // FIXME(antoyo): calling `or` might not work on vectors.
12241224
simd_xor: Uint, Int => xor;
1225-
simd_fmin: Float => vector_fmin;
1226-
simd_fmax: Float => vector_fmax;
1225+
simd_minimum_number_nsz: Float => vector_minimum_number_nsz;
1226+
simd_maximum_number_nsz: Float => vector_maximum_number_nsz;
12271227
}
12281228

12291229
macro_rules! arith_unary {

‎compiler/rustc_codegen_llvm/src/builder.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,12 +1605,16 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
16051605
*self = Self::build(self.cx, next_bb);
16061606
}
16071607

1608-
pub(crate) fn minnum(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
1609-
self.call_intrinsic("llvm.minnum", &[self.val_ty(lhs)], &[lhs, rhs])
1608+
pub(crate) fn minimum_number_nsz(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
1609+
let call = self.call_intrinsic("llvm.minimumnum", &[self.val_ty(lhs)], &[lhs, rhs]);
1610+
unsafe { llvm::LLVMRustSetNoSignedZeros(call) };
1611+
call
16101612
}
16111613

1612-
pub(crate) fn maxnum(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
1613-
self.call_intrinsic("llvm.maxnum", &[self.val_ty(lhs)], &[lhs, rhs])
1614+
pub(crate) fn maximum_number_nsz(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
1615+
let call = self.call_intrinsic("llvm.maximumnum", &[self.val_ty(lhs)], &[lhs, rhs]);
1616+
unsafe { llvm::LLVMRustSetNoSignedZeros(call) };
1617+
call
16141618
}
16151619

16161620
pub(crate) fn insert_element(

‎compiler/rustc_codegen_llvm/src/intrinsic.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2779,8 +2779,8 @@ fn generic_simd_intrinsic<'ll, 'tcx>(
27792779
simd_and: Uint, Int => and;
27802780
simd_or: Uint, Int => or;
27812781
simd_xor: Uint, Int => xor;
2782-
simd_fmax: Float => maxnum;
2783-
simd_fmin: Float => minnum;
2782+
simd_maximum_number_nsz: Float => maximum_number_nsz;
2783+
simd_minimum_number_nsz: Float => minimum_number_nsz;
27842784

27852785
}
27862786
macro_rules! arith_unary {

‎compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
174174
| sym::simd_le
175175
| sym::simd_gt
176176
| sym::simd_ge
177-
| sym::simd_fmax
178-
| sym::simd_fmin
177+
| sym::simd_maximum_number_nsz
178+
| sym::simd_minimum_number_nsz
179179
| sym::simd_saturating_add
180180
| sym::simd_saturating_sub
181181
| sym::simd_arith_offset => {
@@ -211,8 +211,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
211211
sym::simd_le => Op::MirOp(BinOp::Le),
212212
sym::simd_gt => Op::MirOp(BinOp::Gt),
213213
sym::simd_ge => Op::MirOp(BinOp::Ge),
214-
sym::simd_fmax => Op::FMinMax(MinMax::MaximumNumberNsz),
215-
sym::simd_fmin => Op::FMinMax(MinMax::MinimumNumberNsz),
214+
sym::simd_maximum_number_nsz => Op::FMinMax(MinMax::MaximumNumberNsz),
215+
sym::simd_minimum_number_nsz => Op::FMinMax(MinMax::MinimumNumberNsz),
216216
sym::simd_saturating_add => Op::SaturatingOp(BinOp::Add),
217217
sym::simd_saturating_sub => Op::SaturatingOp(BinOp::Sub),
218218
sym::simd_arith_offset => Op::WrappingOffset,

‎compiler/rustc_hir_analysis/src/check/intrinsic.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -720,8 +720,8 @@ pub(crate) fn check_intrinsic_type(
720720
| sym::simd_and
721721
| sym::simd_or
722722
| sym::simd_xor
723-
| sym::simd_fmin
724-
| sym::simd_fmax
723+
| sym::simd_minimum_number_nsz
724+
| sym::simd_maximum_number_nsz
725725
| sym::simd_saturating_add
726726
| sym::simd_saturating_sub
727727
| sym::simd_carryless_mul => (1, 0, vec![param(0), param(0)], param(0)),

‎compiler/rustc_span/src/symbol.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1853,8 +1853,6 @@ symbols! {
18531853
simd_flog10,
18541854
simd_floor,
18551855
simd_fma,
1856-
simd_fmax,
1857-
simd_fmin,
18581856
simd_fsin,
18591857
simd_fsqrt,
18601858
simd_funnel_shl,
@@ -1868,6 +1866,8 @@ symbols! {
18681866
simd_lt,
18691867
simd_masked_load,
18701868
simd_masked_store,
1869+
simd_maximum_number_nsz,
1870+
simd_minimum_number_nsz,
18711871
simd_mul,
18721872
simd_ne,
18731873
simd_neg,

0 commit comments

Comments
 (0)