-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fix fgCanTailCallViaJitHelper #70033
Fix fgCanTailCallViaJitHelper #70033
Conversation
FEATURE_READYTORUN is always defined except for single-file JIT. The right check here is Compiler::Options::IsReadyToRun(). Fix dotnet#70013
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsFEATURE_READYTORUN is always defined except for single-file JIT. The Fix #70013
|
@@ -17526,10 +17526,15 @@ bool Compiler::fgCheckStmtAfterTailCall() | |||
// | |||
bool Compiler::fgCanTailCallViaJitHelper() | |||
{ | |||
#if !defined(TARGET_X86) || defined(UNIX_X86_ABI) || defined(FEATURE_READYTORUN) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did it basically turn helper-based tailcalls off for jit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If so I guess we need more (if we have any) microbenchmarks written in F#
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I thought I responded but can't see my comment..?)
did it basically turn helper-based tailcalls off for jit?
No, it just made x86 use the same mechanism we use on all other platforms.
If so I guess we need more (if we have any) microbenchmarks written in F#
That would be great, and yes, this would probably have shown up as a noticeable perf regression.
Looks like this bit rotted slightly, probably because of #67238. |
/azp run runtime-coreclr jitstress |
Azure Pipelines successfully started running 1 pipeline(s). |
FEATURE_READYTORUN is always defined except for single-file JIT. The
right check here is
Compiler::Options::IsReadyToRun()
.Fix #70013