Skip to content
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.

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

VM: float32 values incorrect, resulting in differences between RT and CT #12884

Closed
timotheecour opened this issue Dec 11, 2019 · 5 comments
Closed
Labels
Documentation Content Related to documentation content (not generation). Language Design Postponed VM see also `const` label

Comments

@timotheecour
Copy link
Member

VM code should as much as possible produce identical results as runtime code; but that's not the case for float32:

Example1

proc main()=
  const x0: float32 = 1.32
  let x1 = 1.32
  let x2 = 1.32'f32
  var x3: float32 = 1.32
  echo (x0, x1, x2, x3)

static: main()
main()

Current Output

(1.32, 1.32, 1.32, 1.32)
(1.320000052452087, 1.32, 1.320000052452087, 1.320000052452087)

Expected Output

(1.320000052452087, 1.32, 1.320000052452087, 1.320000052452087)
(1.320000052452087, 1.32, 1.320000052452087, 1.320000052452087)

Example2

this shows that there's a discrepency in VM between float32's from FFI and float32 from litterals:
get_float returns 1.320000052452087 (as it should) which does not match the incorrect 1.32 from 1.32'f32

when defined(timn_D20191211T153325):
  {.emit:"""
  float get_float(){
    return 1.32;
  }
  """.}

else:
  import std/[strformat,os]
  const libF = "/tmp/libD20191211T153325.dylib"
  const nim = getCurrentCompilerExe()
  const file = currentSourcePath()
  static:
    let cmd = fmt"{nim} c -d:timn_D20191211T153325 --app:lib -o:{libF} {file}"
    echo cmd
    let (output, exitCode) = gorgeEx cmd
    doAssert exitCode == 0, output

  proc get_float(): float32 {.importc, dynlib: libF.}

  proc main()=
    let x = get_float()
    let x2 = 1.32'f32
    var x3: float32 = 1.32
    echo (x, x2, x3)

  static: main()
  main()

prints:

(1.320000052452087, 1.32, 1.32)
(1.320000052452087, 1.320000052452087, 1.320000052452087)

Additional Information

recent devel 7213969

@mratsim mratsim added the VM see also `const` label label Dec 12, 2019
@mratsim
Copy link
Collaborator

mratsim commented Dec 12, 2019

That's because in the VM everything is a float64.

Now, we don't lose precision here and I think float32 in the VM would be extra maintenance with few use cases when we already have float64.

My take is to add this as an exception to the specs. "As an exception to VM and constant folding computations being equal to runtime computations, all floating point operations are carried with 64-bit precision at compile-time."

@timotheecour
Copy link
Member Author

timotheecour commented Dec 12, 2019

the final representation is up-casted to float64 but the intermediate one should be whatever user asked for, ie float32 in case above; float64 can represent all float32 values so these values should be preserved
so I'd expect it to be float64(1.32'f32) = 1.320000052452087, not 1.32

on a related note, echo 1.32'f32 prints differently from

  let x2 = 1.32'f32
  echo x2

probably due to same issue; I consider it a bug

proc main()=
  let x = float(1.32'f32)
  echo x 
  echo 1.32
  echo 1.32'f32
  let x2 = 1.32'f32
  echo x2

main()

output:

1.320000052452087
1.32
1.32
1.320000052452087

@Araq
Copy link
Member

Araq commented Dec 12, 2019

The C spec also says that floating point compile time evaluations can have higher precision. It's just an inherit complexity in compilers, it needs to be documented, it is not a bug.

@Araq Araq added the Documentation Content Related to documentation content (not generation). label Dec 12, 2019
timotheecour added a commit to timotheecour/Nim that referenced this issue Dec 16, 2019
@timotheecour
Copy link
Member Author

#12906 mitigates this; after this PR operations (eg multiplications) can be computed using higher precision, but declarations (eg float32 litterals) and float32 conversions shall use the user requested precision to match RT code

@Araq
Copy link
Member

Araq commented Dec 18, 2019

In fact, the spec already says that:

An implementation should always use the maximum precision available to evaluate
floating pointer values during semantic analysis; this means expressions like
0.09'f32 + 0.01'f32 == 0.09'f64 + 0.01'f64 that are evaluating during
constant folding are true.

So ... closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Content Related to documentation content (not generation). Language Design Postponed VM see also `const` label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants