Skip to content

Commit

Permalink
JIT: import entire method for OSR, prune unneeded parts later
Browse files Browse the repository at this point in the history
For OSR compiles, always import from the original entry point in addtion
to the OSR entry point. This gives the OSR compiler a chance
to see all of the method and so properly compute address exposure,
instead of relying on the Tier0
analysis.

Once address exposure has been determined, revoke special protection
for the original entry and try and prune away blocks that are no longer
needed.

Fixes dotnet#83783.

May also fix some of the cases where OSR perf is lagging (though
don't expect this to fix them all).
  • Loading branch information
AndyAyersMS committed Mar 24, 2023
1 parent 8a7a91c commit b25148e
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 14 deletions.
10 changes: 10 additions & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4721,6 +4721,16 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
//
DoPhase(this, PHASE_STR_ADRLCL, &Compiler::fgMarkAddressExposedLocals);

// For OSR, we no longer need to keep the parts of the method
// that may not ever be reached.
//
if (opts.IsOSR())
{
assert(fgEntryBB->bbFlags & BBF_DONT_REMOVE);
fgEntryBB->bbFlags &= ~BBF_DONT_REMOVE;
DoPhase(this, PHASE_EARLY_UPDATE_FLOW_GRAPH, &Compiler::fgUpdateFlowGraphPhase);
}

// Do an early pass of liveness for forward sub and morph. This data is
// valid until after morph.
//
Expand Down
9 changes: 9 additions & 0 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12624,6 +12624,15 @@ void Compiler::impImport()
// which we should verify over when we find jump targets.
impImportBlockPending(entryBlock);

if (opts.IsOSR())
{
// We now import all the IR and keep it around so we can
// analyze address exposure more robustly.
//
impImportBlockPending(fgEntryBB);
fgEntryBB->bbFlags |= BBF_DONT_REMOVE;
}

/* Import blocks in the worker-list until there are no more */

while (impPendingList)
Expand Down
15 changes: 1 addition & 14 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ void Compiler::lvaInitTypeRef()
}
}

// If this is an OSR method, mark all the OSR locals and model OSR exposure.
// If this is an OSR method, mark all the OSR locals.
//
// Do this before we add the GS Cookie Dummy or Outgoing args to the locals
// so we don't have to do special checks to exclude them.
Expand All @@ -334,19 +334,6 @@ void Compiler::lvaInitTypeRef()
{
LclVarDsc* const varDsc = lvaGetDesc(lclNum);
varDsc->lvIsOSRLocal = true;

if (info.compPatchpointInfo->IsExposed(lclNum))
{
JITDUMP("-- V%02u is OSR exposed\n", lclNum);
varDsc->lvHasLdAddrOp = 1;

// todo: Why does it apply only to non-structs?
//
if (!varTypeIsStruct(varDsc) && !varTypeIsSIMD(varDsc))
{
lvaSetVarAddrExposed(lclNum DEBUGARG(AddressExposedReason::OSR_EXPOSED));
}
}
}
}

Expand Down

0 comments on commit b25148e

Please sign in to comment.