Skip to content

Commit c8da2fd

Browse files
authored
JIT: OSR jitstress fixes; enable struct promotion (#65903)
Fix OSR jitstress failures by disabling STRESS_LCL_FLD for two cases: * for Tier0 methods with patchpoints (because patchpoint info generation does not produce the right offsets when locals are padded) * for OSR locals in OSR methods (because they live on the Tier0 frame and so can't have their storage altered. Enable struct promotion for OSR locals. In most cases this ends up being dependent promotion. Enabling independent promotion will take more work.
1 parent a3f7d29 commit c8da2fd

File tree

1 file changed

+67
-28
lines changed

1 file changed

+67
-28
lines changed

src/coreclr/jit/lclvars.cpp

Lines changed: 67 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1829,13 +1829,6 @@ bool Compiler::StructPromotionHelper::CanPromoteStructVar(unsigned lclNum)
18291829
return false;
18301830
}
18311831

1832-
// TODO-CQ: enable promotion for OSR locals
1833-
if (compiler->lvaIsOSRLocal(lclNum))
1834-
{
1835-
JITDUMP(" struct promotion of V%02u is disabled because it is an OSR local\n", lclNum);
1836-
return false;
1837-
}
1838-
18391832
CORINFO_CLASS_HANDLE typeHnd = varDsc->GetStructHnd();
18401833
assert(typeHnd != NO_CLASS_HANDLE);
18411834

@@ -5934,6 +5927,11 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum,
59345927
for (unsigned i = 0; i < varDsc->lvFieldCnt; i++)
59355928
{
59365929
LclVarDsc* fieldVarDsc = lvaGetDesc(firstFieldNum + i);
5930+
5931+
JITDUMP("Adjusting offset of dependent V%02u of arg V%02u: parent %u field %u net %u\n", lclNum,
5932+
firstFieldNum + i, varDsc->GetStackOffset(), fieldVarDsc->lvFldOffset,
5933+
varDsc->GetStackOffset() + fieldVarDsc->lvFldOffset);
5934+
59375935
fieldVarDsc->SetStackOffset(varDsc->GetStackOffset() + fieldVarDsc->lvFldOffset);
59385936
}
59395937
}
@@ -6409,7 +6407,7 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals()
64096407
In other words, we will not calculate the "base" address of the struct local if
64106408
the promotion type is PROMOTION_TYPE_FIELD_DEPENDENT.
64116409
*/
6412-
if (!opts.IsOSR() && lvaIsFieldOfDependentlyPromotedStruct(varDsc))
6410+
if (lvaIsFieldOfDependentlyPromotedStruct(varDsc))
64136411
{
64146412
continue;
64156413
}
@@ -6436,21 +6434,34 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals()
64366434
// will refer to their memory homes.
64376435
if (lvaIsOSRLocal(lclNum))
64386436
{
6439-
// TODO-CQ: enable struct promotion for OSR locals; when that
6440-
// happens, figure out how to properly refer to the original
6441-
// frame slots for the promoted fields.
6442-
assert(!varDsc->lvIsStructField);
6437+
if (varDsc->lvIsStructField)
6438+
{
6439+
const unsigned parentLclNum = varDsc->lvParentLcl;
6440+
const int parentOriginalOffset = info.compPatchpointInfo->Offset(parentLclNum);
6441+
const int offset = originalFrameStkOffs + parentOriginalOffset + varDsc->lvFldOffset;
6442+
6443+
JITDUMP("---OSR--- V%02u (promoted field of V%02u; on tier0 frame) tier0 FP-rel offset %d tier0 "
6444+
"frame offset %d field offset %d new virt offset "
6445+
"%d\n",
6446+
lclNum, parentLclNum, parentOriginalOffset, originalFrameStkOffs, varDsc->lvFldOffset,
6447+
offset);
64436448

6444-
// Add frampointer-relative offset of this OSR live local in the original frame
6445-
// to the offset of original frame in our new frame.
6446-
int originalOffset = info.compPatchpointInfo->Offset(lclNum);
6447-
int offset = originalFrameStkOffs + originalOffset;
6449+
lvaTable[lclNum].SetStackOffset(offset);
6450+
}
6451+
else
6452+
{
6453+
// Add frampointer-relative offset of this OSR live local in the original frame
6454+
// to the offset of original frame in our new frame.
6455+
const int originalOffset = info.compPatchpointInfo->Offset(lclNum);
6456+
const int offset = originalFrameStkOffs + originalOffset;
64486457

6449-
JITDUMP("---OSR--- V%02u (on tier0 frame) tier0 FP-rel offset %d tier0 frame offset %d new virt offset "
6458+
JITDUMP(
6459+
"---OSR--- V%02u (on tier0 frame) tier0 FP-rel offset %d tier0 frame offset %d new virt offset "
64506460
"%d\n",
64516461
lclNum, originalOffset, originalFrameStkOffs, offset);
64526462

6453-
lvaTable[lclNum].SetStackOffset(offset);
6463+
lvaTable[lclNum].SetStackOffset(offset);
6464+
}
64546465
continue;
64556466
}
64566467

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

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

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

7946+
// Ignore OSR locals; if in memory, they will live on the
7947+
// Tier0 frame and so can't have their storage adjusted.
7948+
//
7949+
if (pComp->lvaIsOSRLocal(lclNum))
7950+
{
7951+
varDsc->lvNoLclFldStress = true;
7952+
return WALK_SKIP_SUBTREES;
7953+
}
7954+
7955+
// Likewise for Tier0 methods with patchpoints --
7956+
// if we modify them we'll misreport their locations in the patchpoint info.
7957+
//
7958+
if (pComp->doesMethodHavePatchpoints() || pComp->doesMethodHavePartialCompilationPatchpoints())
7959+
{
7960+
varDsc->lvNoLclFldStress = true;
7961+
return WALK_SKIP_SUBTREES;
7962+
}
7963+
79257964
// Fix for lcl_fld stress mode
79267965
if (varDsc->lvKeepType)
79277966
{

0 commit comments

Comments
 (0)