Skip to content

use correct LLVM intrinsic for min/max on floats #153343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RalfJung wants to merge 1 commit into
base: main
Choose a base branch
from
+296 −141
Open

use correct LLVM intrinsic for min/max on floats #153343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1267,7 +1267,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
ret.write_cvalue(fx, val);
}

sym::minnumf16 => {
sym::minimum_number_nsz_f16 => {
intrinsic_args!(fx, args => (a, b); intrinsic);
let a = a.load_scalar(fx);
let b = b.load_scalar(fx);
Expand All @@ -1276,7 +1276,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f16));
ret.write_cvalue(fx, val);
}
sym::minnumf32 => {
sym::minimum_number_nsz_f32 => {
intrinsic_args!(fx, args => (a, b); intrinsic);
let a = a.load_scalar(fx);
let b = b.load_scalar(fx);
Expand All @@ -1285,7 +1285,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f32));
ret.write_cvalue(fx, val);
}
sym::minnumf64 => {
sym::minimum_number_nsz_f64 => {
intrinsic_args!(fx, args => (a, b); intrinsic);
let a = a.load_scalar(fx);
let b = b.load_scalar(fx);
Expand All @@ -1294,7 +1294,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f64));
ret.write_cvalue(fx, val);
}
sym::minnumf128 => {
sym::minimum_number_nsz_f128 => {
intrinsic_args!(fx, args => (a, b); intrinsic);
let a = a.load_scalar(fx);
let b = b.load_scalar(fx);
Expand All @@ -1303,7 +1303,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f128));
ret.write_cvalue(fx, val);
}
sym::maxnumf16 => {
sym::maximum_number_nsz_f16 => {
intrinsic_args!(fx, args => (a, b); intrinsic);
let a = a.load_scalar(fx);
let b = b.load_scalar(fx);
Expand All @@ -1312,7 +1312,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f16));
ret.write_cvalue(fx, val);
}
sym::maxnumf32 => {
sym::maximum_number_nsz_f32 => {
intrinsic_args!(fx, args => (a, b); intrinsic);
let a = a.load_scalar(fx);
let b = b.load_scalar(fx);
Expand All @@ -1321,7 +1321,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f32));
ret.write_cvalue(fx, val);
}
sym::maxnumf64 => {
sym::maximum_number_nsz_f64 => {
intrinsic_args!(fx, args => (a, b); intrinsic);
let a = a.load_scalar(fx);
let b = b.load_scalar(fx);
Expand All @@ -1330,7 +1330,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f64));
ret.write_cvalue(fx, val);
}
sym::maxnumf128 => {
sym::maximum_number_nsz_f128 => {
intrinsic_args!(fx, args => (a, b); intrinsic);
let a = a.load_scalar(fx);
let b = b.load_scalar(fx);
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_cranelift/src/num.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,10 @@ fn codegen_ptr_binop<'tcx>(
}
}

