Skip to content

Commit 5f47048

Browse files
stephentyronestephentyrone
authored
Fix sNaN behavior of .minimum, .maximum, .minimumMagnitude and .maximumMagnitude. (#3361)
* Bring the behavior of Self.minimum, .maximum, .minimumMagnitude and .maximumMagnitude in line with the IEEE 754 rules. Specifically, if either argument is a signaling NaN, the result should be a quiet NaN. Previously, signaling NaN inputs were treated identically to quiet NaN inputs. Some other changes folded in with this: - Formally define which argument is returned if lhs == rhs or abs(lhs) == abs(rhs) in the case of the Magnitude versions. This guarantees that the set {min,max} is identitical to the set {lhs,rhs} so long as neither argument is NaN. This is not only a nod to formalism; it's also a critical property for generic head-tail arithmetic to simulate wider types. - Added test coverage for these four operations.
1 parent 09a19bb commit 5f47048

File tree

2 files changed

+95
-23
lines changed

2 files changed

+95
-23
lines changed

‎stdlib/public/core/FloatingPoint.swift.gyb

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -332,40 +332,36 @@ public protocol FloatingPoint: Comparable, IntegerLiteralConvertible,
332332

333333
/// The minimum of `x` and `y`. Implements the IEEE 754 `minNum` operation.
334334
///
335-
/// Returns `x` if `x <= y`, `y` if `y < x`, and whichever of `x` or `y`
336-
/// is a number if the other is NaN. The result is NaN only if both
337-
/// arguments are NaN.
338-
///
339-
/// This function is an implementation hook to be used by the free function
340-
/// min(Self, Self) -> Self so that we get the IEEE 754 behavior with regard
341-
/// to NaNs.
335+
/// If either of `x` or `y` is a signaling NaN, the result is a quiet NaN.
336+
/// Otherwise, the result is `x` if x <= y, `y` if x > y, and whichever
337+
/// is a number if the other is a quiet NaN. If both `x` and `y` are quiet
338+
/// NaNs, a quiet NaN is returned.
342339
static func minimum(_ x: Self, _ y: Self) -> Self
343340

344341
/// The maximum of `x` and `y`. Implements the IEEE 754 `maxNum` operation.
345342
///
346-
/// Returns `x` if `x >= y`, `y` if `y > x`, and whichever of `x` or `y`
347-
/// is a number if the other is NaN. The result is NaN only if both
348-
/// arguments are NaN.
349-
///
350-
/// This function is an implementation hook to be used by the free function
351-
/// max(Self, Self) -> Self so that we get the IEEE 754 behavior with regard
352-
/// to NaNs.
343+
/// If either of `x` or `y` is a signaling NaN, the result is a quiet NaN.
344+
/// Otherwise, the result is `x` if x > y, `y` if x <= y, and whichever
345+
/// is a number if the other is a quiet NaN. If both `x` and `y` are quiet
346+
/// NaNs, a quiet NaN is returned.
353347
static func maximum(_ x: Self, _ y: Self) -> Self
354348

355349
/// Whichever of `x` or `y` has lesser magnitude. Implements the IEEE 754
356350
/// `minNumMag` operation.
357351
///
358-
/// Returns `x` if abs(x) <= abs(y), `y` if abs(y) < abs(x), and whichever of
359-
/// `x` or `y` is a number if the other is NaN. The result is NaN
360-
/// only if both arguments are NaN.
352+
/// If either of `x` or `y` is a signaling NaN, the result is a quiet NaN.
353+
/// Otherwise, the result is `x` if abs(x) <= abs(y), `y` if abs(x) > abs(y),
354+
/// and whichever is a number if the other is a quiet NaN. If both `x` and
355+
/// `y` are quiet NaNs, a quiet NaN is returned.
361356
static func minimumMagnitude(_ x: Self, _ y: Self) -> Self
362357

363358
/// Whichever of `x` or `y` has greater magnitude. Implements the IEEE 754
364359
/// `maxNumMag` operation.
365360
///
366-
/// Returns `x` if abs(x) >= abs(y), `y` if abs(y) > abs(x), and whichever of
367-
/// `x` or `y` is a number if the other is NaN. The result is NaN
368-
/// only if both arguments are NaN.
361+
/// If either of `x` or `y` is a signaling NaN, the result is a quiet NaN.
362+
/// Otherwise, the result is `x` if abs(x) > abs(y), `y` if abs(x) <= abs(y),
363+
/// and whichever is a number if the other is a quiet NaN. If both `x` and
364+
/// `y` are quiet NaNs, a quiet NaN is returned.
369365
static func maximumMagnitude(_ x: Self, _ y: Self) -> Self
370366

371367
/// The least representable value that compares greater than `self`.
@@ -710,22 +706,38 @@ extension FloatingPoint {
710706
*/
711707

712708
public static func minimum(_ left: Self, _ right: Self) -> Self {
709+
if left.isSignalingNaN || right.isSignalingNaN {
710+
// Produce a quiet NaN matching platform arithmetic behavior.
711+
return left + right
712+
}
713713
if left <= right || right.isNaN { return left }
714714
return right
715715
}
716716

717717
public static func maximum(_ left: Self, _ right: Self) -> Self {
718-
if left >= right || right.isNaN { return left }
718+
if left.isSignalingNaN || right.isSignalingNaN {
719+
// Produce a quiet NaN matching platform arithmetic behavior.
720+
return left + right
721+
}
722+
if left > right || right.isNaN { return left }
719723
return right
720724
}
721725

722726
public static func minimumMagnitude(_ left: Self, _ right: Self) -> Self {
727+
if left.isSignalingNaN || right.isSignalingNaN {
728+
// Produce a quiet NaN matching platform arithmetic behavior.
729+
return left + right
730+
}
723731
if abs(left) <= abs(right) || right.isNaN { return left }
724732
return right
725733
}
726734

727735
public static func maximumMagnitude(_ left: Self, _ right: Self) -> Self {
728-
if abs(left) >= abs(right) || right.isNaN { return left }
736+
if left.isSignalingNaN || right.isSignalingNaN {
737+
// Produce a quiet NaN matching platform arithmetic behavior.
738+
return left + right
739+
}
740+
if abs(left) > abs(right) || right.isNaN { return left }
729741
return right
730742
}
731743

‎test/1_stdlib/FloatingPoint.swift.gyb

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,11 @@ FloatingPoint.test("Float80.nextUp, .nextDown")
375375

376376
#endif
377377

378+
379+
extension FloatingPoint {
380+
public var isQuietNaN: Bool { return isNaN && !isSignalingNaN }
381+
}
382+
378383
% for FloatSelf in ['Float32', 'Float64']:
379384

380385
func checkFloatingPointComparison_${FloatSelf}(
@@ -417,6 +422,7 @@ FloatingPoint.test("${FloatSelf}/{Comparable,Hashable,Equatable}") {
417422
${FloatSelf}.greatestFiniteMagnitude,
418423
${FloatSelf}.infinity,
419424
${FloatSelf}.nan,
425+
${FloatSelf}.signalingNaN,
420426
]
421427
#else
422428
let interestingValues: [${FloatSelf}] = [
@@ -438,6 +444,7 @@ FloatingPoint.test("${FloatSelf}/{Comparable,Hashable,Equatable}") {
438444
${FloatSelf}.greatestFiniteMagnitude,
439445
${FloatSelf}.infinity,
440446
${FloatSelf}.nan,
447+
${FloatSelf}.signalingNaN,
441448
]
442449
#endif
443450

@@ -470,6 +477,60 @@ FloatingPoint.test("${FloatSelf}/{Comparable,Hashable,Equatable}") {
470477
ExpectedComparisonResult.gt),
471478
lhs, rhs)
472479
}
480+
481+
// Check minimum, maximum, minimumMagnitude and maximumMagnitude.
482+
let min = ${FloatSelf}.minimum(lhs, rhs)
483+
let max = ${FloatSelf}.maximum(lhs, rhs)
484+
let minMag = ${FloatSelf}.minimumMagnitude(lhs, rhs)
485+
let maxMag = ${FloatSelf}.maximumMagnitude(lhs, rhs)
486+
// If either lhs or rhs is signaling, or if both are quiet NaNs, the
487+
// result is a quiet NaN.
488+
if lhs.isSignalingNaN || rhs.isSignalingNaN || (lhs.isNaN && rhs.isNaN) {
489+
expectTrue(min.isQuietNaN)
490+
expectTrue(max.isQuietNaN)
491+
expectTrue(minMag.isQuietNaN)
492+
expectTrue(maxMag.isQuietNaN)
493+
}
494+
// If only one of lhs and rhs is NaN, the result of the min/max
495+
// operations is always the other value. While contrary to all other
496+
// IEEE 754 basic operations, this property is critical to ensure
497+
// that clamping to a valid range behaves as expected.
498+
else if lhs.isNaN && !rhs.isNaN {
499+
expectEqual(min.bitPattern, rhs.bitPattern)
500+
expectEqual(max.bitPattern, rhs.bitPattern)
501+
expectEqual(minMag.bitPattern, rhs.bitPattern)
502+
expectEqual(maxMag.bitPattern, rhs.bitPattern)
503+
}
504+
else if rhs.isNaN && !lhs.isNaN {
505+
expectEqual(min.bitPattern, lhs.bitPattern)
506+
expectEqual(max.bitPattern, lhs.bitPattern)
507+
expectEqual(minMag.bitPattern, lhs.bitPattern)
508+
expectEqual(maxMag.bitPattern, lhs.bitPattern)
509+
}
510+
// If lhs and rhs are equal, min is lhs and max is rhs. This ensures
511+
// that the set {lhs, rhs} is the same as the set {min, max} so long
512+
// as lhs and rhs are non-NaN. This is less important for min/max than
513+
// it is for minMag and maxMag, but it makes sense to define this way.
514+
if lhs <= rhs {
515+
expectEqual(min.bitPattern, lhs.bitPattern)
516+
expectEqual(max.bitPattern, rhs.bitPattern)
517+
} else if lhs > rhs {
518+
expectEqual(max.bitPattern, lhs.bitPattern)
519+
expectEqual(min.bitPattern, rhs.bitPattern)
520+
}
521+
// If lhs and rhs have equal magnitude, minMag is lhs and maxMag is rhs.
522+
// This ensures that the set {lhs, rhs} is the same as the set
523+
// {minMag, maxMag} so long as lhs and rhs are non-NaN; this is a
524+
// restriction of the IEEE 754 rules, but it makes good sense and also
525+
// makes this primitive more useful for building head-tail arithmetic
526+
// operations.
527+
if abs(lhs) <= abs(rhs) {
528+
expectEqual(minMag.bitPattern, lhs.bitPattern)
529+
expectEqual(maxMag.bitPattern, rhs.bitPattern)
530+
} else if abs(lhs) > abs(rhs) {
531+
expectEqual(maxMag.bitPattern, lhs.bitPattern)
532+
expectEqual(minMag.bitPattern, rhs.bitPattern)
533+
}
473534
}
474535
}
475536
}
@@ -1022,4 +1083,3 @@ FloatingPointClassification.test("FloatingPointClassification/Equatable") {
10221083
}
10231084

10241085
runAllTests()
1025-

0 commit comments

Comments
 (0)