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

cmd/gc: seems expand stack uneccessary #40069

Closed
zylthinking opened this issue Jul 6, 2020 · 2 comments
Closed

cmd/gc: seems expand stack uneccessary #40069

zylthinking opened this issue Jul 6, 2020 · 2 comments

Comments

@zylthinking
Copy link

@zylthinking zylthinking commented Jul 6, 2020

What version of Go are you using (go version)?

1.14

Does this issue reproduce with the latest release?

Yes

The problem:

run go tool compile -S a.go
the function

func a() uintptr {
    var x [1024]uintptr
    for i := 0; i < len(x); i++ {
        x[i] = uintptr(time.Now().Unix())
    }   
    return x[0]
}

output as:

"".a STEXT size=239 args=0x8 locals=0x2028
	0x0000 00000 (a.go:6)	TEXT	"".a(SB), ABIInternal, $8232-8
	0x0000 00000 (a.go:6)	MOVQ	(TLS), CX
	0x0009 00009 (a.go:6)	MOVQ	16(CX), SI
	0x000d 00013 (a.go:6)	PCDATA	$0, $-2
	0x000d 00013 (a.go:6)	CMPQ	SI, $-1314
	0x0014 00020 (a.go:6)	JEQ	229
	0x001a 00026 (a.go:6)	LEAQ	896(SP), AX
	0x0022 00034 (a.go:6)	SUBQ	SI, AX
	0x0025 00037 (a.go:6)	CMPQ	AX, $9000
	0x002b 00043 (a.go:6)	JLS	229
	0x0031 00049 (a.go:6)	PCDATA	$0, $-1
	0x0031 00049 (a.go:6)	SUBQ	$8232, SP
	0x0038 00056 (a.go:6)	MOVQ	BP, 8224(SP)
	0x0040 00064 (a.go:6)	LEAQ	8224(SP), BP

I don't know why 768 more bytes is needed in the following line:
0x0025 00037 (a.go:6) CMPQ AX, $9000

Should't it be CMPQ AX, $8232?
In some cases when there less than 9000 bytes but more than 8232, stack expand is triggered unecessarily

@zylthinking zylthinking changed the title cmd/gc: seems have a wasting of stack cmd/gc: seems expand stack uneccessary Jul 6, 2020
@martisch
Copy link
Contributor

@martisch martisch commented Jul 6, 2020

8232 bytes for the stack wont suffice to reserve enough additional stack space for the called functions of a because later the functiona calls time.Now(SB) which itself will need stack space ontop of the used 8232. time.Now then calls other functions e.g. time.local ... . And this continues recursively. Reserving just 8232 would predictably trigger another stack growth in the other leave functions.

Unlike many projects, the Go project does not use GitHub Issues for general discussion or asking questions.
https://github.com/golang/go/wiki/Questions

@martisch
Copy link
Contributor

@martisch martisch commented Jul 6, 2020

Feel free to reopen the issue if you think there is a bug here and the above explanation why this is more than 8232 is wrong.

@martisch martisch closed this Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.