add math.isNaN

[timotheecour]

as:
https://en.cppreference.com/w/c/numeric/math/isnan
https://numpy.org/doc/stable/reference/generated/numpy.isnan.html
etc

note

making isNaN work with --passc:-ffast-math is tricky (see also https://bugs.launchpad.net/mixxx/+bug/1524561); I made it work by wrapping cmath.isnan which I've added to a new module cmath, intended to grow (refs: nim-lang/RFCs#92)

in particular:

• this worked on windows but for some obscure reason not on OSX:

proc isNaN*(x: SomeFloat): bool {.inline, codegenDecl: """ __attribute__ ((optimize("fno-fast-math"))) $1 $2 $3""".} =
  result = x != x

• {.localpassc:"-fno-fast-math”.} would apply to whole module, and moving it to a different module with different compilation options would prevent inlining.

So wrapping cmath.isnan was the simplest, and is also the most efficient in benchmark below.

future work

• make classify work with --passc:-ffast-math
• add targets:"c js" to tmath after fixing js bugs
• address this: add math.isNaN #16179 (comment) (add isFinite etc)

CI failure unrelated:

timotheecour#442

[ringabout]

It needs some benchmarks.

proc isNaN*(x: SomeFloat): bool {.inline.} =
  ## Returns whether `x` is a `NaN`, more efficiently than via `classify(x) == fcNan`.
  ## Does not work with: `--passc:-ffast-math`.
  result = x != x


proc cisNAN*(x: float): bool {.importc: "isnan", header: "<math.h>".}

import criterion


var cfg = newDefaultConfig()

benchmark cfg:
  proc tisNaN() {.measure.} =
    doAssert not isNaN(1.0)
    doAssert isNaN(NaN)


  proc tcisNan() {.measure.} =
    doAssert not cisNAN(1.0)
    doAssert cisNAN(NaN)

[ringabout]

It seems to generate the same asm codes in release build.

sub	rsp, 56	 #,
xor	ecx, ecx	 #
mov	QWORD PTR 40[rsp], 0	 # MEM[(void *)&T1_],

[timotheecour]

import math
proc isNaN*(x: SomeFloat): bool {.inline.} =
  ## Returns whether `x` is a `NaN`, more efficiently than via `classify(x) == fcNan`.
  ## Does not work with: `--passc:-ffast-math`.
  result = x != x

proc cisNAN*(x: float): bool {.importc: "isnan", header: "<math.h>".}
proc isNaN2*(x: float): bool {.inline.} = classify(x) == fcNan

when true:
  import math
  import times
  import random
  template mainAux(fn) =
    let n = 100_000_000
    var x = 0

    var list: seq[float]
    for i in 0..<n:
      let i2 = rand(int.high)
      let a = cast[float](i2)
      list.add a

    let t = cpuTime()
    for i in 0..<n:
      let a = list[i]
      x += cast[int](fn(a))
    let t2 = cpuTime()
    doAssert x != -1
    echo (astToStr(fn), t2 - t, x, x / n)

  proc main() =
    for i in 0..<3:
      echo (i,)
      mainAux(cisNAN)
      mainAux(isNaN)
      mainAux(isNaN2)
  main()

nim r -d:danger main

(0,)
("cisNAN", 0.04864900000000016, 48569, 0.00048569)
("isNaN", 0.04862300000000008, 48797, 0.00048797)
("isNaN2", 0.2606590000000004, 48618, 0.00048618)
(1,)
("cisNAN", 0.0488110000000006, 48811, 0.00048811)
("isNaN", 0.04828999999999972, 48755, 0.00048755)
("isNaN2", 0.270857000000003, 48865, 0.00048865)
(2,)
("cisNAN", 0.05039000000000016, 48807, 0.00048807)
("isNaN", 0.04703400000000002, 48778, 0.00048778)
("isNaN2", 0.2743899999999968, 49253, 0.00049253)

one interesting point is that cisNAN works with --passc:-ffast-math;

I'll need to see whether i can "disable" locally -ffast-math with push/pop

Note: there's some slight statistical flaw in this benchmark, because whichever is first (cisNAN here) gets less cache misses than others; let me know if you have a simple fix for that

[ringabout]

I look at the generated asm, they are the same

proc tcisNan() =
  let x1 = cisNaN(1.0)
  echo x1

proc tisNaN() =
  let x2 = isNaN(1.0)
  echo x2

tisNaN()
tcisNan()

asm codes

sub	rsp, 56	 #,
xor	ecx, ecx	 #
mov	QWORD PTR 40[rsp], 0	 # MEM[(void *)&T1_],

[timotheecour]

PTAL

• isNaN now works with --passc:-ffast-math
• added std/cmath

[Araq]

It seems to cause less friction if you also add isFinite to math.nim and remove the declarations from dom.nim. Well, dom.nim can import math and re-export these.

[timotheecour]

It seems to cause less friction if you also add isFinite to math.nim and remove the declarations from dom.nim. Well, dom.nim can import math and re-export these.

based on #16179 (comment) this can be done in future work

[Araq]

Put this declaration in math.nim and don't export it. And name it cisnan or better.

[timotheecour]

done (with c_isnan) to unblock this PR but my plan is nim-lang/RFCs#92 (comment), can we continue this conversation there ? (already in an RFC)

[timotheecour]

PTAL

[timotheecour]

ping (rebased after bitrot conflicts)

[Araq]

It's not clear to me if the CI failures are related.

[ringabout]

CI failure is related to #16297 (comment)

[timotheecour]

@Araq PTAL

It's not clear to me if the CI failures are related.

I've reported the problem upstream and it's now fixed: https://todo.sr.ht/~sircmpwn/builds.sr.ht/312/#event-61438