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: replace indirect call with direct call when target is known after inlining #32577

Closed
mariecurried opened this issue Jun 12, 2019 · 3 comments
Closed

cmd/compile: replace indirect call with direct call when target is known after inlining #32577

mariecurried opened this issue Jun 12, 2019 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Unplanned

Comments

@mariecurried
Copy link

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

$ go version
go version devel +ef84fa0 Mon Jun 10 23:23:39 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

I compiled the following functions: https://godbolt.org/z/dHW0pK.

func f(x int) int {
    return x
}

func getf() func (int) int {
    return f
}

func applyf() int {
    return getf()(2)
}

What did you expect to see?

I expected the last function not to contain any unnecessary instructions or, in a perfect world, that it would be compiled to a simple return.

What did you see instead?

Instead, there was an ineffective instruction on line 37 of the assembly code generated in the website mentioned above.

leaq    "".f·f(SB), DX

PS: Thanks to @mundaym for helping me figure this out.
@randall77
Copy link
Contributor

That instruction loads a pointer to the closure into a register for the call.
That is required for the calling convention.
It's not necessary in this case because the closure doesn't close over any variables. But if we know that it doesn't close over any variables, we likely know the exact target and we might as well replace the indirect call with a direct call (which could then inline, which would then match your perfect world).

@randall77 randall77 added this to the Unplanned milestone Jun 12, 2019
@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 12, 2019
@mundaym mundaym changed the title cmd/compile: ineffective store due to closure cmd/compile: replace indirect call with direct call when target is known Jun 12, 2019
@mundaym mundaym changed the title cmd/compile: replace indirect call with direct call when target is known cmd/compile: replace indirect call with direct call when target is known after inlining Jun 12, 2019
@mundaym
Copy link
Member

mundaym commented Jun 12, 2019

I think it would be fairly straightforward to convert a ClosureCall into a StaticCall in SSA when the target is known (perhaps taking a similar approach to the InterCall optimization @philhofer added in CL 38139). As far as I know going from a direct call to an inlined function isn't currently possible in the SSA backend though - and inlining is probably more valuable than a ClosureCall to StaticCall optimization in this scenario.

@cespare cespare mentioned this issue Nov 20, 2020
Open
@mariecurried
Copy link
Author

No longer reproduces

@golang golang locked and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@agnivade @randall77 @gopherbot @mundaym @mariecurried