// In Rust floating point min and max don't propagate NaN. In Cranelift they do however.
// For this reason it is necessary to use `a.is_nan() ? b : (a >= b ? b : a)` for `minnumf*`
// and `a.is_nan() ? b : (a <= b ? b : a)` for `maxnumf*`. NaN checks are done by comparing
// a float against itself. Only in case of NaN is it not equal to itself.
// In Rust floating point min and max don't propagate NaN (not even SNaN). In Cranelift they do
// however. For this reason it is necessary to use `a.is_nan() ? b : (a >= b ? b : a)` for
// `minnumf*` and `a.is_nan() ? b : (a <= b ? b : a)` for `maxnumf*`. NaN checks are done by
// comparing a float against itself. Only in case of NaN is it not equal to itself.
pub(crate) fn codegen_float_min(fx: &mut FunctionCx<'_, '_, '_>, a: Value, b: Value) -> Value {
// FIXME(bytecodealliance/wasmtime#8312): Replace with Cranelift `fcmp` once
// `f16`/`f128` backend lowerings have been added to Cranelift.
Expand Down
10 changes: 0 additions & 10 deletions compiler/rustc_codegen_gcc/src/intrinsic/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ fn get_simple_intrinsic<'gcc, 'tcx>(
sym::fmuladdf64 => "fma", // TODO: use gcc intrinsic analogous to llvm.fmuladd.f64
sym::fabsf32 => "fabsf",
sym::fabsf64 => "fabs",
sym::minnumf32 => "fminf",
sym::minnumf64 => "fmin",
sym::minimumf32 => "fminimumf",
sym::minimumf64 => "fminimum",
sym::minimumf128 => {
Expand All @@ -92,8 +90,6 @@ fn get_simple_intrinsic<'gcc, 'tcx>(
false,
));
}
sym::maxnumf32 => "fmaxf",
sym::maxnumf64 => "fmax",
sym::maximumf32 => "fmaximumf",
sym::maximumf64 => "fmaximum",
sym::maximumf128 => {
Expand Down Expand Up @@ -236,8 +232,6 @@ fn get_simple_function_f128_2args<'gcc, 'tcx>(

let f128_type = cx.type_f128();
let func_name = match name {
sym::maxnumf128 => "fmaxf128",
sym::minnumf128 => "fminf128",
sym::copysignf128 => "copysignf128",
_ => return None,
};
Expand Down Expand Up @@ -266,8 +260,6 @@ fn f16_builtin<'gcc, 'tcx>(
sym::fabsf16 => "fabsf",
sym::floorf16 => "__builtin_floorf",
sym::fmaf16 => "fmaf",
sym::maxnumf16 => "__builtin_fmaxf",
sym::minnumf16 => "__builtin_fminf",
sym::powf16 => "__builtin_powf",
sym::powif16 => {
let func = cx.context.get_builtin_function("__builtin_powif");
Expand Down Expand Up @@ -333,8 +325,6 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tc
| sym::fabsf16
| sym::floorf16
| sym::fmaf16
| sym::maxnumf16
| sym::minnumf16
| sym::powf16
| sym::powif16
| sym::roundf16
Expand Down
36 changes: 26 additions & 10 deletions compiler/rustc_codegen_llvm/src/intrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,23 +113,13 @@ fn call_simple_intrinsic<'ll, 'tcx>(
sym::fabsf64 => ("llvm.fabs", &[bx.type_f64()]),
sym::fabsf128 => ("llvm.fabs", &[bx.type_f128()]),

sym::minnumf16 => ("llvm.minnum", &[bx.type_f16()]),
sym::minnumf32 => ("llvm.minnum", &[bx.type_f32()]),
sym::minnumf64 => ("llvm.minnum", &[bx.type_f64()]),
sym::minnumf128 => ("llvm.minnum", &[bx.type_f128()]),

// FIXME: LLVM currently mis-compile those intrinsics, re-enable them
// when llvm/llvm-project#{139380,139381,140445} are fixed.
//sym::minimumf16 => ("llvm.minimum", &[bx.type_f16()]),
//sym::minimumf32 => ("llvm.minimum", &[bx.type_f32()]),
//sym::minimumf64 => ("llvm.minimum", &[bx.type_f64()]),
//sym::minimumf128 => ("llvm.minimum", &[cx.type_f128()]),
//
sym::maxnumf16 => ("llvm.maxnum", &[bx.type_f16()]),
sym::maxnumf32 => ("llvm.maxnum", &[bx.type_f32()]),
sym::maxnumf64 => ("llvm.maxnum", &[bx.type_f64()]),
sym::maxnumf128 => ("llvm.maxnum", &[bx.type_f128()]),

// FIXME: LLVM currently mis-compile those intrinsics, re-enable them
// when llvm/llvm-project#{139380,139381,140445} are fixed.
//sym::maximumf16 => ("llvm.maximum", &[bx.type_f16()]),
Expand Down Expand Up @@ -196,6 +186,32 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
let simple = call_simple_intrinsic(self, name, args);
let llval = match name {
_ if simple.is_some() => simple.unwrap(),
sym::minimum_number_nsz_f16
| sym::minimum_number_nsz_f32
| sym::minimum_number_nsz_f64
| sym::minimum_number_nsz_f128
| sym::maximum_number_nsz_f16
| sym::maximum_number_nsz_f32
| sym::maximum_number_nsz_f64
| sym::maximum_number_nsz_f128
// Need at least LLVM 22 for `min/maximumnum` to not crash LLVM.
if crate::llvm_util::get_version() >= (22, 0, 0) =>
{
let intrinsic_name = if name.as_str().starts_with("min") {
"llvm.minimumnum"
} else {
"llvm.maximumnum"
};
let call = self.call_intrinsic(
intrinsic_name,
&[args[0].layout.immediate_llvm_type(self.cx)],
&[args[0].immediate(), args[1].immediate()],
);
// `nsz` on minimumnum/maximumnum is special: its only effect is to make
// signed-zero ordering non-deterministic.
unsafe { llvm::LLVMRustSetNoSignedZeros(call) };
call
}
sym::ptr_mask => {
let ptr = args[0].immediate();
self.call_intrinsic(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2045,6 +2045,7 @@ unsafe extern "C" {
pub(crate) fn LLVMRustSetFastMath(Instr: &Value);
pub(crate) fn LLVMRustSetAlgebraicMath(Instr: &Value);
pub(crate) fn LLVMRustSetAllowReassoc(Instr: &Value);
pub(crate) fn LLVMRustSetNoSignedZeros(Instr: &Value);

// Miscellaneous instructions
pub(crate) fn LLVMRustBuildMemCpy<'a>(
Expand Down
50 changes: 25 additions & 25 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,20 @@ pub(crate) enum MinMax {
/// In particular, `-0.0` is considered smaller than `+0.0` and
/// if either input is NaN, the result is NaN.
Minimum,
/// The IEEE-2008 `minNum` operation with the SNaN handling of the
/// IEEE-2019 `minimumNumber` operation - see `f32::min` etc.
/// The IEEE-2019 `minimumNumber` operation but with non-deterministic signed zero handling
/// (like in IEEE-2008 `minNum`) - see `f32::min` etc.
/// In particular, if the inputs are `-0.0` and `+0.0`, the result is non-deterministic,
/// and if one argument is NaN (quiet or signaling), the other one is returned.
MinimumNumber,
MinimumNumberNsz,
/// The IEEE-2019 `maximum` operation - see `f32::maximum` etc.
/// In particular, `-0.0` is considered smaller than `+0.0` and
/// if either input is NaN, the result is NaN.
Maximum,
/// The IEEE-2008 `maxNum` operation with the SNaN handling of the
/// IEEE-2019 `maximumNumber` operation - see `f32::max` etc.
/// The IEEE-2019 `maximumNumber` operation but with non-deterministic signed zero handling
/// (like in IEEE-2008 `maxNum`) - see `f32::max` etc.
/// In particular, if the inputs are `-0.0` and `+0.0`, the result is non-deterministic,
/// and if one argument is NaN (quiet or signaling), the other one is returned.
MaximumNumber,
MaximumNumberNsz,
}

/// Directly returns an `Allocation` containing an absolute path representation of the given type.
Expand Down Expand Up @@ -542,17 +542,17 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
self.write_scalar(Scalar::from_target_usize(align.bytes(), self), dest)?;
}

sym::minnumf16 => {
self.float_minmax_intrinsic::<Half>(args, MinMax::MinimumNumber, dest)?
sym::minimum_number_nsz_f16 => {
self.float_minmax_intrinsic::<Half>(args, MinMax::MinimumNumberNsz, dest)?
}
sym::minnumf32 => {
self.float_minmax_intrinsic::<Single>(args, MinMax::MinimumNumber, dest)?
sym::minimum_number_nsz_f32 => {
self.float_minmax_intrinsic::<Single>(args, MinMax::MinimumNumberNsz, dest)?
}
sym::minnumf64 => {
self.float_minmax_intrinsic::<Double>(args, MinMax::MinimumNumber, dest)?
sym::minimum_number_nsz_f64 => {
self.float_minmax_intrinsic::<Double>(args, MinMax::MinimumNumberNsz, dest)?
}
sym::minnumf128 => {
self.float_minmax_intrinsic::<Quad>(args, MinMax::MinimumNumber, dest)?
sym::minimum_number_nsz_f128 => {
self.float_minmax_intrinsic::<Quad>(args, MinMax::MinimumNumberNsz, dest)?
}

sym::minimumf16 => self.float_minmax_intrinsic::<Half>(args, MinMax::Minimum, dest)?,
Expand All @@ -564,17 +564,17 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
}
sym::minimumf128 => self.float_minmax_intrinsic::<Quad>(args, MinMax::Minimum, dest)?,

sym::maxnumf16 => {
self.float_minmax_intrinsic::<Half>(args, MinMax::MaximumNumber, dest)?
sym::maximum_number_nsz_f16 => {
self.float_minmax_intrinsic::<Half>(args, MinMax::MaximumNumberNsz, dest)?
}
sym::maxnumf32 => {
self.float_minmax_intrinsic::<Single>(args, MinMax::MaximumNumber, dest)?
sym::maximum_number_nsz_f32 => {
self.float_minmax_intrinsic::<Single>(args, MinMax::MaximumNumberNsz, dest)?
}
sym::maxnumf64 => {
self.float_minmax_intrinsic::<Double>(args, MinMax::MaximumNumber, dest)?
sym::maximum_number_nsz_f64 => {
self.float_minmax_intrinsic::<Double>(args, MinMax::MaximumNumberNsz, dest)?
}
sym::maxnumf128 => {
self.float_minmax_intrinsic::<Quad>(args, MinMax::MaximumNumber, dest)?
sym::maximum_number_nsz_f128 => {
self.float_minmax_intrinsic::<Quad>(args, MinMax::MaximumNumberNsz, dest)?
}

sym::maximumf16 => self.float_minmax_intrinsic::<Half>(args, MinMax::Maximum, dest)?,
Expand Down Expand Up @@ -1051,16 +1051,16 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
{
let a: F = a.to_float()?;
let b: F = b.to_float()?;
let res = if matches!(op, MinMax::MinimumNumber | MinMax::MaximumNumber) && a == b {
let res = if matches!(op, MinMax::MinimumNumberNsz | MinMax::MaximumNumberNsz) && a == b {
// They are definitely not NaN (those are never equal), but they could be `+0` and `-0`.
// Let the machine decide which one to return.
M::equal_float_min_max(self, a, b)
} else {
let result = match op {
MinMax::Minimum => a.minimum(b),
MinMax::MinimumNumber => a.min(b),
MinMax::MinimumNumberNsz => a.min(b),
MinMax::Maximum => a.maximum(b),
MinMax::MaximumNumber => a.max(b),
MinMax::MaximumNumberNsz => a.max(b),
};
self.adjust_nan(result, &[a, b])
};
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
sym::simd_le => Op::MirOp(BinOp::Le),
sym::simd_gt => Op::MirOp(BinOp::Gt),
sym::simd_ge => Op::MirOp(BinOp::Ge),
sym::simd_fmax => Op::FMinMax(MinMax::MaximumNumber),
sym::simd_fmin => Op::FMinMax(MinMax::MinimumNumber),
sym::simd_fmax => Op::FMinMax(MinMax::MaximumNumberNsz),
sym::simd_fmin => Op::FMinMax(MinMax::MinimumNumberNsz),
sym::simd_saturating_add => Op::SaturatingOp(BinOp::Add),
sym::simd_saturating_sub => Op::SaturatingOp(BinOp::Sub),
sym::simd_arith_offset => Op::WrappingOffset,
Expand Down Expand Up @@ -304,8 +304,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
sym::simd_reduce_xor => Op::MirOp(BinOp::BitXor),
sym::simd_reduce_any => Op::MirOpBool(BinOp::BitOr),
sym::simd_reduce_all => Op::MirOpBool(BinOp::BitAnd),
sym::simd_reduce_max => Op::MinMax(MinMax::MaximumNumber),
sym::simd_reduce_min => Op::MinMax(MinMax::MinimumNumber),
sym::simd_reduce_max => Op::MinMax(MinMax::MaximumNumberNsz),
sym::simd_reduce_min => Op::MinMax(MinMax::MinimumNumberNsz),
_ => unreachable!(),
};

Expand All @@ -329,8 +329,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
} else {
// Just boring integers, no NaNs to worry about.
let mirop = match mmop {
MinMax::MinimumNumber | MinMax::Minimum => BinOp::Le,
MinMax::MaximumNumber | MinMax::Maximum => BinOp::Ge,
MinMax::MinimumNumberNsz | MinMax::Minimum => BinOp::Le,
MinMax::MaximumNumberNsz | MinMax::Maximum => BinOp::Ge,
};
if self.binary_op(mirop, &res, &op)?.to_scalar().to_bool()? {
res
Expand Down
36 changes: 20 additions & 16 deletions compiler/rustc_hir_analysis/src/check/intrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,22 +147,22 @@ fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) -> hi
| sym::logf32
| sym::logf64
| sym::logf128
| sym::maximum_number_nsz_f16
| sym::maximum_number_nsz_f32
| sym::maximum_number_nsz_f64
| sym::maximum_number_nsz_f128
| sym::maximumf16
| sym::maximumf32
| sym::maximumf64
| sym::maximumf128
| sym::maxnumf16
| sym::maxnumf32
| sym::maxnumf64
| sym::maxnumf128
| sym::minimum_number_nsz_f16
| sym::minimum_number_nsz_f32
| sym::minimum_number_nsz_f64
| sym::minimum_number_nsz_f128
| sym::minimumf16
| sym::minimumf32
| sym::minimumf64
| sym::minimumf128
| sym::minnumf16
| sym::minnumf32
| sym::minnumf64
| sym::minnumf128
| sym::mul_with_overflow
| sym::needs_drop
| sym::offload
Expand Down Expand Up @@ -468,20 +468,24 @@ pub(crate) fn check_intrinsic_type(
sym::fabsf64 => (0, 0, vec![tcx.types.f64], tcx.types.f64),
sym::fabsf128 => (0, 0, vec![tcx.types.f128], tcx.types.f128),

sym::minnumf16 => (0, 0, vec![tcx.types.f16, tcx.types.f16], tcx.types.f16),
sym::minnumf32 => (0, 0, vec![tcx.types.f32, tcx.types.f32], tcx.types.f32),
sym::minnumf64 => (0, 0, vec![tcx.types.f64, tcx.types.f64], tcx.types.f64),
sym::minnumf128 => (0, 0, vec![tcx.types.f128, tcx.types.f128], tcx.types.f128),
sym::minimum_number_nsz_f16 => (0, 0, vec![tcx.types.f16, tcx.types.f16], tcx.types.f16),
sym::minimum_number_nsz_f32 => (0, 0, vec![tcx.types.f32, tcx.types.f32], tcx.types.f32),
sym::minimum_number_nsz_f64 => (0, 0, vec![tcx.types.f64, tcx.types.f64], tcx.types.f64),
sym::minimum_number_nsz_f128 => {
(0, 0, vec![tcx.types.f128, tcx.types.f128], tcx.types.f128)
}

sym::minimumf16 => (0, 0, vec![tcx.types.f16, tcx.types.f16], tcx.types.f16),
sym::minimumf32 => (0, 0, vec![tcx.types.f32, tcx.types.f32], tcx.types.f32),
sym::minimumf64 => (0, 0, vec![tcx.types.f64, tcx.types.f64], tcx.types.f64),
sym::minimumf128 => (0, 0, vec![tcx.types.f128, tcx.types.f128], tcx.types.f128),

sym::maxnumf16 => (0, 0, vec![tcx.types.f16, tcx.types.f16], tcx.types.f16),
sym::maxnumf32 => (0, 0, vec![tcx.types.f32, tcx.types.f32], tcx.types.f32),
sym::maxnumf64 => (0, 0, vec![tcx.types.f64, tcx.types.f64], tcx.types.f64),
sym::maxnumf128 => (0, 0, vec![tcx.types.f128, tcx.types.f128], tcx.types.f128),
sym::maximum_number_nsz_f16 => (0, 0, vec![tcx.types.f16, tcx.types.f16], tcx.types.f16),
sym::maximum_number_nsz_f32 => (0, 0, vec![tcx.types.f32, tcx.types.f32], tcx.types.f32),
sym::maximum_number_nsz_f64 => (0, 0, vec![tcx.types.f64, tcx.types.f64], tcx.types.f64),
sym::maximum_number_nsz_f128 => {
(0, 0, vec![tcx.types.f128, tcx.types.f128], tcx.types.f128)
}

sym::maximumf16 => (0, 0, vec![tcx.types.f16, tcx.types.f16], tcx.types.f16),
sym::maximumf32 => (0, 0, vec![tcx.types.f32, tcx.types.f32], tcx.types.f32),
Expand Down
Loading
Loading