Skip to content

Commit

Permalink
JIT: fix osr gc info this reporting
Browse files Browse the repository at this point in the history
With the advent of dotnet#38229 an optimized method may need to report generics
context via `this` while the un-optimzed version did not need to report.

This impacts OSR, which previously was always trying to use the unoptimized
root method frame reporting. Now under OSR we must sometimes add a slot to
the OSR frame instead.

Addresses one of the failure cases in dotnet#43534.
  • Loading branch information
AndyAyersMS committed Apr 10, 2021
1 parent f4df53b commit 829ca65
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 21 deletions.
19 changes: 15 additions & 4 deletions src/coreclr/jit/gcencode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3931,13 +3931,24 @@ void GCInfo::gcInfoBlockHdrSave(GcInfoEncoder* gcInfoEncoder, unsigned methodSiz

int offset = 0;

// OSR can report the root method's frame slot, if that method reported context.
//
bool isOsrAndUsingRootFrameSlot = false;
if (compiler->opts.IsOSR())
{
PatchpointInfo* ppInfo = compiler->info.compPatchpointInfo;
offset = ppInfo->GenericContextArgOffset();
assert(offset != -1);
PatchpointInfo* const ppInfo = compiler->info.compPatchpointInfo;

if (ppInfo->HasKeptAliveThis())
{
offset = ppInfo->GenericContextArgOffset();
assert(offset != -1);
isOsrAndUsingRootFrameSlot = true;
}
}
else

// If not OSR, or OSR but newly reporting context, use the current frame offset.
//
if (!isOsrAndUsingRootFrameSlot)
{
offset = compiler->lvaToCallerSPRelativeOffset(compiler->lvaCachedGenericContextArgOffset(),
compiler->isFramePointerUsed());
Expand Down
45 changes: 28 additions & 17 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6267,19 +6267,15 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals()
}
#endif // JIT32_GCENCODER

// OSR methods use the original method slot for the cached kept alive this,
// so don't need to allocate a slot on the new frame.
if (opts.IsOSR())
{
if (lvaKeepAliveAndReportThis())
{
PatchpointInfo* ppInfo = info.compPatchpointInfo;
assert(ppInfo->HasKeptAliveThis());
int originalOffset = ppInfo->KeptAliveThisOffset();
lvaCachedGenericContextArgOffs = originalFrameStkOffs + originalOffset;
}
}
else if (lvaReportParamTypeArg())
// For OSR methods, param type args are always reportable via the root method frame slot.
// (see gcInfoBlockHdrSave) and so do not need a new slot on the frame.
//
// OSR methods may also be able to use the root frame kept alive this, if the root
// method needed to report this.
//
// Inlining done under OSR may introduce new reporting, in which case the OSR frame
// must allocate a slot.
if (!opts.IsOSR() && lvaReportParamTypeArg())
{
#ifdef JIT32_GCENCODER
noway_assert(codeGen->isFramePointerUsed());
Expand All @@ -6292,10 +6288,25 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals()
#ifndef JIT32_GCENCODER
else if (lvaKeepAliveAndReportThis())
{
// When "this" is also used as generic context arg.
lvaIncrementFrameSize(TARGET_POINTER_SIZE);
stkOffs -= TARGET_POINTER_SIZE;
lvaCachedGenericContextArgOffs = stkOffs;
bool canUseExistingSlot = false;
if (opts.IsOSR())
{
PatchpointInfo* ppInfo = info.compPatchpointInfo;
if (ppInfo->HasKeptAliveThis())
{
int originalOffset = ppInfo->KeptAliveThisOffset();
lvaCachedGenericContextArgOffs = originalFrameStkOffs + originalOffset;
canUseExistingSlot = true;
}
}

if (!canUseExistingSlot)
{
// When "this" is also used as generic context arg.
lvaIncrementFrameSize(TARGET_POINTER_SIZE);
stkOffs -= TARGET_POINTER_SIZE;
lvaCachedGenericContextArgOffs = stkOffs;
}
}
#endif

Expand Down

0 comments on commit 829ca65

Please sign in to comment.