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

cmd/compile: remove stack check for leaf routines #18269

Closed
randall77 opened this issue Dec 9, 2016 · 5 comments
Closed

cmd/compile: remove stack check for leaf routines #18269

randall77 opened this issue Dec 9, 2016 · 5 comments
Labels
FrozenDueToAge Performance
Milestone
Unplanned

Comments

@randall77
Copy link
Contributor 

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

There's no need for a stack check for this function.
386 and arm64 emit a stack check anyway. Those should be eliminated.

See #8474
@randall77 randall77 added this to the Go1.9 milestone Dec 9, 2016
@mundaym
Copy link
Member

mundaym commented Dec 9, 2016

#13379 (automatic nosplit) is related to this. Are you sure 386 is affected? I thought cmd/internal/obj/x86 automatically marked functions with small stack frames as nosplit.

@mdempsky
Copy link
Member

mdempsky commented Dec 9, 2016

386 looks affected to me:

$ cat add64.go
package p

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

$ GOARCH=386 go tool compile -S add64.go | grep add64.go:
	0x0000 00000 (add64.go:3)	TEXT	"".Add64(SB), $0-12
	0x0000 00000 (add64.go:3)	MOVL	TLS, CX
	0x0007 00007 (add64.go:3)	MOVL	(CX)(TLS*2), CX
	0x000d 00013 (add64.go:3)	CMPL	SP, 8(CX)
	0x0010 00016 (add64.go:3)	JLS	33
	0x0012 00018 (add64.go:3)	FUNCDATA	$0, gclocals·54241e171da8af6ae173d69da0236748(SB)
	0x0012 00018 (add64.go:3)	FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x0012 00018 (add64.go:4)	MOVL	"".b+8(FP), AX
	0x0016 00022 (add64.go:4)	MOVL	"".a+4(FP), CX
	0x001a 00026 (add64.go:4)	ADDL	CX, AX
	0x001c 00028 (add64.go:5)	MOVL	AX, "".~r2+12(FP)
	0x0020 00032 (add64.go:5)	RET
	0x0021 00033 (add64.go:5)	NOP
	0x0021 00033 (add64.go:3)	PCDATA	$0, $-1
	0x0021 00033 (add64.go:3)	CALL	runtime.morestack_noctxt(SB)
	0x0026 00038 (add64.go:3)	JMP	0

$ GOARCH=amd64 go tool compile -S add64.go | grep add64.go:
	0x0000 00000 (add64.go:3)	TEXT	"".Add64(SB), $0-24
	0x0000 00000 (add64.go:3)	FUNCDATA	$0, gclocals·54241e171da8af6ae173d69da0236748(SB)
	0x0000 00000 (add64.go:3)	FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x0000 00000 (add64.go:4)	MOVQ	"".b+16(FP), AX
	0x0005 00005 (add64.go:4)	MOVQ	"".a+8(FP), CX
	0x000a 00010 (add64.go:4)	ADDQ	CX, AX
	0x000d 00013 (add64.go:5)	MOVQ	AX, "".~r2+24(FP)
	0x0012 00018 (add64.go:5)	RET

@randall77
Copy link
Contributor Author

This issue is definitely related to #13379, not sure if it is a dup or not.

@mundaym
Copy link
Member

mundaym commented Dec 9, 2016 •
edited

Ah yes, the relevant x86 code only automatically applies nosplit when the pointer size is 64-bits.

// TODO(rsc): Remove 'p.Mode == 64 &&'.
if p.Mode == 64 && autoffset < obj.StackSmall && p.From3Offset()&obj.NOSPLIT == 0 {

go/src/cmd/internal/obj/x86/obj6.go

Line 673 in 3a067cc

if p.Mode == 64 && autoffset < obj.StackSmall && p.From3Offset()&obj.NOSPLIT == 0 {

@mundaym mundaym mentioned this issue May 29, 2017
Closed
@randall77 randall77 modified the milestones: Go1.10, Go1.9 May 31, 2017
@bradfitz bradfitz modified the milestones: Go1.10, Go1.11 Nov 28, 2017
@bradfitz bradfitz modified the milestones: Go1.11, Unplanned May 18, 2018
@randall77
Copy link
Contributor Author

I'm going to close this issue. We do remove the stack check for leaf routines on amd64.
Let's use #13379 for implementing this on other architectures (including 386).

@golang golang locked and limited conversation to collaborators Apr 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Performance
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@bradfitz @mdempsky @randall77 @gopherbot @mundaym