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.

Sign up for GitHub

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

cmd/compile: unnecessary zeroing of register on arm #8474

Closed
davecheney opened this issue Aug 5, 2014 · 3 comments
Closed

cmd/compile: unnecessary zeroing of register on arm #8474

davecheney opened this issue Aug 5, 2014 · 3 comments
Labels
FrozenDueToAge
Milestone
Go1.8

Comments

@davecheney
Copy link
Contributor 

What steps will reproduce the problem?

func add64(a, b int) int {
        x := a + b
        return x
}

What is the expected output? What do you see instead?

Expected (from 6g)

"".add64 t=1 size=32 value=0 args=0x18 locals=0x0
        0x0000 00000 (/home/dfc/src/add64.go:3) TEXT    "".add64+0(SB),4,$0-24
        0x0000 00000 (/home/dfc/src/add64.go:3) NOP     ,
        0x0000 00000 (/home/dfc/src/add64.go:3) NOP     ,
        0x0000 00000 (/home/dfc/src/add64.go:3) FUNCDATA        $2,gclocals·f90cfd099b5ec2b453c391fece9d42bb+0(SB)
        0x0000 00000 (/home/dfc/src/add64.go:3) FUNCDATA        $3,gclocals·3280bececceccd33cb74587feedb1f9f+0(SB)
        0x0000 00000 (/home/dfc/src/add64.go:4) MOVQ    "".a+8(FP),BX
        0x0005 00005 (/home/dfc/src/add64.go:4) MOVQ    "".b+16(FP),BP
        0x000a 00010 (/home/dfc/src/add64.go:4) ADDQ    BP,BX
        0x000d 00013 (/home/dfc/src/add64.go:5) MOVQ    BX,"".~r2+24(FP)
        0x0012 00018 (/home/dfc/src/add64.go:5) RET     ,

But got on 5g

"".add64 t=1 size=24 value=0 args=0xc locals=0x0 leaf
        0x0000 00000 (/home/dfc/src/add64.go:3) TEXT    "".add64+0(SB),16,$-4-12
        0x0000 00000 (/home/dfc/src/add64.go:3) FUNCDATA        $2,gclocals·f90cfd099b5ec2b453c391fece9d42bb+0(SB)
        0x0000 00000 (/home/dfc/src/add64.go:3) FUNCDATA        $3,gclocals·3280bececceccd33cb74587feedb1f9f+0(SB)
        0x0000 00000 (/home/dfc/src/add64.go:3) MOVW    $0,R0
        0x0004 00004 (/home/dfc/src/add64.go:4) MOVW    "".a+0(FP),R0
        0x0008 00008 (/home/dfc/src/add64.go:4) MOVW    "".b+4(FP),R1
        0x000c 00012 (/home/dfc/src/add64.go:4) ADD     R1,R0
        0x0010 00016 (/home/dfc/src/add64.go:5) MOVW    R0,"".~r2+8(FP)
        0x0014 00020 (/home/dfc/src/add64.go:5) B       ,0(R14)

Note the unnecessary zeroing of R0, which is then overwritten by the load of 0(FP)

Please use labels and text to provide additional information.

linux/arm

% go version
go version devel +6b696a34e0af Sun Aug 03 15:14:59 2014 -0700 linux/arm
@rsc
Copy link
Contributor

rsc commented Sep 15, 2014

Comment 1:

Labels changed: added release-none, removed release-go1.4.

Status changed to Accepted.

@rsc rsc removed the arch-arm label Apr 10, 2015
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc changed the title cmd/5g: unnecessary zeroing of register cmd/compile: unnecessary zeroing of register on arm Jun 8, 2015
@navytux
Copy link
Contributor

navytux commented Dec 9, 2016

As of

go version devel +3a067cc812 Fri Dec 9 06:45:08 2016 +0000 linux/amd64

this issue is no longer present:

$ GOARCH=arm go tool compile -S q.go 
"".add64 t=1 size=20 args=0xc locals=0x0 leaf
        0x0000 00000 (q.go:3)   TEXT    "".add64(SB), $-4-12
        0x0000 00000 (q.go:3)   FUNCDATA        $0, gclocals·54241e171da8af6ae173d69da0236748(SB)
        0x0000 00000 (q.go:3)   FUNCDATA        $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
        0x0000 00000 (q.go:4)   MOVW    "".a(FP), R0
        0x0004 00004 (q.go:4)   MOVW    "".b+4(FP), R1
        0x0008 00008 (q.go:4)   ADD     R1, R0, R0
        0x000c 00012 (q.go:5)   MOVW    R0, "".~r2+8(FP)
        0x0010 00016 (q.go:5)   JMP     (R14)
...

on arm64 there is unnecessary (imho) more stack check but no zeroing:

$ GOARCH=arm64 go tool compile -S q.go 
"".add64 t=1 size=48 args=0x18 locals=0x0 leaf
        0x0000 00000 (q.go:3)   TEXT    "".add64(SB), $-8-24
        0x0000 00000 (q.go:3)   MOVD    16(g), R1
        0x0004 00004 (q.go:3)   MOVD    RSP, R2
        0x0008 00008 (q.go:3)   CMP     R1, R2
        0x000c 00012 (q.go:3)   BLS     36
        0x0010 00016 (q.go:3)   FUNCDATA        ZR, gclocals·54241e171da8af6ae173d69da0236748(SB)
        0x0010 00016 (q.go:3)   FUNCDATA        $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
        0x0010 00016 (q.go:4)   MOVD    "".a(FP), R0
        0x0014 00020 (q.go:4)   MOVD    "".b+8(FP), R1
        0x0018 00024 (q.go:4)   ADD     R1, R0, R0
        0x001c 00028 (q.go:5)   MOVD    R0, "".~r2+16(FP)
        0x0020 00032 (q.go:5)   RET     (R30)
        0x0024 00036 (q.go:5)   NOP
        0x0024 00036 (q.go:3)   PCDATA  $0, $-1
        0x0024 00036 (q.go:3)   MOVD    R30, R3
        0x0028 00040 (q.go:3)   CALL    runtime.morestack_noctxt(SB)
        0x002c 00044 (q.go:3)   JMP     0

@davecheney
Copy link
Contributor Author

Thanks for confirming. Closing.

@davecheney davecheney modified the milestones: Go1.8, Unplanned Dec 9, 2016
@randall77 randall77 mentioned this issue Dec 9, 2016
Closed
@golang golang locked and limited conversation to collaborators Dec 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.8
Development

No branches or pull requests
4 participants
@davecheney @rsc @navytux @gopherbot