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

Jump to bottom

math: math.Log does not handle subnormal floating point number on amd64 #56600

Open
liu-du opened this issue Nov 6, 2022 · 3 comments · May be fixed by #56601
Open

math: math.Log does not handle subnormal floating point number on amd64 #56600

liu-du opened this issue Nov 6, 2022 · 3 comments · May be fixed by #56601
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@liu-du
Copy link

liu-du commented Nov 6, 2022

What version of Go are you using (go version)?

$ go version
go version go1.18.3 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

macOS and amd64

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/$USER/Library/Caches/go-build"
GOENV="/Users/$USER/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/$USER/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/$USER/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.18.3/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.18.3/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.18.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/$USER/code/go/src/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/nn/pgfwsh7d0kn99mpmf7fd35_r0000gn/T/go-build681644206=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I tried math.Log on very small float64 (subnormal IEEE floating point number), it gives incorrect result. This only happens on amd64, when math/log_amd64.S is used.

Running the same go code on, for example, arm64 gives correct results. I found math/log_amd64.S doesn't normalize subnormal floating number, whereas the go implementation of math.log (which calls math.Frexp) does normalise the floating point number. Hence different arch gives different results.

What did you expect to see?

The natural log for subnormal floating number should be correct. The equivalent c code gives correct result (python/java also gives the same results as c, although they possibly just call the c implementation):

#include <stdio.h>
#include <math.h>

int main() {
    printf("%f\n", log(pow(2, -1074))); // -744.440072
    printf("%f\n", log(exp(1) * pow(2, -1074))); // -743.341460
    printf("%f\n", log(exp(2) * pow(2, -1074))); // -742.494162
    printf("%f\n", log(exp(3) * pow(2, -1074))); // -741.444340
}

What did you see instead?

// x should be around -744 but it's -709
x := math.Log(math.SmallestNonzeroFloat64) // -709.0895657128241

// should be around x + 1, but it's equal x
fmt.Println(math.Log(math.Exp(1) * math.SmallestNonzeroFloat64))// -709.0895657128241

// should be around x + 2, but it's equal x
fmt.Println(math.Log(math.Exp(2) * math.SmallestNonzeroFloat64)) // -709.0895657128241

// should be around x + 3, but it's equal x
fmt.Println(math.Log(math.Exp(3) * math.SmallestNonzeroFloat64)) // -709.0895657128241

go.dev/play link: https://go.dev/play/p/uksqrp1Zq0-
liu-du added a commit to liu-du/go that referenced this issue Nov 6, 2022
@liu-du
math: improve math.Log to handle subnormal floating number correctly …
d584d8d 
…on amd64

The existing implementation for math.Log on amd64/s390x, math/log_amd64.S,
doesn't normalize subnormal IEEE 64-bit floating point number correctly
this only occurs on amd64/s390x, the go implementation of log, which is used on
other architecture does handle subnormal floating number correctly. This
commit add normalize logic to math/log_amd64.S.

Fixes golang#56600
@liu-du liu-du changed the title affected/package: math math: math.Log does not handle subnormal floating point number on amd64 Nov 6, 2022
@liu-du liu-du linked a pull request Nov 6, 2022 that will close this issue
Open
@gopherbot
Copy link

Change https://go.dev/cl/448216 mentions this issue: math: improve math.Log to handle subnormal floating number on amd64

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 6, 2022
@seankhliao
Copy link
Member

cc @griesemer @rsc

liu-du added a commit to liu-du/go that referenced this issue Nov 6, 2022
@liu-du
math: improve math.Log to handle subnormal floating number correctly …
d6edc2c 
…on amd64

The existing implementation for math.Log on amd64/s390x, math/log_amd64.S,
doesn't normalize subnormal IEEE 64-bit floating point number correctly.
This only occurs on amd64/s390x. The go implementation of log, which is used
on other architecture does handle subnormal floating number correctly.
This commit add normalize logic to math/log_amd64.S.

Fixes golang#56600
@seankhliao seankhliao added this to the Unplanned milestone Nov 19, 2022
@liu-du
Copy link
Author

liu-du commented Aug 10, 2023

Hi @seankhliao, I'm revisiting this issue after while and hoping to make a (small) contribution and close the loop, do you have any feedback on the PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

Successfully merging a pull request may close this issue.

math: improve math.Log to handle subnormal floating number on amd64 liu-du/go
3 participants
@gopherbot @seankhliao @liu-du