4ad/go.arm64

cmd/internal/ld: portable registerizer generates invalid code

4ad opened this issue · 7 comments

4ad commented

Surely this is bad:

GOARCH=arm64 go tool 7g -S -o /home/aram/tmp/timediv.7 /home/aram/tmp/timediv.go
"".timediv t=1 nosplit size=416 value=0 args=0x20 locals=0x0 leaf
    0x0000 00000 (/home/aram/tmp/timediv.go:4)  TEXT    "".timediv(SB),4,$-8-32
    0x0000 00000 (/home/aram/tmp/timediv.go:4)  MOVD    "".rem+16(FP),R0
    0x0004 00004 (/home/aram/tmp/timediv.go:4)  MOVW    "".div+8(FP),R0
    0x0008 00008 (/home/aram/tmp/timediv.go:4)  MOVD    "".v(FP),R0
davecheney commented

is that it ? where is the RET ?

On Fri, Mar 6, 2015 at 11:05 PM, Aram Hăvărneanu notifications@github.com
wrote:

Surely this is bad:

GOARCH=arm64 go tool 7g -S -o /home/aram/tmp/timediv.7 /home/aram/tmp/timediv.go
"".timediv t=1 nosplit size=416 value=0 args=0x20 locals=0x0 leaf
0x0000 00000 (/home/aram/tmp/timediv.go:4) TEXT "".timediv(SB),4,$-8-32
0x0000 00000 (/home/aram/tmp/timediv.go:4) MOVD "".rem+16(FP),R0
0x0004 00004 (/home/aram/tmp/timediv.go:4) MOVW "".div+8(FP),R0
0x0008 00008 (/home/aram/tmp/timediv.go:4) MOVD "".v(FP),R0

—
Reply to this email directly or view it on GitHub
https://github.com/4ad/go/issues/145.

4ad commented

Well, no, that's only the beginning of the function, but it's already wrong, using the same register for all arguments is bad.

davecheney commented

omg, i missed that. /facepalm

On Fri, Mar 6, 2015 at 11:09 PM, Aram Hăvărneanu notifications@github.com
wrote:

Well, no, that's only the beginning of the function, but it's already
wrong, using the same register for all arguments is bad.

—
Reply to this email directly or view it on GitHub
https://github.com/4ad/go/issues/145#issuecomment-77549642.

4ad commented

Also, further down R0 is not even used, it's overwritten with ZR.

4ad commented

Fixed I think, pushing fix soon.

minux commented
minux commented

OK. I see. the ppc64 code is correct, as R0 is the zero register,
and shouldn't be assigned.