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

runtime: mkfastlog2table produces different output on darwin/arm64 #49891

Open
josharian opened this issue Dec 1, 2021 · 9 comments
Open

runtime: mkfastlog2table produces different output on darwin/arm64 #49891

josharian opened this issue Dec 1, 2021 · 9 comments
Labels
Milestone

Comments

@josharian
Copy link
Contributor

@josharian josharian commented Dec 1, 2021

On darwin/arm64 at 3ca57c7 (Nov 29, 2021), running go generate in package runtime generates a diff:

diff --git a/src/runtime/fastlog2table.go b/src/runtime/fastlog2table.go
index 6ba4a7d3f2..d8bb953939 100644
--- a/src/runtime/fastlog2table.go
+++ b/src/runtime/fastlog2table.go
@@ -8,22 +8,22 @@ const fastlogNumBits = 5
 
 var fastlog2Table = [1<<fastlogNumBits + 1]float64{
 	0,
-	0.0443941193584535,
-	0.08746284125033943,
-	0.12928301694496647,
-	0.16992500144231248,
-	0.2094533656289499,
+	0.044394119358453485,
+	0.08746284125033939,
+	0.12928301694496644,
+	0.16992500144231246,
+	0.20945336562894987,
 	0.24792751344358555,
 	0.28540221886224837,
-	0.3219280948873623,
-	0.3575520046180837,
-	0.39231742277876036,
-	0.4262647547020979,
-	0.4594316186372973,
-	0.4918530963296748,
+	0.32192809488736235,
+	0.35755200461808373,
+	0.3923174227787603,
+	0.42626475470209796,
+	0.45943161863729726,
+	0.4918530963296747,
 	0.5235619560570128,
 	0.5545888516776374,
-	0.5849625007211563,
+	0.5849625007211562,
 	0.6147098441152082,
 	0.6438561897747247,
 	0.6724253419714956,
@@ -31,7 +31,7 @@ var fastlog2Table = [1<<fastlogNumBits + 1]float64{
 	0.7279204545631992,
 	0.7548875021634686,
 	0.7813597135246596,
-	0.8073549220576042,
+	0.8073549220576041,
 	0.8328900141647417,
 	0.8579809951275721,
 	0.8826430493618412,
diff --git a/src/runtime/preempt_mips64x.s b/src/runtime/preempt_mips64x.s
index 996b592ae0..99e680c39e 100644
--- a/src/runtime/preempt_mips64x.s
+++ b/src/runtime/preempt_mips64x.s
@@ -1,7 +1,6 @@
 // Code generated by mkpreempt.go; DO NOT EDIT.
 
 //go:build mips64 || mips64le
-
 #include "go_asm.h"
 #include "textflag.h"
 
diff --git a/src/runtime/preempt_mipsx.s b/src/runtime/preempt_mipsx.s
index 7b169acd99..6d808813d1 100644
--- a/src/runtime/preempt_mipsx.s
+++ b/src/runtime/preempt_mipsx.s
@@ -1,7 +1,6 @@
 // Code generated by mkpreempt.go; DO NOT EDIT.
 
 //go:build mips || mipsle
-
 #include "go_asm.h"
 #include "textflag.h"
 
diff --git a/src/runtime/preempt_ppc64x.s b/src/runtime/preempt_ppc64x.s
index 2c4d02edfe..66c33276ec 100644
--- a/src/runtime/preempt_ppc64x.s
+++ b/src/runtime/preempt_ppc64x.s
@@ -1,7 +1,6 @@
 // Code generated by mkpreempt.go; DO NOT EDIT.
 
 //go:build ppc64 || ppc64le
-
 #include "go_asm.h"
 #include "textflag.h"
 

Should we regenerate and commit prior to the release?

@josharian josharian added this to the Go1.18 milestone Dec 1, 2021
@robpike
Copy link
Contributor

@robpike robpike commented Dec 1, 2021

Is this a change due to registerization of arguments? Or some other optimization? It's just the last digit changing, but it would be nice to understand why.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 1, 2021

Why did it delete the blank lines? That's definitely a bug and not a math problem.

I see that's a different generator. That one's easy to fix.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 1, 2021

We don't need to regenerate and commit. The table is only used to get 5 digits of log.

It is still interesting to find out why arm64 produces slightly different numbers. We have excised all the 387 intrinsics, so that's not it. I thought maybe the x86-64 asm was producing a different answer from the pure Go that arm64 would use, but setting haveArchLog = false in math/log_asm.go on x86-64 did not change the behavior of mkfastlog2table.

At that point I think it's all pure Go code - Log2 calls Log calls Frexp, and only Log had any assembly (on x86-64). It's a bit concerning but not enough to block the release.

@rsc rsc changed the title runtime: 'go generate' makes changes runtime: mkfastlog2table produces different output on darwin/arm64 Dec 1, 2021
@randall77
Copy link
Contributor

@randall77 randall77 commented Dec 1, 2021

Could be FMA on arm64. mkfastlog2table doesn't have any FMA issues, but maybe there are FMA candidates in math.Log2?

@rsc
Copy link
Contributor

@rsc rsc commented Dec 1, 2021

log2 itself is a single FMA. Could be.

@randall77
Copy link
Contributor

@randall77 randall77 commented Dec 1, 2021

I've verified that disabling FMA in math.log2 (adding a float64 cast) fixes the precision differences.

@josharian
Copy link
Contributor Author

@josharian josharian commented Dec 1, 2021

Phew. So now it's just a question of finding a way to make the generator programs stable and match the existing runtime directory.

@randall77
Copy link
Contributor

@randall77 randall77 commented Dec 1, 2021

I think the easiest thing would be just to copy log2 out of math and into mkfastlog2table and add the float64 cast that disables FMA.

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Dec 2, 2021

This doesn't seem like it's necessarily a new issue in this release, and it certainly shouldn't block it. Moving this to the 1.19 milestone for now.

@mknyszek mknyszek removed this from the Go1.18 milestone Dec 2, 2021
@mknyszek mknyszek added this to the Go1.19 milestone Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants