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

cmd/compile: incorrect double rounding for conversion from uint64 to float32 on 32-bit platforms #48807

Closed
ianlancetaylor opened this issue Oct 5, 2021 · 2 comments

Comments

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 5, 2021

Test program:

package main

import (
        "fmt"
)

func s(k uint64) float32 {
        var x float32
        x = float32(k)
        return x
}

func main() {
        k := uint64(0x8234508000000001)
	x2 := float64(k)
	fmt.Printf("%x\n", x2)
        x := float64(s(k))
	fmt.Printf("%x\n", x)
}

On amd64 this prints

0x1.0468a1p+63
0x1.0468a2p+63

On 386 it prints

0x1.0468a1p+63
0x1.0468ap+63

The second number is different. This is happening because on 386 we convert a uint64 to float32 by first converting to float64 (via runtime.uint64tofloat64) and then converting that to float32. This introduces a double-rounding. Specifically the uint64 value 0x1.0468a1p+63 converts to float32 0x1.0468ap+63, but the correct result of the original rounding is 0x1.0468a2p+63.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 6, 2021

Change https://golang.org/cl/354429 mentions this issue: cmd/compile,runtime: implement uint64->float32 correctly on 32-bit archs

Loading

@gopherbot gopherbot closed this in 2043b3b Oct 7, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 7, 2021

Change https://golang.org/cl/354613 mentions this issue: runtime: fix uint64->float32 conversion for softfloat

Loading

gopherbot pushed a commit that referenced this issue Oct 8, 2021
The fix for #48807 in CL 354429 forgot that we also need to fix
the softfloat implementation.

Update #48807

Change-Id: I596fb4e14e78145d1ad43c130b2cc5122b73655c
Reviewed-on: https://go-review.googlesource.com/c/go/+/354613
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants