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: inline functions that are called only once #34525

Open
marigonzes opened this issue Sep 25, 2019 · 6 comments

Comments

@marigonzes
Copy link

commented Sep 25, 2019

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

$ go version
go version devel +211932b Mon Sep 23 22:33:23 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

Take the following functions as an example:

func f(n int) int {
    var sum int
    for i := 0; i < n; i++ {
        sum += aux(i)
    }
    return sum
}

func aux(x int) (res int) {
    // Long function that goes over the inline budget
    // ...

    return res
}

If it is known that function aux is only called once (in this case, in f), in spite of a going over the inline budget, I think it would be beneficial to inline it in the caller.
I imagine there are many situations where developers extract a piece of code from a loop into a new function and then only call it once.

@mvdan

This comment has been minimized.

Copy link
Member

commented Sep 25, 2019

Would this be about having a higher budget, or about ignoring the budget entirely?

@marigonzes

This comment has been minimized.

Copy link
Author

commented Sep 25, 2019

I think it should ignore the budget entirely or, in case this is not possible, raise it substantially for cases like this. Given that the functions in question are only called once, I don't see it becoming a problem (but I could be wrong).

@martisch

This comment has been minimized.

Copy link
Member

commented Sep 25, 2019

Do you have some concrete examples where this is useful?

I assume called once does mean only referenced once with a call? Whether a function is called once might only be found out at runtime.

There are possible bad interactions of always inlining that come to my mind:

  • it effects the midstack inline budget of inlined too function
  • its a function with a large codebase and slow path that was explicitly split out by the programmer to separate cold from hot code
  • increases register pressure and more work for the compiler to figure out what needed when
@marigonzes

This comment has been minimized.

Copy link
Author

commented Sep 25, 2019

This came to my mind when thinking about loops that call non-inlineable functions (when these functions are only used once). Normally, in these kinds of loops, it would be beneficial to not pay the cost of the function call. I don't have a concrete example apart from the constructed functions above.

I'm not at all into how the insides of Go work, but this was an idea that I came up with.
Leaving up to you to decide if this is feasible and if it helps at all 😉

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Sep 27, 2019

Inlining a function can increase cost, by increasing register pressure and increasing cache line usage. For example, it would be a mistake to inline a large function called once if the function is rarely called in practice. In fact, optimizing compilers often support outlining, in which a frequently executed part of a function is inlined but the rest of the function is left behind.

That said, I agree that if a function is only called once, that should be a signal to consider it more seriously for inlining.

@CAFxX

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2019

If a function had a single caller, and the callsite is always (or almost always) executed, then arguably it should be unconditionally inlined regardless of size (as register pressure, stack usage and cache line usage should not increase).

OTOH if the callsite is unlikely to be executed, it may make sense to still inline non-huge functions, and ensure the cold basic blocks are reordered so that they don't pollute the icache. If stack growth is a concern in this case, maybe a second morestack check could be maintained guarding the inlined code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.