math/big: ExampleFloat_Add test failure on s390x

[mundaym]

The builder started failing when CL 35555 was merged (0358367). The failure looks like this:

--- FAIL: ExampleFloat_Add (0.00s)
got:
x = 1000 (0x.fap+10, prec = 64, acc = Exact)
y = 2.718281828 (0x.adf85458248cd8p+2, prec = 53, acc = Exact)
z = 1002.718282 (0x.faadf854p+10, prec = 32, acc = Accuracy(8))
want:
x = 1000 (0x.fap+10, prec = 64, acc = Exact)
y = 2.718281828 (0x.adf85458248cd8p+2, prec = 53, acc = Exact)
z = 1002.718282 (0x.faadf854p+10, prec = 32, acc = Below)
FAIL
FAIL	math/big	1.459s

I will investigate further tomorrow.

[mundaym]

cc @josharian @billotosyr

[josharian]

Interesting. I don't have s390x access, but please keep me posted and ask here if there are any questions I can answer.

[josharian]

I've started poking a bit at this, but am unlikely to have answers tonight or tomorrow. A few notes as I poke around:

• I suspect the problem is that the int8 -1 value ("Below") is getting transformed into a positive 8 ("Accuracy(8)") instead of a positive 255 before being adding to the address of staticbytes.
• I don't know S390x, but it looks like some constant folding is missing. Const 255 * Const 1 gets lowered to (1 << 8) - 1, rather than just plain 255. (At least, I assume SLDconst is <<.)

[josharian]

Reproduction code for the missing constant folding:

package main

import "fmt"

func main() {
	x := int8(-1)
	fmt.Println(x)
}

(This should also get optimized at the generic level, but it currently isn't.)

[gopherbot]

CL https://golang.org/cl/36217 mentions this issue.

[josharian]

CL 36217 adds some generic optimizations for code like this. S390x puzzle remains. I hope @billotosyr has more luck than I.

If the above guess at a standalone reproducer doesn't do it, you could try:

package main

import "fmt"

//go:noinline
func f() int8 { return -1 }

func main() { fmt.Println(f()) }

If not, well, ExampleFloat_Add it is.

[mundaym]

Thanks for taking a look @josharian.

This looks like a type propagation issue to me. The big.Accuracy value is correctly zero extended when it is first loaded but when it is spilled and re-loaded it is sign-extended:

v79 = MOVBZload <big.Accuracy> [5] v155 v74 : R0
v149 = StoreReg <big.Accuracy> v79 : ~r0[big.Accuracy]
...
v52 = LoadReg <big.Accuracy> v149 : R0
v46 = MOVDaddr <*[256]uint8> {runtime.staticbytes} v3 : R1
v125 = ADD <*uint8> v46 v52 : R0

v52 is turned into a MOVBload (sign extending load) because big.Accuracy is a signed type. So the offset from staticbytes is -1 rather than +255 (i.e. we are reading the data preceding the staticbytes array).

The StoreReg and LoadReg should, I think, have the type <uint8> or equivalent.

[mundaym]

Actually, StoreReg and LoadReg should have the type <int>:

v79 = Load <big.Accuracy> v78 v74
v123 = ZeroExt8to64 <int> v79

[gopherbot]

CL https://golang.org/cl/36256 mentions this issue.

[gopherbot]

CL https://golang.org/cl/36350 mentions this issue.