Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Track single def locals in importer #21251

Merged
merged 2 commits into from
Nov 29, 2018
Merged

Track single def locals in importer #21251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0626684
Defer setting lvSingleDef until lvaMarkLocalVars
AndyAyersMS Nov 14, 2018
09b69f8
Track single-def locals in the importer
AndyAyersMS Nov 15, 2018
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
5 changes: 4 additions & 1 deletion src/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,11 @@ class LclVarDsc
#if OPT_BOOL_OPS
unsigned char lvIsBoolean : 1; // set if variable is boolean
#endif
unsigned char lvSingleDef : 1; // variable has a single def
// before lvaMarkLocalVars: identifies ref type locals that can get type updates
// after lvaMarkLocalVars: identifies locals that are suitable for optAddCopies

#if ASSERTION_PROP
unsigned char lvSingleDef : 1; // variable has a single def
unsigned char lvDisqualify : 1; // variable is no longer OK for add copy optimization
unsigned char lvVolatileHint : 1; // hint for AssertionProp
#endif
Expand Down
56 changes: 56 additions & 0 deletions src/jit/flowgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4944,6 +4944,34 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
{
fgAdjustForAddressExposedOrWrittenThis();
}

// Now that we've seen the IL, set lvSingleDef for root method
// locals.
//
// We could also do this for root method arguments but single-def
// arguments are set by the caller and so we don't know anything
// about the possible values or types.
//
// For inlinees we do this over in impInlineFetchLocal and
// impInlineFetchArg (here args are included as we somtimes get
// new information about the types of inlinee args).
if (!isInlining)
{
const unsigned firstLcl = info.compArgsCount;
const unsigned lastLcl = firstLcl + info.compMethodInfo->locals.numArgs;
for (unsigned lclNum = firstLcl; lclNum < lastLcl; lclNum++)
{
LclVarDsc* lclDsc = lvaGetDesc(lclNum);
assert(lclDsc->lvSingleDef == 0);
// could restrict this to TYP_REF
lclDsc->lvSingleDef = !lclDsc->lvHasMultipleILStoreOp && !lclDsc->lvHasLdAddrOp;

if (lclDsc->lvSingleDef)
{
JITDUMP("Marked V%02u as a single def local\n", lclNum);
}
}
}
}

#ifdef _PREFAST_
Expand Down Expand Up @@ -5876,6 +5904,9 @@ void Compiler::fgFindBasicBlocks()
// out we can prove the method returns a more specific type.
if (info.compRetType == TYP_REF)
{
lvaTable[lvaInlineeReturnSpillTemp].lvSingleDef = 1;
JITDUMP("Marked V%02u as a single def temp\n", lvaInlineeReturnSpillTemp);

CORINFO_CLASS_HANDLE retClassHnd = impInlineInfo->inlineCandidateInfo->methInfo.args.retTypeClass;
if (retClassHnd != nullptr)
{
Expand Down Expand Up @@ -22423,6 +22454,31 @@ Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkD
comp->impDevirtualizeCall(call, &method, &methodFlags, &context, nullptr);
}
}
else if (tree->OperGet() == GT_ASG)
{
// If we're assigning to a ref typed local that has one definition,
// we may be able to sharpen the type for the local.
GenTree* lhs = tree->gtGetOp1()->gtEffectiveVal();

if ((lhs->OperGet() == GT_LCL_VAR) && (lhs->TypeGet() == TYP_REF))
{
const unsigned lclNum = lhs->gtLclVarCommon.gtLclNum;
LclVarDsc* lcl = comp->lvaGetDesc(lclNum);

if (lcl->lvSingleDef)
{
GenTree* rhs = tree->gtGetOp2();
bool isExact = false;
bool isNonNull = false;
CORINFO_CLASS_HANDLE newClass = comp->gtGetClassHandle(rhs, &isExact, &isNonNull);

if (newClass != NO_CLASS_HANDLE)
{
comp->lvaUpdateClass(lclNum, newClass, isExact);
}
}
}
}

return WALK_CONTINUE;
}
Expand Down
39 changes: 33 additions & 6 deletions src/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2255,6 +2255,9 @@ bool Compiler::impSpillStackEntry(unsigned level,
// If temp is newly introduced and a ref type, grab what type info we can.
if (isNewTemp && (lvaTable[tnum].lvType == TYP_REF))
{
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);

Expand Down Expand Up @@ -5686,9 +5689,11 @@ void Compiler::impImportAndPushBox(CORINFO_RESOLVED_TOKEN* pResolvedToken)
{
// When optimizing, use a new temp for each box operation
// since we then know the exact class of the box temp.
impBoxTemp = lvaGrabTemp(true DEBUGARG("Single-def Box Helper"));
lvaTable[impBoxTemp].lvType = TYP_REF;
const bool isExact = true;
impBoxTemp = lvaGrabTemp(true DEBUGARG("Single-def Box Helper"));
lvaTable[impBoxTemp].lvType = TYP_REF;
lvaTable[impBoxTemp].lvSingleDef = 1;
JITDUMP("Marking V%02u as a single def local\n", impBoxTemp);
const bool isExact = true;
lvaSetClass(impBoxTemp, pResolvedToken->hClass, isExact);
}

Expand Down Expand Up @@ -10192,6 +10197,10 @@ GenTree* Compiler::impCastClassOrIsInstToTree(GenTree* op1,
//
// See also gtGetHelperCallClassHandle where we make the same
// determination for the helper call variants.
LclVarDsc* lclDsc = lvaGetDesc(tmp);
assert(lclDsc->lvSingleDef == 0);
lclDsc->lvSingleDef = 1;
JITDUMP("Marked V%02u as a single def temp\n", tmp);
lvaSetClass(tmp, pResolvedToken->hClass);
return gtNewLclvNode(tmp, TYP_REF);
#endif
Expand Down Expand Up @@ -10847,14 +10856,14 @@ void Compiler::impImportBlockCode(BasicBlock* block)
// We should have seen a stloc in our IL prescan.
assert(lvaTable[lclNum].lvHasILStoreOp);

const bool isSingleILStoreLocal =
!lvaTable[lclNum].lvHasMultipleILStoreOp && !lvaTable[lclNum].lvHasLdAddrOp;
// Is there just one place this local is defined?
const bool isSingleDefLocal = lvaTable[lclNum].lvSingleDef;

// Conservative check that there is just one
// definition that reaches this store.
const bool hasSingleReachingDef = (block->bbStackDepthOnEntry() == 0);

if (isSingleILStoreLocal && hasSingleReachingDef)
if (isSingleDefLocal && hasSingleReachingDef)
{
lvaUpdateClass(lclNum, op1, clsHnd);
}
Expand Down Expand Up @@ -12718,6 +12727,9 @@ void Compiler::impImportBlockCode(BasicBlock* block)
// 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 @@ -13434,6 +13446,10 @@ void Compiler::impImportBlockCode(BasicBlock* block)
// without exhaustive walk over all expressions.

impAssignTempGen(lclNum, op1, (unsigned)CHECK_SPILL_NONE);

assert(lvaTable[lclNum].lvSingleDef == 0);
lvaTable[lclNum].lvSingleDef = 1;
JITDUMP("Marked V%02u as a single def local\n", lclNum);
lvaSetClass(lclNum, resolvedToken.hClass, true /* is Exact */);

newObjThisPtr = gtNewLclvNode(lclNum, TYP_REF);
Expand Down Expand Up @@ -18737,6 +18753,14 @@ unsigned Compiler::impInlineFetchLocal(unsigned lclNum DEBUGARG(const char* reas
// 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 @@ -18928,6 +18952,9 @@ GenTree* Compiler::impInlineFetchArg(unsigned lclNum, InlArgInfo* inlArgInfo, In
// 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, argInfo.argNode, lclInfo.lclVerTypeInfo.GetClassHandleForObjRef());
}
else
Expand Down
105 changes: 49 additions & 56 deletions src/jit/lclvars.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,11 +387,7 @@ void Compiler::lvaInitThisPtr(InitVarDscInfo* varDscInfo)
if (!info.compIsStatic)
{
varDsc->lvIsParam = 1;
#if ASSERTION_PROP
varDsc->lvSingleDef = 1;
#endif

varDsc->lvIsPtr = 1;
varDsc->lvIsPtr = 1;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still have varDsc->lvSingleDef here?
It is important to mark the 'this' pointer as a single def variable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is set later on (as you note below).


lvaArg0Var = info.compThisArg = varDscInfo->varNum;
noway_assert(info.compThisArg == 0);
Expand Down Expand Up @@ -474,9 +470,7 @@ void Compiler::lvaInitRetBuffArg(InitVarDscInfo* varDscInfo)
varDsc->lvType = TYP_BYREF;
varDsc->lvIsParam = 1;
varDsc->lvIsRegArg = 1;
#if ASSERTION_PROP
varDsc->lvSingleDef = 1;
#endif

if (hasFixedRetBuffReg())
{
varDsc->lvArgReg = theFixedRetBuffReg();
Expand Down Expand Up @@ -564,9 +558,6 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo)

CorInfoTypeWithMod corInfoType = info.compCompHnd->getArgType(&info.compMethodInfo->args, argLst, &typeHnd);
varDsc->lvIsParam = 1;
#if ASSERTION_PROP
varDsc->lvSingleDef = 1;
#endif

lvaInitVarDsc(varDsc, varDscInfo->varNum, strip(corInfoType), typeHnd, argLst, &info.compMethodInfo->args);

Expand Down Expand Up @@ -1046,11 +1037,7 @@ void Compiler::lvaInitGenericsCtxt(InitVarDscInfo* varDscInfo)

LclVarDsc* varDsc = varDscInfo->varDsc;
varDsc->lvIsParam = 1;
#if ASSERTION_PROP
varDsc->lvSingleDef = 1;
#endif

varDsc->lvType = TYP_I_IMPL;
varDsc->lvType = TYP_I_IMPL;

if (varDscInfo->canEnreg(TYP_I_IMPL))
{
Expand Down Expand Up @@ -1112,10 +1099,6 @@ void Compiler::lvaInitVarArgsHandle(InitVarDscInfo* varDscInfo)
// that other problems are fixed.
lvaSetVarAddrExposed(varDscInfo->varNum);

#if ASSERTION_PROP
varDsc->lvSingleDef = 1;
#endif

if (varDscInfo->canEnreg(TYP_I_IMPL))
{
/* Another register argument */
Expand Down Expand Up @@ -2706,54 +2689,60 @@ void Compiler::lvaUpdateClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool
// We should already have a class
assert(varDsc->lvClassHnd != nullptr);

#if defined(DEBUG)
// We should only be updating classes for single-def locals.
assert(varDsc->lvSingleDef);

// In general we only expect one update per local var. However if
// a block is re-imported and that block has the only STLOC for
// the var, we may see multiple updates. All subsequent updates
// should agree on the type, since reimportation is triggered by
// type mismatches for things other than ref types.
if (varDsc->lvClassInfoUpdated)
{
assert(varDsc->lvClassHnd == clsHnd);
assert(varDsc->lvClassIsExact == isExact);
}
// Now see if we should update.
//
// New information may not always be "better" so do some
// simple analysis to decide if the update is worthwhile.
const bool isNewClass = (clsHnd != varDsc->lvClassHnd);
bool shouldUpdate = false;

// This counts as an update, even if nothing changes.
varDsc->lvClassInfoUpdated = true;
// Are we attempting to update the class? Only check this when we have
// an new type and the existing class is inexact... we should not be
// updating exact classes.
if (!varDsc->lvClassIsExact && isNewClass)
{
// Todo: improve this analysis by adding a new jit interface method
DWORD newAttrs = info.compCompHnd->getClassAttribs(clsHnd);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Computing class attribs is relatively expensive operation. It computes a ton more than just the CORINFO_FLG_SHAREDINST flag that you care about here. It would be better to change mergeClasses to return NULL when any of the types is shared.

DWORD oldAttrs = info.compCompHnd->getClassAttribs(varDsc->lvClassHnd);

#endif // defined(DEBUG)
// Avoid funny things with __Canon by only merging if both shared or both unshared
if ((newAttrs & CORINFO_FLG_SHAREDINST) == (oldAttrs & CORINFO_FLG_SHAREDINST))
{
// If we merge types and we get back the old class, the new class is more
// specific and we should update to it.
CORINFO_CLASS_HANDLE mergeClass = info.compCompHnd->mergeClasses(clsHnd, varDsc->lvClassHnd);

// If previous type was exact, there is nothing to update. Would
// like to verify new type is compatible but can't do this yet.
if (varDsc->lvClassIsExact)
{
return;
if (mergeClass == varDsc->lvClassHnd)
{
shouldUpdate = true;
}
}
else if ((newAttrs & CORINFO_FLG_SHAREDINST) == 0)
{
// Update if we go from shared to unshared
shouldUpdate = true;
}
}

// Are we updating the type?
if (varDsc->lvClassHnd != clsHnd)
// Else are we attempting to update exactness?
else if (isExact && !varDsc->lvClassIsExact && !isNewClass)
{
JITDUMP("\nlvaUpdateClass: Updating class for V%02i from (%p) %s to (%p) %s %s\n", varNum,
dspPtr(varDsc->lvClassHnd), info.compCompHnd->getClassName(varDsc->lvClassHnd), dspPtr(clsHnd),
info.compCompHnd->getClassName(clsHnd), isExact ? " [exact]" : "");

varDsc->lvClassHnd = clsHnd;
varDsc->lvClassIsExact = isExact;
return;
shouldUpdate = true;
}

// Class info matched. Are we updating exactness?
if (isExact)
{
JITDUMP("\nlvaUpdateClass: Updating class for V%02i (%p) %s to be exact\n", varNum, dspPtr(varDsc->lvClassHnd),
info.compCompHnd->getClassName(varDsc->lvClassHnd));
JITDUMP("\nlvaUpdateClass:%s Updating class for V%02u from (%p) %s%s to (%p) %s%s\n", varNum,
shouldUpdate ? "" : " NOT", dspPtr(varDsc->lvClassHnd), info.compCompHnd->getClassName(varDsc->lvClassHnd),
varDsc->lvClassIsExact ? " [exact]" : "", dspPtr(clsHnd), info.compCompHnd->getClassName(clsHnd),
isExact ? " [exact]" : "");

if (shouldUpdate)
{
varDsc->lvClassHnd = clsHnd;
varDsc->lvClassIsExact = isExact;
return;
}

// Else we have the same handle and (in)exactness as before. Do nothing.
return;
}

Expand Down Expand Up @@ -4155,6 +4144,10 @@ void Compiler::lvaComputeRefCounts(bool isRecompute, bool setSlotNumbers)
{
varDsc->lvSlotNum = lclNum;
}

// Set initial value for lvSingleDef for explicit and implicit
// argument locals as they are "defined" on entry.
varDsc->lvSingleDef = varDsc->lvIsParam;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this where we get the 'this' pointers lcSingleDef set to true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

}

JITDUMP("\n*** lvaComputeRefCounts -- explicit counts ***\n");
Expand Down