Skip to content

Commit

Permalink
Set lvSingleDef for non TYP_REF locals (#85398)
Browse files Browse the repository at this point in the history 
* Set lvSingleDef for non TYP_REF locals

* Apply suggestions from code review

Co-authored-by: Andy Ayers <[email protected]>

* Rewrite an if

---------

Co-authored-by: Andy Ayers <[email protected]>
  • Loading branch information
@MichalPetryka @AndyAyersMS
MichalPetryka and AndyAyersMS authored May 1, 2023
1 parent b8aeaad commit a193cb5
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 47 deletions.
34 changes: 17 additions & 17 deletions src/coreclr/jit/fgbasic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2536,7 +2536,6 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
{
LclVarDsc* lclDsc = lvaGetDesc(lclNum);
assert(lclDsc->lvSingleDef == 0);
// could restrict this to TYP_REF
lclDsc->lvSingleDef = !lclDsc->lvHasMultipleILStoreOp && !lclDsc->lvHasLdAddrOp;

if (lclDsc->lvSingleDef)
Expand Down Expand Up @@ -3494,19 +3493,20 @@ void Compiler::fgFindBasicBlocks()
// This temp should already have the type of the return value.
JITDUMP("\nInliner: re-using pre-existing spill temp V%02u\n", lvaInlineeReturnSpillTemp);

if (info.compRetType == TYP_REF)
// We may have co-opted an existing temp for the return spill.
// We likely assumed it was single-def at the time, but now
// we can see it has multiple definitions.
if ((fgReturnCount > 1) && (lvaTable[lvaInlineeReturnSpillTemp].lvSingleDef == 1))
{
// We may have co-opted an existing temp for the return spill.
// We likely assumed it was single-def at the time, but now
// we can see it has multiple definitions.
if ((fgReturnCount > 1) && (lvaTable[lvaInlineeReturnSpillTemp].lvSingleDef == 1))
// Make sure it is no longer marked single def. This is only safe
// to do if we haven't ever updated the type.
if (info.compRetType == TYP_REF)
{
// Make sure it is no longer marked single def. This is only safe
// to do if we haven't ever updated the type.
assert(!lvaTable[lvaInlineeReturnSpillTemp].lvClassInfoUpdated);
JITDUMP("Marked return spill temp V%02u as NOT single def temp\n", lvaInlineeReturnSpillTemp);
lvaTable[lvaInlineeReturnSpillTemp].lvSingleDef = 0;
}

JITDUMP("Marked return spill temp V%02u as NOT single def temp\n", lvaInlineeReturnSpillTemp);
lvaTable[lvaInlineeReturnSpillTemp].lvSingleDef = 0;
}
}
else
Expand All @@ -3515,18 +3515,18 @@ void Compiler::fgFindBasicBlocks()
lvaInlineeReturnSpillTemp = lvaGrabTemp(false DEBUGARG("Inline return value spill temp"));
lvaTable[lvaInlineeReturnSpillTemp].lvType = info.compRetType;

// The return spill temp is single def only if the method has a single return block.
if (fgReturnCount == 1)
{
lvaTable[lvaInlineeReturnSpillTemp].lvSingleDef = 1;
JITDUMP("Marked return spill temp V%02u as a single def temp\n", lvaInlineeReturnSpillTemp);
}

// If the method returns a ref class, set the class of the spill temp
// to the method's return value. We may update this later if it turns
// out we can prove the method returns a more specific type.
if (info.compRetType == TYP_REF)
{
// The return spill temp is single def only if the method has a single return block.
if (fgReturnCount == 1)
{
lvaTable[lvaInlineeReturnSpillTemp].lvSingleDef = 1;
JITDUMP("Marked return spill temp V%02u as a single def temp\n", lvaInlineeReturnSpillTemp);
}

CORINFO_CLASS_HANDLE retClassHnd = impInlineInfo->inlineCandidateInfo->methInfo.args.retTypeClass;
if (retClassHnd != nullptr)
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/fgopt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1453,7 +1453,7 @@ PhaseStatus Compiler::fgPostImportationCleanup()
{
LclVarDsc* returnSpillVarDsc = lvaGetDesc(lvaInlineeReturnSpillTemp);

if (returnSpillVarDsc->lvSingleDef)
if ((returnSpillVarDsc->lvType == TYP_REF) && returnSpillVarDsc->lvSingleDef)
{
lvaUpdateClass(lvaInlineeReturnSpillTemp, retExprClassHnd, impInlineInfo->retExprClassHndIsExact);
}
Expand Down
55 changes: 30 additions & 25 deletions src/coreclr/jit/importer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1862,14 +1862,18 @@ bool Compiler::impSpillStackEntry(unsigned level,
/* Assign the spilled entry to the temp */
impAssignTempGen(tnum, tree, verCurrentState.esStack[level].seTypeInfo.GetClassHandle(), level);

// If temp is newly introduced and a ref type, grab what type info we can.
if (isNewTemp && (lvaTable[tnum].lvType == TYP_REF))
if (isNewTemp)
{
assert(lvaTable[tnum].lvSingleDef == 0);
lvaTable[tnum].lvSingleDef = 1;
JITDUMP("Marked V%02u as a single def temp\n", tnum);
CORINFO_CLASS_HANDLE stkHnd = verCurrentState.esStack[level].seTypeInfo.GetClassHandle();
lvaSetClass(tnum, tree, stkHnd);

// If temp is newly introduced and a ref type, grab what type info we can.
if (lvaTable[tnum].lvType == TYP_REF)
{
CORINFO_CLASS_HANDLE stkHnd = verCurrentState.esStack[level].seTypeInfo.GetClassHandle();
lvaSetClass(tnum, tree, stkHnd);
}

// If we're assigning a GT_RET_EXPR, note the temp over on the call,
// so the inliner can use it in case it needs a return spill temp.
Expand Down Expand Up @@ -8373,12 +8377,12 @@ void Compiler::impImportBlockCode(BasicBlock* block)
impAssignTempGen(tmpNum, op1, tiRetVal.GetClassHandle(), CHECK_SPILL_ALL);
var_types type = genActualType(lvaTable[tmpNum].TypeGet());

assert(lvaTable[tmpNum].lvSingleDef == 0);
lvaTable[tmpNum].lvSingleDef = 1;
JITDUMP("Marked V%02u as a single def local\n", tmpNum);
// Propagate type info to the temp from the stack and the original tree
if (type == TYP_REF)
{
assert(lvaTable[tmpNum].lvSingleDef == 0);
lvaTable[tmpNum].lvSingleDef = 1;
JITDUMP("Marked V%02u as a single def local\n", tmpNum);
lvaSetClass(tmpNum, tree, tiRetVal.GetClassHandle());
}

Expand Down Expand Up @@ -13582,19 +13586,19 @@ unsigned Compiler::impInlineFetchLocal(unsigned lclNum DEBUGARG(const char* reas
lvaTable[tmpNum].lvHasILStoreOp = inlineeLocal.lclHasStlocOp;
lvaTable[tmpNum].lvHasMultipleILStoreOp = inlineeLocal.lclHasMultipleStlocOp;

assert(lvaTable[tmpNum].lvSingleDef == 0);

lvaTable[tmpNum].lvSingleDef = !inlineeLocal.lclHasMultipleStlocOp && !inlineeLocal.lclHasLdlocaOp;
if (lvaTable[tmpNum].lvSingleDef)
{
JITDUMP("Marked V%02u as a single def temp\n", tmpNum);
}

// Copy over class handle for ref types. Note this may be a
// shared type -- someday perhaps we can get the exact
// signature and pass in a more precise type.
if (lclTyp == TYP_REF)
{
assert(lvaTable[tmpNum].lvSingleDef == 0);

lvaTable[tmpNum].lvSingleDef = !inlineeLocal.lclHasMultipleStlocOp && !inlineeLocal.lclHasLdlocaOp;
if (lvaTable[tmpNum].lvSingleDef)
{
JITDUMP("Marked V%02u as a single def temp\n", tmpNum);
}

lvaSetClass(tmpNum, inlineeLocal.lclVerTypeInfo.GetClassHandleForObjRef());
}

Expand Down Expand Up @@ -13779,20 +13783,21 @@ GenTree* Compiler::impInlineFetchArg(unsigned lclNum, InlArgInfo* inlArgInfo, In

lvaTable[tmpNum].lvType = lclTyp;

// For ref types, determine the type of the temp.
if (lclTyp == TYP_REF)
// If arg can't be modified, mark it as single def.
// For ref types, determine the class of the arg temp.
if (!argCanBeModified)
{
if (!argCanBeModified)
assert(lvaTable[tmpNum].lvSingleDef == 0);
lvaTable[tmpNum].lvSingleDef = 1;
JITDUMP("Marked V%02u as a single def temp\n", tmpNum);
if (lclTyp == TYP_REF)
{
// If the arg can't be modified in the method
// body, use the type of the value, if
// known. Otherwise, use the declared type.
assert(lvaTable[tmpNum].lvSingleDef == 0);
lvaTable[tmpNum].lvSingleDef = 1;
JITDUMP("Marked V%02u as a single def temp\n", tmpNum);
lvaSetClass(tmpNum, argNode, lclInfo.lclVerTypeInfo.GetClassHandleForObjRef());
}
else
}
else
{
if (lclTyp == TYP_REF)
{
// Arg might be modified, use the declared type of
// the argument.
Expand Down
7 changes: 3 additions & 4 deletions src/coreclr/jit/importercalls.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5477,12 +5477,11 @@ class SpillRetExprHelper
comp->impAssignTempGen(tmp, retExpr, (unsigned)Compiler::CHECK_SPILL_NONE);
*pRetExpr = comp->gtNewLclvNode(tmp, retExpr->TypeGet());

assert(comp->lvaTable[tmp].lvSingleDef == 0);
comp->lvaTable[tmp].lvSingleDef = 1;
JITDUMP("Marked V%02u as a single def temp\n", tmp);
if (retExpr->TypeGet() == TYP_REF)
{
assert(comp->lvaTable[tmp].lvSingleDef == 0);
comp->lvaTable[tmp].lvSingleDef = 1;
JITDUMP("Marked V%02u as a single def temp\n", tmp);

bool isExact = false;
bool isNonNull = false;
CORINFO_CLASS_HANDLE retClsHnd = comp->gtGetClassHandle(retExpr, &isExact, &isNonNull);
Expand Down

0 comments on commit a193cb5

Please sign in to comment.