GeezerVison

Thank you for porting!

Before this goes in we should do a round of asm diffs.

Thanks for the review @jashook. I still have this as WIP as I believe there are some things left to do, eg. you have pointed some of them out in the reviews, and the runtime lookup done in morph needs to be investigated carefully. I don't really have the expertise to sign off on that change as it stands.

Does the design document https://github.com/dotnet/runtime/blob/master/docs/design/features/tailcalls-with-helpers.md need to be updated to match this implementation? (Either with this change, or after it is merged?)

@BruceForstall Yes, I have an updated document I will add soon.

@jakobbotsch I'd like to help get this PR to a point we can merge it. You mentioned in a few comments above that you were going to do some experiments and make some changes. Do you have anything in progress? Can you add an updated document? Let me know what you think is the best way to get this ready for a merge. We don't want this work to bit-rot. I have cycles to help.

@erozenfeld Sounds great. Unfortunately I have not had time to experiment further with this.
Also, as you can see, there is some unaddressed feedback from the team.

I have just added the updated design doc so you can take a look at that for a description of how it works. Also, I am happy to answer any questions you might have.

Let me know what you think is the best way to get this ready for a merge.

Essentially the missing part is dealing with tailcalls to generic functions which require generic contexts (or maybe even other nonstandard arguments). My idea was to pass a callable function pointer to the IL stubs. Such a function pointer can be obtained by doing something like ldftn or ldvirtftn, which will create instantiating stubs when a generic context parameter is required.
The problem is that we are in the JIT, more specifically in morph when we figure out that such a callable function pointer will be needed. At this point it is hard to create the complex trees required for runtime lookups which the JIT normally does directly during import (of eg. ldvirtftn).

My idea was to repurpose GT_RUNTIMELOOKUP nodes to hold information about runtime lookups. Then, change import and tailcall morph to generate such nodes when needed, eg. for ldvirtftn or for tailcalls requiring the same. Finally, expand these nodes later in the JIT, eg. during rationalization or lowering. This might have some impact on CSE which is what I wanted to experiment with.

There is some more discussion in the original thread, starting at dotnet/coreclr#26418 (comment). Once again, let me know if you have any questions about the design.

@jakobbotsch Can you please rebase the changes? I plan to work on getting us closer to merge this week.

@erozenfeld I addressed some feedback and resolved the conflicts. I merged instead of a rebase to make it a bit easier to check the merge resolutions, we can squash it later.

Congratulations!

Many, many thanks to @jakobbotsch for the implementation (and design updating) work, @janvorli for the original design, @jashook for mentoring, and @erozenfeld for doing the hard work to get it over the line. It's really great to see this get in!

Also thanks to @CarolEidt , @AndyAyersMS and @jkotas for code reviews and feedback along the way!

I opened two follow-up issues: #35551 (Jit: move runtime lookup expansion to lower) and #35423 (Add support for R2R tail calls with helpers in crossgen/crossgen2).

Thanks to all for the help, and a special thanks to @erozenfeld for pushing this past the finishing line!

I also opened #35559 about moving the dispatcher to System.Private.CoreLib once we have function pointer support.

Awesome, I am very happy to see this going in!

@jakobbotsch If you still have slides from your presentation about this work, can you send them to me?

@erozenfeld Unfortunately I seem to have kept it on my Microsoft OneDrive which I do not have access to anymore.

@erozenfeld there is the video from that presentation available if that's sufficient for you.