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

Inlining and explicit tail calls #10487

Open
AndyAyersMS opened this issue Jun 10, 2018 · 2 comments
Open

Inlining and explicit tail calls #10487

AndyAyersMS opened this issue Jun 10, 2018 · 2 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI optimization
Milestone

Comments

@AndyAyersMS
Copy link
Member

AndyAyersMS commented Jun 10, 2018

We should re-examine the logic the jit uses for inlining in the presence of explicit tail calls (calls with a tail prefix). Currently the jit won't inline any method that is explicitly tail called

https://github.com/dotnet/coreclr/blob/7e215941c8d6a68fa52f98bb8fd2a419d8a1ef5d/src/jit/importer.cpp#L18943-L18949

or any method that makes an explicit tail call:

https://github.com/dotnet/coreclr/blob/7e215941c8d6a68fa52f98bb8fd2a419d8a1ef5d/src/jit/flowgraph.cpp#L5434-L5442

Both of these should be allowed provided with reasonable checks (former: callee frame is not overly huge; latter: call site is in tail position).

category:cq
theme:tail-call
skill-level:expert
cost:medium
impact:medium

@AndyAyersMS
Copy link
Member Author

Probably also: if the jit is going to emit a slow tail call (or fail to tail call), consider allowing inlining?

  • not clear how many slow tail call candidates will be good inline candidates.
  • not clear how easily we can anticipate failure or slowness early on.

@jakobbotsch
Copy link
Member

jakobbotsch commented Aug 12, 2019

former: callee frame is not overly huge; latter: call site is in tail position

Are these checks even required?

I think, from a programmer's perspective, the following is a reasonable assumption: for any sequence of explicit tail calls, the stack usage will be constant. The important case is when the sequence of explicit tail calls depends on an incoming data structure like a list, where this assumption is necessary to get correct behavior (no stack overflow).

Even inlining without these two checks should satisfy this assumption. For the former we are bloating the stack somewhat, but only in the beginning of the sequence: presumably the inlinee will also contain an explicit tail call, and if the inlinee is not inlined in tail-call position, this will turn into a regular call. However, once this regular call begins the future call sequence will use only constant amount of stack. So the stack usage is amortized constant in the input.

For the latter case we have the exact same situation. The explicit tail call may turn into a regular call bloating the stack in the beginning of the sequence. But we still only end up with amortized constant stack usage.

Now that is not say that it is not beneficial to use these decisions in the profitability analysis. But I think we already get them basically for free, since a callee with a large frame would (usually?) have large IL.

Probably also: if the jit is going to emit a slow tail call (or fail to tail call), consider allowing inlining?

  • not clear how many slow tail call candidates will be good inline candidates.
  • not clear how easily we can anticipate failure or slowness early on.

I think these are the more important pieces of information. Inlining a callee in tail call position should not be done if it means we end up requiring helper where we wouldn't before. However, I think due to the above points we can basically ignore any explicit tail prefix in inlinees during inlining, in which case we will never end up with helper anyway. What do you think?

EDIT: I have just realized I am wrong that we can ignore tail prefixes in inlinees – we have to honor them if they are inlined in explicit tail call position, since the inliner could participate in the call sequence. That makes things harder.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@BruceForstall BruceForstall removed the JitUntriaged CLR JIT issues needing additional triage label Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI optimization
Projects
None yet
Development

No branches or pull requests

4 participants