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/compile: inlined closure remnants (unnecessarily) set Addrtaken in local variables #65158

Open
dr2chase opened this issue Jan 18, 2024 · 0 comments
Assignees
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@dr2chase
Copy link
Contributor

Go version

go version devel go1.22-66d34c7d08

Output of go env in your module/workspace:

does not depend on platform, only on `GOEXPERIMENT=rangefunc`

What did you do?

Go play link: https://go.dev/play/p/9faujPo3akR?v=gotip
I actually compiled this code GOSSAFUNC=main GOEXPERIMENT=rangefunc go run main.go and then examined ssa.html in a browser.

What did you see happen?

The two interesting places are the genssa column of ssa.html (the rightmost) and the AST column (2nd from left).

in genssa there are references to main.#exit1 and main.i as a stack variable

 v76	00023 (+18) MOVB $0, main.#exit1-137(SP)
v126	00024 (?) NOP
v157	00025 (+21) XORL AX, AX
  b4	00026 (9) JMP 29
v151	00027 (+22) ADDQ CX, main.i-128(SP)
v155	00028 (+9) INCQ AX
v165	00029 (+9) CMPQ AX, $14
  b5	00030 (9) JGE 42
v138	00031 (9) MOVQ main..autotmp_13-112(SP)(AX*8), CX
v139	00032 (+10) XCHGL AX, AX
v119	00033 (+21) CMPB main.#exit1-137(SP), $0
  b6	00034 (21) JEQ 27
v136	00035 (9) MOVQ AX, main..autotmp_26-120(SP)
v116	00036 (9) MOVQ CX, main.x-136(SP)
v143	00037 (21) PCDATA $1, $0
v143	00038 (21) CALL runtime.panicrangeexit(SB)

The cause for this can be seen in the AST column, where both i and #exit1 are marked as Addrtaken, because once-upon-a-time they were locals modified in a closure, but that closure has been inlined and is called nowhere. I understand it cannot quite be completely dead code eliminated because of mentions of the name in debugging information.

. DCL # main.go:20:2
. . NAME-main.i esc(no) Class:PAUTO Offset:0 Addrtaken OnStack Used int tc(1) # main.go:20:2
. AS Def tc(1) # main.go:20:4
. . NAME-main.i esc(no) Class:PAUTO Offset:0 Addrtaken OnStack Used int tc(1) # main.go:20:2
. . LITERAL-0 int tc(1) # main.go:20:7
. DCL
. . NAME-main.#exit1 esc(no) Class:PAUTO Offset:0 Addrtaken OnStack Used bool tc(1) # main.go:21:2
. AS tc(1)
. . NAME-main.#exit1 esc(no) Class:PAUTO Offset:0 Addrtaken OnStack Used bool tc(1) # main.go:21:2

I poked around the compiler a bit, and believe that the deed is done (Addrtaken set) in flowClosure in escape.go

What did you expect to see?

#exit1 and i not marked Addrtaken, allocated to registers, and (in the case of #exit1) completely removed from the code because it is statically evident that it is not needed.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 18, 2024
@dr2chase dr2chase added Performance and removed compiler/runtime Issues related to the Go compiler and/or runtime. labels Jan 18, 2024
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 19, 2024
@cagedmantis cagedmantis added this to the Backlog milestone Jan 19, 2024
@mdempsky mdempsky self-assigned this Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Development

No branches or pull requests

4 participants