fix min/max for float numbers (#12068)

8 files changed, 30 insertions, 36 deletions

diff --git a/compiler/ast.nim b/compiler/ast.nim
index 71d19be30..db0321f6c 100644
--- a/compiler/ast.nim
+++ b/compiler/ast.nim
@@ -608,7 +608,6 @@ type
     mAddF64, mSubF64, mMulF64, mDivF64,
     mShrI, mShlI, mAshrI, mBitandI, mBitorI, mBitxorI,
     mMinI, mMaxI,
-    mMinF64, mMaxF64,
     mAddU, mSubU, mMulU, mDivU, mModU,
     mEqI, mLeI, mLtI,
     mEqF64, mLeF64, mLtF64,
@@ -621,7 +620,7 @@ type
     mXor, mEqCString, mEqProc,
     mUnaryMinusI, mUnaryMinusI64, mAbsI, mNot,
     mUnaryPlusI, mBitnotI,
-    mUnaryPlusF64, mUnaryMinusF64, mAbsF64,
+    mUnaryPlusF64, mUnaryMinusF64,
     mCharToStr, mBoolToStr, mIntToStr, mInt64ToStr, mFloatToStr, mCStrToStr,
     mStrToStr, mEnumToStr,
     mAnd, mOr,
@@ -678,7 +677,6 @@ const
     mAddF64, mSubF64, mMulF64, mDivF64,
     mShrI, mShlI, mBitandI, mBitorI, mBitxorI,
     mMinI, mMaxI,
-    mMinF64, mMaxF64,
     mAddU, mSubU, mMulU, mDivU, mModU,
     mEqI, mLeI, mLtI,
     mEqF64, mLeF64, mLtF64,
@@ -689,7 +687,7 @@ const
     mEqB, mLeB, mLtB,
     mEqRef, mEqProc, mEqUntracedRef, mLePtr, mLtPtr, mEqCString, mXor,
     mUnaryMinusI, mUnaryMinusI64, mAbsI, mNot, mUnaryPlusI, mBitnotI,
-    mUnaryPlusF64, mUnaryMinusF64, mAbsF64,
+    mUnaryPlusF64, mUnaryMinusF64,
     mCharToStr, mBoolToStr, mIntToStr, mInt64ToStr, mFloatToStr, mCStrToStr,
     mStrToStr, mEnumToStr,
     mAnd, mOr,
diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim
index 0ab592a50..0902e2e19 100644
--- a/compiler/ccgexprs.nim
+++ b/compiler/ccgexprs.nim
@@ -588,8 +588,6 @@ proc binaryArith(p: BProc, e: PNode, d: var TLoc, op: TMagic) =
   of mBitxorI: applyFormat("($4)($1 ^ $2)")
   of mMinI: applyFormat("(($1 <= $2) ? $1 : $2)")
   of mMaxI: applyFormat("(($1 >= $2) ? $1 : $2)")
-  of mMinF64: applyFormat("(($1 <= $2) ? $1 : $2)")
-  of mMaxF64: applyFormat("(($1 >= $2) ? $1 : $2)")
   of mAddU: applyFormat("($4)((NU$3)($1) + (NU$3)($2))")
   of mSubU: applyFormat("($4)((NU$3)($1) - (NU$3)($2))")
   of mMulU: applyFormat("($4)((NU$3)($1) * (NU$3)($2))")
@@ -663,9 +661,6 @@ proc unaryArith(p: BProc, e: PNode, d: var TLoc, op: TMagic) =
     applyFormat("$1")
   of mUnaryMinusF64:
     applyFormat("-($1)")
-  of mAbsF64:
-    applyFormat("($1 < 0? -($1) : ($1))")
-    # BUGFIX: fabs() makes problems for Tiny C
   else:
     assert false, $op
 
@@ -2086,7 +2081,7 @@ proc genEnumToStr(p: BProc, e: PNode, d: var TLoc) =
 proc genMagicExpr(p: BProc, e: PNode, d: var TLoc, op: TMagic) =
   case op
   of mOr, mAnd: genAndOr(p, e, d, op)
-  of mNot..mAbsF64: unaryArith(p, e, d, op)
+  of mNot..mUnaryMinusF64: unaryArith(p, e, d, op)
   of mUnaryMinusI..mAbsI: unaryArithOverflow(p, e, d, op)
   of mAddF64..mDivF64: binaryFloatArith(p, e, d, op)
   of mShrI..mXor: binaryArith(p, e, d, op)
diff --git a/compiler/guards.nim b/compiler/guards.nim
index 1ab50b8b6..be23c1598 100644
--- a/compiler/guards.nim
+++ b/compiler/guards.nim
@@ -35,8 +35,8 @@ const
   someMul = {mMulI, mMulF64}
   someDiv = {mDivI, mDivF64}
   someMod = {mModI}
-  someMax = {mMaxI, mMaxF64}
-  someMin = {mMinI, mMinF64}
+  someMax = {mMaxI}
+  someMin = {mMinI}
   someBinaryOp = someAdd+someSub+someMul+someMax+someMin
 
 proc isValue(n: PNode): bool = n.kind in {nkCharLit..nkNilLit}
diff --git a/compiler/jsgen.nim b/compiler/jsgen.nim
index 1d980ab6f..78373ea47 100644
--- a/compiler/jsgen.nim
+++ b/compiler/jsgen.nim
@@ -389,8 +389,6 @@ const # magic checked op; magic unchecked op;
     ["", ""], # BitxorI
     ["nimMin", "nimMin"], # MinI
     ["nimMax", "nimMax"], # MaxI
-    ["nimMin", "nimMin"], # MinF64
-    ["nimMax", "nimMax"], # MaxF64
     ["", ""], # addU
     ["", ""], # subU
     ["", ""], # mulU
@@ -430,7 +428,6 @@ const # magic checked op; magic unchecked op;
     ["", ""], # BitnotI
     ["", ""], # UnaryPlusF64
     ["", ""], # UnaryMinusF64
-    ["", ""], # AbsF64
     ["nimCharToStr", "nimCharToStr"],
     ["nimBoolToStr", "nimBoolToStr"],
     ["cstrToNimstr", "cstrToNimstr"],
@@ -566,8 +563,6 @@ proc arithAux(p: PProc, n: PNode, r: var TCompRes, op: TMagic) =
   of mBitxorI: applyFormat("($1 ^ $2)", "($1 ^ $2)")
   of mMinI: applyFormat("nimMin($1, $2)", "nimMin($1, $2)")
   of mMaxI: applyFormat("nimMax($1, $2)", "nimMax($1, $2)")
-  of mMinF64: applyFormat("nimMin($1, $2)", "nimMin($1, $2)")
-  of mMaxF64: applyFormat("nimMax($1, $2)", "nimMax($1, $2)")
   of mAddU: applyFormat("", "")
   of mSubU: applyFormat("", "")
   of mMulU: applyFormat("", "")
@@ -607,7 +602,6 @@ proc arithAux(p: PProc, n: PNode, r: var TCompRes, op: TMagic) =
   of mBitnotI: applyFormat("~($1)", "~($1)")
   of mUnaryPlusF64: applyFormat("+($1)", "+($1)")
   of mUnaryMinusF64: applyFormat("-($1)", "-($1)")
-  of mAbsF64: applyFormat("Math.abs($1)", "Math.abs($1)")
   of mCharToStr: applyFormat("nimCharToStr($1)", "nimCharToStr($1)")
   of mBoolToStr: applyFormat("nimBoolToStr($1)", "nimBoolToStr($1)")
   of mIntToStr: applyFormat("cstrToNimstr(($1)+\"\")", "cstrToNimstr(($1)+\"\")")
diff --git a/compiler/semfold.nim b/compiler/semfold.nim
index bfb5077d6..3bc6f0f23 100644
--- a/compiler/semfold.nim
+++ b/compiler/semfold.nim
@@ -197,7 +197,6 @@ proc evalOp(m: TMagic, n, a, b, c: PNode; g: ModuleGraph): PNode =
       result = newIntNodeT(toInt128(sonsLen(a)), n, g)
   of mUnaryPlusI, mUnaryPlusF64: result = a # throw `+` away
   # XXX: Hides overflow/underflow
-  of mAbsF64: result = newFloatNodeT(abs(getFloat(a)), n, g)
   of mAbsI: result = foldAbs(getInt(a), n, g)
   of mUnaryLt: result = foldSub(getOrdValue(a), One, n, g)
   of mSucc: result = foldAdd(getOrdValue(a), getInt(b), n, g)
@@ -276,12 +275,6 @@ proc evalOp(m: TMagic, n, a, b, c: PNode; g: ModuleGraph): PNode =
   of mMulF64: result = newFloatNodeT(getFloat(a) * getFloat(b), n, g)
   of mDivF64:
     result = newFloatNodeT(getFloat(a) / getFloat(b), n, g)
-  of mMaxF64:
-    if getFloat(a) > getFloat(b): result = newFloatNodeT(getFloat(a), n, g)
-    else: result = newFloatNodeT(getFloat(b), n, g)
-  of mMinF64:
-    if getFloat(a) > getFloat(b): result = newFloatNodeT(getFloat(b), n, g)
-    else: result = newFloatNodeT(getFloat(a), n, g)
   of mIsNil: result = newIntNodeT(ord(a.kind == nkNilLit), n, g)
   of mLtI, mLtB, mLtEnum, mLtCh:
     result = newIntNodeT(ord(getOrdValue(a) < getOrdValue(b)), n, g)
diff --git a/compiler/vmgen.nim b/compiler/vmgen.nim
index 15ddfe496..9a16bd5b9 100644
--- a/compiler/vmgen.nim
+++ b/compiler/vmgen.nim
@@ -1294,7 +1294,7 @@ proc genMagic(c: PCtx; n: PNode; dest: var TDest; m: TMagic) =
     if dest < 0: dest = c.getTemp(n.typ)
     c.gABC(n, opcCallSite, dest)
   of mNGenSym: genBinaryABC(c, n, dest, opcGenSym)
-  of mMinI, mMaxI, mAbsF64, mMinF64, mMaxF64, mAbsI, mDotDot:
+  of mMinI, mMaxI, mAbsI, mDotDot:
     c.genCall(n, dest)
   of mExpandToAst:
     if n.len != 2:
diff --git a/lib/system.nim b/lib/system.nim
index 6b1421160..05dc43ab2 100644
--- a/lib/system.nim
+++ b/lib/system.nim
@@ -2880,17 +2880,21 @@ proc max*[T](x: openArray[T]): T =
   for i in 1..high(x):
     if result < x[i]: result = x[i]
 
-proc abs*(x: float): float {.magic: "AbsF64", noSideEffect.} =
+proc abs*(x: float64): float64 {.noSideEffect, inline.} =
   if x < 0.0: -x else: x
-proc min*(x, y: float): float {.magic: "MinF64", noSideEffect.} =
-  if x <= y: x else: y
-proc max*(x, y: float): float {.magic: "MaxF64", noSideEffect.} =
-  if y <= x: x else: y
-
-proc min*[T](x, y: T): T {.inline.}=
+proc abs*(x: float32): float32 {.noSideEffect, inline.} =
+  if x < 0.0: -x else: x
+proc min*(x, y: float32): float32 {.noSideEffect, inline.} =
+  if x <= y or y != y: x else: y
+proc min*(x, y: float64): float64 {.noSideEffect, inline.} =
+  if x <= y or y != y: x else: y
+proc max*(x, y: float32): float32 {.noSideEffect, inline.} =
+  if y <= x or y != y: x else: y
+proc max*(x, y: float64): float64 {.noSideEffect, inline.} =
+  if y <= x or y != y: x else: y
+proc min*[T: not SomeFloat](x, y: T): T {.inline.} =
   if x <= y: x else: y
-
-proc max*[T](x, y: T): T {.inline.}=
+proc max*[T: not SomeFloat](x, y: T): T {.inline.} =
   if y <= x: x else: y
 {.pop.}
 
diff --git a/tests/system/tsystem_misc.nim b/tests/system/tsystem_misc.nim
index 9dcd9ac9f..38be01a13 100644
--- a/tests/system/tsystem_misc.nim
+++ b/tests/system/tsystem_misc.nim
@@ -160,7 +160,6 @@ block: # `$`*[T: tuple|object](x: T)
   doAssert $Foo(x:2) == "(x: 2, x2: 0.0)"
   doAssert $() == "()"
 
-
 # this is a call indirection to prevent `toInt` to be resolved at compile time.
 proc testToInt(arg: float64, a: int, b: BiggestInt) =
   doAssert toInt(arg) == a
@@ -174,3 +173,14 @@ testToInt(13.37, 13, 13)    # should round towards 0
 testToInt(-13.37, -13, -13) # should round towards 0
 testToInt(7.8, 8, 8)     # should round away from 0
 testToInt(-7.8, -8, -8)  # should round away from 0
+
+# test min/max for correct NaN handling
+
+proc testMinMax(a,b: float32) =
+  doAssert max(float32(a),float32(b)) == 0'f32
+  doAssert min(float32(a),float32(b)) == 0'f32
+  doAssert max(float64(a),float64(b)) == 0'f64
+  doAssert min(float64(a),float64(b)) == 0'f64
+
+testMinMax(0.0, NaN)
+testMinMax(NaN, 0.0)