Skip to content
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.

Sign up for GitHub

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

JIT: OSR jitstress fixes; enable struct promotion #65903

Merged
merged 2 commits into from
Feb 26, 2022
Merged

JIT: OSR jitstress fixes; enable struct promotion #65903

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87c865f
JIT: OSR jitstress fixes; enable struct promotion
AndyAyersMS Feb 23, 2022
2b244cc
Fix typo
AndyAyersMS Feb 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 67 additions & 28 deletions src/coreclr/jit/lclvars.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1829,13 +1829,6 @@ bool Compiler::StructPromotionHelper::CanPromoteStructVar(unsigned lclNum)
return false;
}

// TODO-CQ: enable promotion for OSR locals
if (compiler->lvaIsOSRLocal(lclNum))
{
JITDUMP(" struct promotion of V%02u is disabled because it is an OSR local\n", lclNum);
return false;
}

CORINFO_CLASS_HANDLE typeHnd = varDsc->GetStructHnd();
assert(typeHnd != NO_CLASS_HANDLE);

Expand Down Expand Up @@ -5934,6 +5927,11 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum,
for (unsigned i = 0; i < varDsc->lvFieldCnt; i++)
{
LclVarDsc* fieldVarDsc = lvaGetDesc(firstFieldNum + i);

JITDUMP("Adjusting offset of dependent V%02u of arg V%02u: parent %u field %u net %u\n", lclNum,
firstFieldNum + i, varDsc->GetStackOffset(), fieldVarDsc->lvFldOffset,
varDsc->GetStackOffset() + fieldVarDsc->lvFldOffset);

fieldVarDsc->SetStackOffset(varDsc->GetStackOffset() + fieldVarDsc->lvFldOffset);
}
}
Expand Down Expand Up @@ -6409,7 +6407,7 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals()
In other words, we will not calculate the "base" address of the struct local if
the promotion type is PROMOTION_TYPE_FIELD_DEPENDENT.
*/
if (!opts.IsOSR() && lvaIsFieldOfDependentlyPromotedStruct(varDsc))
if (lvaIsFieldOfDependentlyPromotedStruct(varDsc))
{
continue;
}
Expand All @@ -6436,21 +6434,34 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals()
// will refer to their memory homes.
if (lvaIsOSRLocal(lclNum))
{
// TODO-CQ: enable struct promotion for OSR locals; when that
// happens, figure out how to properly refer to the original
// frame slots for the promoted fields.
assert(!varDsc->lvIsStructField);
if (varDsc->lvIsStructField)
{
const unsigned parentLclNum = varDsc->lvParentLcl;
const int parentOriginalOffset = info.compPatchpointInfo->Offset(parentLclNum);
const int offset = originalFrameStkOffs + parentOriginalOffset + varDsc->lvFldOffset;

JITDUMP("---OSR--- V%02u (promoted field of V%02u; on tier0 frame) tier0 FP-rel offset %d tier0 "
"frame offset %d field offset %d new virt offset "
"%d\n",
lclNum, parentLclNum, parentOriginalOffset, originalFrameStkOffs, varDsc->lvFldOffset,
offset);

// Add frampointer-relative offset of this OSR live local in the original frame
// to the offset of original frame in our new frame.
int originalOffset = info.compPatchpointInfo->Offset(lclNum);
int offset = originalFrameStkOffs + originalOffset;
lvaTable[lclNum].SetStackOffset(offset);
}
else
{
// Add frampointer-relative offset of this OSR live local in the original frame
// to the offset of original frame in our new frame.
const int originalOffset = info.compPatchpointInfo->Offset(lclNum);
const int offset = originalFrameStkOffs + originalOffset;

JITDUMP("---OSR--- V%02u (on tier0 frame) tier0 FP-rel offset %d tier0 frame offset %d new virt offset "
JITDUMP(
"---OSR--- V%02u (on tier0 frame) tier0 FP-rel offset %d tier0 frame offset %d new virt offset "
"%d\n",
lclNum, originalOffset, originalFrameStkOffs, offset);

lvaTable[lclNum].SetStackOffset(offset);
lvaTable[lclNum].SetStackOffset(offset);
}
continue;
}

Expand Down Expand Up @@ -7091,18 +7102,25 @@ void Compiler::lvaAssignFrameOffsetsToPromotedStructs()
// This is not true for the System V systems since there is no
// outgoing args space. Assign the dependently promoted fields properly.
//
if (varDsc->lvIsStructField
#if !defined(UNIX_AMD64_ABI) && !defined(TARGET_ARM) && !defined(TARGET_X86)
// ARM: lo/hi parts of a promoted long arg need to be updated.
CLANG_FORMAT_COMMENT_ANCHOR;

// For System V platforms there is no outgoing args space.
#if defined(UNIX_AMD64_ABI) || defined(TARGET_ARM) || defined(TARGET_X86)
// ARM: lo/hi parts of a promoted long arg need to be updated.
//
// For System V platforms there is no outgoing args space.
//
// For System V and x86, a register passed struct arg is homed on the stack in a separate local var.
// The offset of these structs is already calculated in lvaAssignVirtualFrameOffsetToArg methos.
// Make sure the code below is not executed for these structs and the offset is not changed.
//
const bool mustProcessParams = true;
#else
// OSR must also assign offsets here.
//
const bool mustProcessParams = opts.IsOSR();
#endif // defined(UNIX_AMD64_ABI) || defined(TARGET_ARM) || defined(TARGET_X86)

// For System V and x86, a register passed struct arg is homed on the stack in a separate local var.
// The offset of these structs is already calculated in lvaAssignVirtualFrameOffsetToArg methos.
// Make sure the code below is not executed for these structs and the offset is not changed.
&& !varDsc->lvIsParam
#endif // !defined(UNIX_AMD64_ABI) && !defined(TARGET_ARM) && !defined(TARGET_X86)
)
if (varDsc->lvIsStructField && (!varDsc->lvIsParam || mustProcessParams))
{
LclVarDsc* parentvarDsc = lvaGetDesc(varDsc->lvParentLcl);
lvaPromotionType promotionType = lvaGetPromotionType(parentvarDsc);
Expand All @@ -7119,6 +7137,9 @@ void Compiler::lvaAssignFrameOffsetsToPromotedStructs()
noway_assert(varDsc->lvOnFrame);
if (parentvarDsc->lvOnFrame)
{
JITDUMP("Adjusting offset of dependent V%02u of V%02u: parent %u field %u net %u\n", lclNum,
varDsc->lvParentLcl, parentvarDsc->GetStackOffset(), varDsc->lvFldOffset,
parentvarDsc->GetStackOffset() + varDsc->lvFldOffset);
varDsc->SetStackOffset(parentvarDsc->GetStackOffset() + varDsc->lvFldOffset);
}
else
Expand Down Expand Up @@ -7922,6 +7943,24 @@ Compiler::fgWalkResult Compiler::lvaStressLclFldCB(GenTree** pTree, fgWalkData*
return WALK_SKIP_SUBTREES;
}

// Ignore OSR locals; if in memory, they will live on the
// Tier0 frame and so can't have their storage adjusted.
//
if (pComp->lvaIsOSRLocal(lclNum))
{
varDsc->lvNoLclFldStress = true;
return WALK_SKIP_SUBTREES;
}

// Likewise for Tier0 methods with patchpoints --
// if we modify them we'll misreport their locations in the patchpoint info.
//
if (pComp->doesMethodHavePatchpoints() || pComp->doesMethodHavePartialCompilationPatchpoints())
{
varDsc->lvNoLclFldStress = true;
return WALK_SKIP_SUBTREES;
}

// Fix for lcl_fld stress mode
if (varDsc->lvKeepType)
{
Expand Down