Skip to content

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented Sep 3, 2021

Currently we always promote methods with explicit tails to tier1 (and enable opts even in Debug?) so we never instrument them...

From my understanding, QJFL=1 (which is 0 by default) ideally implies a working OSR so we should not hit potential SO

/cc @AndyAyersMS @jakobbotsch

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 3, 2021
@ghost
Copy link

ghost commented Sep 3, 2021

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

I'm not fully sure in this change, but looks like currently we always promote methods with explicit tails to tier1 (and enable opts even in Debug?) so we never instrument them...

From my understanding, QJFL=1 (which is 0 by default) ideally implies a working OSR so we should not hit SO here

/cc @AndyAyersMS @jakobbotsch

Author: EgorBo
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch
Copy link
Member

jakobbotsch commented Sep 3, 2021

From my understanding, QJFL=1 (which is 0 by default) ideally implies a working OSR so we should not hit potential SO

I think the comment is just outdated, we will never hit potential SO anymore since tailcall helpers work in tier-0 as well.
I think it should be ok to always start them out in tier 0 when QJFL=1, but guarding on instrumentation as well seems ok.

EDIT: It should even be ok to always start them out in tier-0 now since we don't optimize recursion to loops in tier0 anyway.

// call in tier 0, switch to optimized to avoid spending too much time running slower code and
// to avoid stack overflow from recursion
// call in tier 0, switch to optimized to avoid spending too much time running slower code
fgSwitchToOptimized();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought it might be a good idea to go back to what you had before -- jitting these requires creating up to two IL stubs for each explicit tailcall in the function, and those stubs are not currently shared, so the rejit will always recreate them.

@JulieLeeMSFT JulieLeeMSFT added this to the 7.0.0 milestone Sep 3, 2021
@EgorBo EgorBo merged commit 32df679 into dotnet:main Sep 6, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Oct 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants