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

JIT: improve types for single def locals and temps #10471

Merged
merged 3 commits into from
Mar 29, 2017
Merged

JIT: improve types for single def locals and temps #10471

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b744613
JIT: improve types for single def locals and temps
AndyAyersMS Mar 24, 2017
0f5141f
incorporate review feedback
AndyAyersMS Mar 27, 2017
45ae3dc
more review feedback
AndyAyersMS Mar 29, 2017
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
19 changes: 15 additions & 4 deletions src/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,10 @@ class LclVarDsc
unsigned char lvStackByref : 1; // This is a compiler temporary of TYP_BYREF that is known to point into our local
// stack frame.

unsigned char lvArgWrite : 1; // variable is a parameter and STARG was used on it
unsigned char lvIsTemp : 1; // Short-lifetime compiler temp
unsigned char lvHasILStoreOp : 1; // there is at least one STLOC or STARG on this local
unsigned char lvHasMultipleILStoreOp : 1; // there is more than one STLOC on this local

unsigned char lvIsTemp : 1; // Short-lifetime compiler temp
#if OPT_BOOL_OPS
unsigned char lvIsBoolean : 1; // set if variable is boolean
#endif
Expand Down Expand Up @@ -324,6 +326,10 @@ class LclVarDsc

unsigned char lvClassIsExact : 1; // lvClassHandle is the exact type

#ifdef DEBUG
unsigned char lvClassInfoUpdated : 1; // true if this var has updated class handle or exactness
#endif

union {
unsigned lvFieldLclStart; // The index of the local var representing the first field in the promoted struct
// local.
Expand Down Expand Up @@ -2672,11 +2678,16 @@ class Compiler
// Returns true if this local var is a multireg struct
bool lvaIsMultiregStruct(LclVarDsc* varDsc);

// If the class is a TYP_STRUCT, get/set a class handle describing it

// If the local is a TYP_STRUCT, get/set a class handle describing it
CORINFO_CLASS_HANDLE lvaGetStruct(unsigned varNum);
void lvaSetStruct(unsigned varNum, CORINFO_CLASS_HANDLE typeHnd, bool unsafeValueClsCheck, bool setTypeInfo = true);

// If the local is TYP_REF, set or update the associated class information.
void lvaSetClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool isExact = false);
void lvaSetClass(unsigned varNum, GenTreePtr tree, CORINFO_CLASS_HANDLE stackHandle = nullptr);
void lvaUpdateClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool isExact = false);
void lvaUpdateClass(unsigned varNum, GenTreePtr tree, CORINFO_CLASS_HANDLE stackHandle = nullptr);

#define MAX_NumOfFieldsInPromotableStruct 4 // Maximum number of fields in promotable struct

// Info about struct fields
Expand Down
4 changes: 2 additions & 2 deletions src/jit/compiler.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2530,10 +2530,10 @@ inline BOOL Compiler::lvaIsOriginalThisArg(unsigned varNum)
// copy to a new local, and mark the original as DoNotEnregister, to
// ensure that it is stack-allocated. It should not be the case that the original one can be modified -- it
// should not be written to, or address-exposed.
assert(!varDsc->lvArgWrite &&
assert(!varDsc->lvHasILStoreOp &&
(!varDsc->lvAddrExposed || ((info.compMethodInfo->options & CORINFO_GENERICS_CTXT_FROM_THIS) != 0)));
#else
assert(!varDsc->lvArgWrite && !varDsc->lvAddrExposed);
assert(!varDsc->lvHasILStoreOp && !varDsc->lvAddrExposed);
#endif
}
#endif
Expand Down
177 changes: 106 additions & 71 deletions src/jit/flowgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4271,7 +4271,7 @@ class FgStack
// jumpTarget[N] is set to a JT_* value if IL offset N is a
// jump target in the method.
//
// Also sets lvAddrExposed and lvArgWrite in lvaTable[].
// Also sets lvAddrExposed and lvHasILStoreOp, ilHasMultipleILStoreOp in lvaTable[].

#ifdef _PREFAST_
#pragma warning(push)
Expand Down Expand Up @@ -4548,12 +4548,67 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, BYTE*
// account for possible hidden param
varNum = compMapILargNum(varNum);

// This check is only intended to prevent an AV. Bad varNum values will later
// be handled properly by the verifier.
if (varNum < lvaTableCnt)
{
// In non-inline cases, note written-to arguments.
lvaTable[varNum].lvHasILStoreOp = 1;
}
}
}
break;

case CEE_STLOC_0:
case CEE_STLOC_1:
case CEE_STLOC_2:
case CEE_STLOC_3:
varNum = (opcode - CEE_STLOC_0);
goto STLOC;

case CEE_STLOC:
case CEE_STLOC_S:
{
noway_assert(sz == sizeof(BYTE) || sz == sizeof(WORD));

if (codeAddr > codeEndp - sz)
{
goto TOO_FAR;
}

varNum = (sz == sizeof(BYTE)) ? getU1LittleEndian(codeAddr) : getU2LittleEndian(codeAddr);

STLOC:
if (isInlining)
{
InlLclVarInfo& lclInfo = impInlineInfo->lclVarInfo[varNum + impInlineInfo->argCnt];

if (lclInfo.lclHasStlocOp)
{
lclInfo.lclHasMultipleStlocOp = 1;
}
else
{
lclInfo.lclHasStlocOp = 1;
}
}
else
{
varNum += info.compArgsCount;

// This check is only intended to prevent an AV. Bad varNum values will later
// be handled properly by the verifier.
if (varNum < lvaTableCnt)
{
// In non-inline cases, note written-to locals.
lvaTable[varNum].lvArgWrite = 1;
if (lvaTable[varNum].lvHasILStoreOp)
{
lvaTable[varNum].lvHasMultipleILStoreOp = 1;
}
else
{
lvaTable[varNum].lvHasILStoreOp = 1;
}
}
}
}
Expand Down Expand Up @@ -4875,11 +4930,11 @@ void Compiler::fgAdjustForAddressExposedOrWrittenThis()
// Optionally enable adjustment during stress.
if (!tiVerificationNeeded && compStressCompile(STRESS_GENERIC_VARN, 15))
{
lvaTable[info.compThisArg].lvArgWrite = true;
lvaTable[info.compThisArg].lvHasILStoreOp = true;
}

// If this is exposed or written to, create a temp for the modifiable this
if (lvaTable[info.compThisArg].lvAddrExposed || lvaTable[info.compThisArg].lvArgWrite)
if (lvaTable[info.compThisArg].lvAddrExposed || lvaTable[info.compThisArg].lvHasILStoreOp)
{
// If there is a "ldarga 0" or "starg 0", grab and use the temp.
lvaArg0Var = lvaGrabTemp(false DEBUGARG("Address-exposed, or written this pointer"));
Expand All @@ -4893,14 +4948,14 @@ void Compiler::fgAdjustForAddressExposedOrWrittenThis()
lvaTable[lvaArg0Var].lvLclFieldExpr = lvaTable[info.compThisArg].lvLclFieldExpr;
lvaTable[lvaArg0Var].lvLiveAcrossUCall = lvaTable[info.compThisArg].lvLiveAcrossUCall;
#endif
lvaTable[lvaArg0Var].lvArgWrite = lvaTable[info.compThisArg].lvArgWrite;
lvaTable[lvaArg0Var].lvVerTypeInfo = lvaTable[info.compThisArg].lvVerTypeInfo;
lvaTable[lvaArg0Var].lvHasILStoreOp = lvaTable[info.compThisArg].lvHasILStoreOp;
lvaTable[lvaArg0Var].lvVerTypeInfo = lvaTable[info.compThisArg].lvVerTypeInfo;

// Clear the TI_FLAG_THIS_PTR in the original 'this' pointer.
noway_assert(lvaTable[lvaArg0Var].lvVerTypeInfo.IsThisPtr());
lvaTable[info.compThisArg].lvVerTypeInfo.ClearThisPtr();
lvaTable[info.compThisArg].lvAddrExposed = false;
lvaTable[info.compThisArg].lvArgWrite = false;
lvaTable[info.compThisArg].lvAddrExposed = false;
lvaTable[info.compThisArg].lvHasILStoreOp = false;
}
}

Expand Down Expand Up @@ -8034,8 +8089,8 @@ void Compiler::fgAddInternal()
lva0CopiedForGenericsCtxt = false;
#endif // JIT32_GCENCODER
noway_assert(lva0CopiedForGenericsCtxt || !lvaTable[info.compThisArg].lvAddrExposed);
noway_assert(!lvaTable[info.compThisArg].lvArgWrite);
noway_assert(lvaTable[lvaArg0Var].lvAddrExposed || lvaTable[lvaArg0Var].lvArgWrite ||
noway_assert(!lvaTable[info.compThisArg].lvHasILStoreOp);
noway_assert(lvaTable[lvaArg0Var].lvAddrExposed || lvaTable[lvaArg0Var].lvHasILStoreOp ||
lva0CopiedForGenericsCtxt);

var_types thisType = lvaTable[info.compThisArg].TypeGet();
Expand Down Expand Up @@ -20428,10 +20483,11 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef
// Should never expose the address of arg 0 or write to arg 0.
// In addition, lvArg0Var should remain 0 if arg0 is not
// written to or address-exposed.
noway_assert(compThisArgAddrExposedOK && !lvaTable[info.compThisArg].lvArgWrite &&
(lvaArg0Var == info.compThisArg ||
lvaArg0Var != info.compThisArg && (lvaTable[lvaArg0Var].lvAddrExposed ||
lvaTable[lvaArg0Var].lvArgWrite || copiedForGenericsCtxt)));
noway_assert(
compThisArgAddrExposedOK && !lvaTable[info.compThisArg].lvHasILStoreOp &&
(lvaArg0Var == info.compThisArg ||
lvaArg0Var != info.compThisArg &&
(lvaTable[lvaArg0Var].lvAddrExposed || lvaTable[lvaArg0Var].lvHasILStoreOp || copiedForGenericsCtxt)));
}
}

Expand Down Expand Up @@ -22331,9 +22387,13 @@ GenTreePtr Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)

for (unsigned argNum = 0; argNum < inlineInfo->argCnt; argNum++)
{
if (inlArgInfo[argNum].argHasTmp)
const InlArgInfo& argInfo = inlArgInfo[argNum];
const bool argIsSingleDef = !argInfo.argHasLdargaOp && !argInfo.argHasStargOp;
GenTree* const argNode = inlArgInfo[argNum].argNode;
Copy link

Choose a reason for hiding this comment

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

Shorten to argInfo.argNode?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, not sure how I missed these. Will clean them up.


if (argInfo.argHasTmp)
{
noway_assert(inlArgInfo[argNum].argIsUsed);
noway_assert(argInfo.argIsUsed);

/* argBashTmpNode is non-NULL iff the argument's value was
referenced exactly once by the original IL. This offers an
Expand All @@ -22347,14 +22407,12 @@ GenTreePtr Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
once) but the optimization cannot be applied.
*/

GenTreePtr argSingleUseNode = inlArgInfo[argNum].argBashTmpNode;
GenTreePtr argSingleUseNode = argInfo.argBashTmpNode;

if (argSingleUseNode && !(argSingleUseNode->gtFlags & GTF_VAR_CLONED) &&
!inlArgInfo[argNum].argHasLdargaOp && !inlArgInfo[argNum].argHasStargOp)
if ((argSingleUseNode != nullptr) && !(argSingleUseNode->gtFlags & GTF_VAR_CLONED) && argIsSingleDef)
{
// Change the temp in-place to the actual argument.
// We currently do not support this for struct arguments, so it must not be a GT_OBJ.
GenTree* argNode = inlArgInfo[argNum].argNode;
assert(argNode->gtOper != GT_OBJ);
argSingleUseNode->CopyFrom(argNode, this);
continue;
Expand All @@ -22363,53 +22421,32 @@ GenTreePtr Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
{
// We're going to assign the argument value to the
// temp we use for it in the inline body.
//
// If we know the argument's value can't be
// changed within the method body, try and improve
// the type of the temp.
if (!inlArgInfo[argNum].argHasLdargaOp && !inlArgInfo[argNum].argHasStargOp)
{
GenTree* argNode = inlArgInfo[argNum].argNode;
bool isExact = false;
bool isNonNull = false;
CORINFO_CLASS_HANDLE refClassHandle = gtGetClassHandle(argNode, &isExact, &isNonNull);

if (refClassHandle != nullptr)
{
const unsigned tmpNum = inlArgInfo[argNum].argTmpNum;

// If we already had an exact type for
// this temp, this new information had
// better agree with what we knew before.
if (lvaTable[tmpNum].lvClassIsExact)
{
assert(isExact);
assert(refClassHandle == lvaTable[tmpNum].lvClassHnd);
}
else
{
lvaTable[tmpNum].lvClassHnd = refClassHandle;
lvaTable[tmpNum].lvClassIsExact = isExact;
}
}
}

/* Create the temp assignment for this argument */
const unsigned tmpNum = argInfo.argTmpNum;
const var_types argType = lclVarInfo[argNum].lclTypeInfo;

// Create the temp assignment for this argument
CORINFO_CLASS_HANDLE structHnd = DUMMY_INIT(0);

if (varTypeIsStruct(lclVarInfo[argNum].lclTypeInfo))
if (varTypeIsStruct(argType))
{
structHnd = gtGetStructHandleIfPresent(inlArgInfo[argNum].argNode);
structHnd = gtGetStructHandleIfPresent(argNode);
noway_assert(structHnd != NO_CLASS_HANDLE);
}

// Unsafe value cls check is not needed for
// argTmpNum here since in-linee compiler instance
// would have iterated over these and marked them
// accordingly.
impAssignTempGen(inlArgInfo[argNum].argTmpNum, inlArgInfo[argNum].argNode, structHnd,
(unsigned)CHECK_SPILL_NONE, &afterStmt, callILOffset, block);
impAssignTempGen(tmpNum, argNode, structHnd, (unsigned)CHECK_SPILL_NONE, &afterStmt, callILOffset,
block);

// If we know the argument's value can't be
// changed within the method body, try and improve
// the type of the temp.
if (argIsSingleDef && (argType == TYP_REF))
{
lvaUpdateClass(tmpNum, argNode);
}

#ifdef DEBUG
if (verbose)
Expand All @@ -22419,44 +22456,42 @@ GenTreePtr Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
#endif // DEBUG
}
}
else if (inlArgInfo[argNum].argIsByRefToStructLocal)
else if (argInfo.argIsByRefToStructLocal)
{
// Do nothing.
// Do nothing. Arg was directly substituted as we read
// the inlinee.
}
else
{
/* The argument is either not used or a const or lcl var */

noway_assert(!inlArgInfo[argNum].argIsUsed || inlArgInfo[argNum].argIsInvariant ||
inlArgInfo[argNum].argIsLclVar);
noway_assert(!argInfo.argIsUsed || argInfo.argIsInvariant || argInfo.argIsLclVar);

/* Make sure we didnt change argNode's along the way, or else
subsequent uses of the arg would have worked with the bashed value */
if (inlArgInfo[argNum].argIsInvariant)
if (argInfo.argIsInvariant)
{
assert(inlArgInfo[argNum].argNode->OperIsConst() || inlArgInfo[argNum].argNode->gtOper == GT_ADDR);
assert(argNode->OperIsConst() || argNode->gtOper == GT_ADDR);
}
noway_assert((inlArgInfo[argNum].argIsLclVar == 0) ==
(inlArgInfo[argNum].argNode->gtOper != GT_LCL_VAR ||
(inlArgInfo[argNum].argNode->gtFlags & GTF_GLOB_REF)));
noway_assert((argInfo.argIsLclVar == 0) ==
(argNode->gtOper != GT_LCL_VAR || (argNode->gtFlags & GTF_GLOB_REF)));

/* If the argument has side effects, append it */

if (inlArgInfo[argNum].argHasSideEff)
if (argInfo.argHasSideEff)
{
noway_assert(inlArgInfo[argNum].argIsUsed == false);
noway_assert(argInfo.argIsUsed == false);

if (inlArgInfo[argNum].argNode->gtOper == GT_OBJ ||
inlArgInfo[argNum].argNode->gtOper == GT_MKREFANY)
if (argNode->gtOper == GT_OBJ || argNode->gtOper == GT_MKREFANY)
{
// Don't put GT_OBJ node under a GT_COMMA.
// Codegen can't deal with it.
// Just hang the address here in case there are side-effect.
newStmt = gtNewStmt(gtUnusedValNode(inlArgInfo[argNum].argNode->gtOp.gtOp1), callILOffset);
newStmt = gtNewStmt(gtUnusedValNode(argNode->gtOp.gtOp1), callILOffset);
}
else
{
newStmt = gtNewStmt(gtUnusedValNode(inlArgInfo[argNum].argNode), callILOffset);
newStmt = gtNewStmt(gtUnusedValNode(argNode), callILOffset);
}
afterStmt = fgInsertStmtAfter(block, afterStmt, newStmt);

Expand Down
Loading