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

Less conservative gt obj #6021

Merged
merged 1 commit into from
Jul 7, 2016
Merged

Less conservative gt obj #6021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 11 additions & 7 deletions src/jit/codegenlegacy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17702,7 +17702,7 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,
fieldSize,
fieldVarDsc->lvRegNum,
compiler->lvaOutgoingArgSpaceVar,
fieldArgOffset);
argOffset + fieldArgOffset);

if (fieldVarDsc->lvExactSize == 8)
{
Expand All @@ -17728,15 +17728,15 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,
fieldSize,
regTmp,
compiler->lvaOutgoingArgSpaceVar,
fieldArgOffset + TARGET_POINTER_SIZE);
argOffset + fieldArgOffset + TARGET_POINTER_SIZE);
}
else
{
getEmitter()->emitIns_S_R(ins_Store(TYP_I_IMPL),
fieldSize,
fieldVarDsc->lvOtherReg,
compiler->lvaOutgoingArgSpaceVar,
fieldArgOffset + TARGET_POINTER_SIZE);
argOffset + fieldArgOffset + TARGET_POINTER_SIZE);
}
}
// Record the fact that we filled in an extra register slot
Expand Down Expand Up @@ -17800,7 +17800,7 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,
fieldSize,
regTmp,
compiler->lvaOutgoingArgSpaceVar,
fieldArgOffset);
argOffset + fieldArgOffset);
// We overwrote "regTmp", so erase any previous value we recorded that it contained.
regTracker.rsTrackRegTrash(regTmp);

Expand All @@ -17815,7 +17815,7 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,
fieldSize,
regTmp,
compiler->lvaOutgoingArgSpaceVar,
fieldArgOffset + TARGET_POINTER_SIZE);
argOffset + fieldArgOffset + TARGET_POINTER_SIZE);
// Record the fact that we filled in an extra stack slot
filledExtraSlot = true;
}
Expand Down Expand Up @@ -17862,6 +17862,8 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,
{
if (curRegNum != MAX_REG_ARG)
{
// We are going to write to the lvaPromotedStructAssemblyScratchVar, at the offset
// of this field within the current slot.
noway_assert(compiler->lvaPromotedStructAssemblyScratchVar != BAD_VAR_NUM);

getEmitter()->emitIns_S_R(ins_Store(fieldTypeForInstr),
Expand All @@ -17877,7 +17879,7 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,
fieldSize,
fieldVarDsc->lvRegNum,
compiler->lvaOutgoingArgSpaceVar,
fieldArgOffset);
argOffset + fieldArgOffset);
}
}
else
Expand All @@ -17897,6 +17899,8 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,

if (curRegNum != MAX_REG_ARG)
{
// We are going to write to the lvaPromotedStructAssemblyScratchVar, at the offset
// of this field within the current slot.
noway_assert(compiler->lvaPromotedStructAssemblyScratchVar != BAD_VAR_NUM);

getEmitter()->emitIns_S_R(ins_Store(fieldTypeForInstr),
Expand All @@ -17911,7 +17915,7 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,
fieldSize,
regTmp,
compiler->lvaOutgoingArgSpaceVar,
fieldArgOffset);
argOffset + fieldArgOffset);
}
}
// Go to the next field.
Expand Down
2 changes: 1 addition & 1 deletion src/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6793,7 +6793,7 @@ public :
#endif // _TARGET_ARM_

// If "tree" is a indirection (GT_IND, or GT_OBJ) whose arg is an ADDR, whose arg is a LCL_VAR, return that LCL_VAR node, else NULL.
GenTreePtr fgIsIndirOfAddrOfLocal(GenTreePtr tree);
static GenTreePtr fgIsIndirOfAddrOfLocal(GenTreePtr tree);

// This is indexed by GT_OBJ nodes that are address of promoted struct variables, which
// have been annotated with the GTF_VAR_DEATH flag. If such a node is *not* mapped in this
Expand Down
14 changes: 8 additions & 6 deletions src/jit/flowgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6756,7 +6756,7 @@ bool Compiler::fgIsCommaThrow(GenTreePtr tree,
return false;
}


// static
GenTreePtr Compiler::fgIsIndirOfAddrOfLocal(GenTreePtr tree)
{
GenTreePtr res = nullptr;
Expand Down Expand Up @@ -20589,15 +20589,15 @@ void Compiler::fgDebugCheckFlags(GenTreePtr tree)
if (chkFlags & ~treeFlags)
{
// Print the tree so we can see it in the log.
printf("Missing flags on tree [%X]: ", tree);
printf("Missing flags on tree [%06d]: ", dspTreeID(tree));
GenTree::gtDispFlags(chkFlags & ~treeFlags, GTF_DEBUG_NONE);
printf("\n");
gtDispTree(tree);

noway_assert(!"Missing flags on tree");

// Print the tree again so we can see it right after we hook up the debugger.
printf("Missing flags on tree [%X]: ", tree);
printf("Missing flags on tree [%06d]: ", dspTreeID(tree));
GenTree::gtDispFlags(chkFlags & ~treeFlags, GTF_DEBUG_NONE);
printf("\n");
gtDispTree(tree);
Expand Down Expand Up @@ -22333,9 +22333,11 @@ GenTreePtr Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
!(argSingleUseNode->gtFlags & GTF_VAR_CLONED) &&
!inlArgInfo[argNum].argHasLdargaOp)
{
/* Change the temp in-place to the actual argument */

argSingleUseNode->CopyFrom(inlArgInfo[argNum].argNode, this);
// 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;
}
else
Expand Down
19 changes: 17 additions & 2 deletions src/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5313,12 +5313,14 @@ bool GenTree::OperMayThrow()

break;

case GT_OBJ:
return !Compiler::fgIsIndirOfAddrOfLocal(this);

case GT_ARR_BOUNDS_CHECK:
case GT_ARR_ELEM:
case GT_ARR_INDEX:
case GT_CATCH_ARG:
case GT_ARR_LENGTH:
case GT_OBJ:
case GT_LCLHEAP:
case GT_CKFINITE:
case GT_NULLCHECK:
Expand Down Expand Up @@ -6045,7 +6047,16 @@ GenTreeObj* Compiler::gtNewObjNode(CORINFO_CLASS_HANDLE structHnd, GenTree* addr
{
var_types nodeType = impNormStructType(structHnd);
assert(varTypeIsStruct(nodeType));
return new (this, GT_OBJ) GenTreeObj(nodeType, addr, structHnd);
GenTreeObj *objNode = new (this, GT_OBJ) GenTreeObj(nodeType, addr, structHnd);
// An Obj is not a global reference, if it is known to be a local struct.
GenTreeLclVarCommon* lclNode = addr->IsLocalAddrExpr();
if ((lclNode != nullptr) &&
!lvaIsImplicitByRefLocal(lclNode->gtLclNum) &&
!lvaTable[lclNode->gtLclNum].lvAddrExposed)
{
objNode->gtFlags &= ~GTF_GLOB_REF;
}
return objNode;
}

// Creates a new CpObj node.
Expand Down Expand Up @@ -14141,6 +14152,8 @@ bool FieldSeqNode::IsPseudoField()
#ifdef FEATURE_SIMD
GenTreeSIMD* Compiler::gtNewSIMDNode(var_types type, GenTreePtr op1, SIMDIntrinsicID simdIntrinsicID, var_types baseType, unsigned size)
{
// TODO-CQ: An operand may be a GT_OBJ(GT_ADDR(GT_LCL_VAR))), in which case it should be
// marked lvUsedInSIMDIntrinsic.
assert(op1 != nullptr);
if (op1->OperGet() == GT_LCL_VAR)
{
Expand All @@ -14154,6 +14167,8 @@ GenTreeSIMD* Compiler::gtNewSIMDNode(var_types type, GenTreePtr op1, SIMDIntrins

GenTreeSIMD* Compiler::gtNewSIMDNode(var_types type, GenTreePtr op1, GenTreePtr op2, SIMDIntrinsicID simdIntrinsicID, var_types baseType, unsigned size)
{
// TODO-CQ: An operand may be a GT_OBJ(GT_ADDR(GT_LCL_VAR))), in which case it should be
// marked lvUsedInSIMDIntrinsic.
assert(op1 != nullptr);
if (op1->OperIsLocal())
{
Expand Down
3 changes: 2 additions & 1 deletion src/jit/gentree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3312,7 +3312,8 @@ struct GenTreeObj: public GenTreeUnOp
GenTreeUnOp(GT_OBJ, type, addr),
gtClass(cls)
{
gtFlags |= GTF_GLOB_REF; // An Obj is always a global reference.
// By default, an OBJ is assumed to be a global reference.
gtFlags |= GTF_GLOB_REF;
}

#if DEBUGGABLE_GENTREE
Expand Down
27 changes: 15 additions & 12 deletions src/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1496,7 +1496,7 @@ GenTreePtr Compiler::impNormStructVal(GenTreePtr structVal,

// Normalize it by wraping it in an OBJ

GenTreePtr structAddr = impGetStructAddr(structVal, structHnd, curLevel, !forceNormalization); // get the addr of struct
GenTreePtr structAddr = impGetStructAddr(structVal, structHnd, curLevel, !forceNormalization); // get the addr of struct
GenTreePtr structObj = new (this, GT_OBJ) GenTreeObj(structType, structAddr, structHnd);

if (structAddr->gtOper == GT_ADDR)
Expand All @@ -1510,9 +1510,10 @@ GenTreePtr Compiler::impNormStructVal(GenTreePtr structVal,
{
// A OBJ on a ADDR(LCL_VAR) can never raise an exception
// so we don't set GTF_EXCEPT here.
//
// TODO-CQ: Clear the GTF_GLOB_REF flag on structObj as well
// but this needs additional work when inlining.
if (!lvaIsImplicitByRefLocal(structVal->AsLclVarCommon()->gtLclNum))
{
structObj->gtFlags &= ~GTF_GLOB_REF;
}
}
else
{
Expand Down Expand Up @@ -16783,15 +16784,17 @@ GenTreePtr Compiler::impInlineFetchArg(unsigned lclNum, InlArgInfo *inlArgInfo,
inlArgInfo[lclNum].argHasTmp = true;
inlArgInfo[lclNum].argTmpNum = tmpNum;

/* If we require strict exception order then, arguments must
be evaulated in sequence before the body of the inlined
method. So we need to evaluate them to a temp.
Also, if arguments have global references, we need to
evaluate them to a temp before the inlined body as the
inlined body may be modifying the global ref.
*/
// If we require strict exception order, then arguments must
// be evaluated in sequence before the body of the inlined method.
// So we need to evaluate them to a temp.
// Also, if arguments have global references, we need to
// evaluate them to a temp before the inlined body as the
// inlined body may be modifying the global ref.
// TODO-1stClassStructs: We currently do not reuse an existing lclVar
// if it is a struct, because it requires some additional handling.

if ((!inlArgInfo[lclNum].argHasSideEff) &&
if (!varTypeIsStruct(lclTyp) &&
(!inlArgInfo[lclNum].argHasSideEff) &&
(!inlArgInfo[lclNum].argHasGlobRef))
{
/* Get a *LARGE* LCL_VAR node */
Expand Down
55 changes: 41 additions & 14 deletions src/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2083,9 +2083,9 @@ GenTreePtr Compiler::fgMakeTmpArgNode(unsigned tmpVarNum
arg = gtNewOperNode(GT_ADDR, TYP_BYREF, arg);
addrNode = arg;

// Get a new Obj node temp to use it as a call argument
// Get a new Obj node temp to use it as a call argument.
// gtNewObjNode will set the GTF_EXCEPT flag if this is not a local stack object.
arg = gtNewObjNode(lvaGetStruct(tmpVarNum), arg);
arg->gtFlags |= GTF_EXCEPT;

#endif // not (_TARGET_AMD64_ or _TARGET_ARM64_)

Expand Down Expand Up @@ -3474,7 +3474,8 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
// we must copyblk to a temp before doing the obj to avoid
// the obj reading memory past the end of the valuetype
#if defined(_TARGET_X86_) && !defined(LEGACY_BACKEND)
// TODO-X86-CQ: [1091733] Revisit for small structs, we should use push instruction
// TODO-X86-CQ: [1091733] We should only be copying when roundupSize > originalSize.
// See also the TODO-CQ just before the call to fgMakeOutgoingStructArgCopy().
copyBlkClass = objClass;
size = roundupSize / TARGET_POINTER_SIZE; // Normalize size to number of pointer sized items
#else // !defined(_TARGET_X86_) || defined(LEGACY_BACKEND)
Expand Down Expand Up @@ -3830,6 +3831,13 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* callNode)
{
noway_assert(!lateArgsComputed);
fgMakeOutgoingStructArgCopy(call, args, argIndex, copyBlkClass FEATURE_UNIX_AMD64_STRUCT_PASSING_ONLY_ARG(&structDesc));
// This can cause a GTF_EXCEPT flag to be set.
// TODO-CQ: Fix the cases where this happens. We shouldn't be adding any new flags.
// This currently occurs in the case where we are re-morphing the args on x86/RyuJIT, and
// there are no register arguments. Then lateArgsComputed is never true, so we keep re-copying
// any struct arguments.
// i.e. assert(((call->gtFlags & GTF_EXCEPT) != 0) || ((args->Current()->gtFlags & GTF_EXCEPT) == 0)
flagsSummary |= (args->Current()->gtFlags & GTF_EXCEPT);
}

#ifndef LEGACY_BACKEND
Expand Down Expand Up @@ -4172,8 +4180,6 @@ void Compiler::fgMorphSystemVStructArgs(GenTreeCall* call, bool hasStructArgumen

// Create an Obj of the temp to use it as a call argument.
arg = new (this, GT_OBJ) GenTreeObj(originalType, arg, lvaGetStruct(lclCommon->gtLclNum));
arg->gtFlags |= GTF_EXCEPT;
flagsSummary |= GTF_EXCEPT;
}
}
}
Expand Down Expand Up @@ -8005,17 +8011,19 @@ GenTreePtr Compiler::fgMorphOneAsgBlockOp(GenTreePtr tree)
}
}

/* Indirect the dest node */
// Indirect the dest node.

dest = gtNewOperNode(GT_IND, type, dest);

/* As long as we don't have more information about the destination we
have to assume it could live anywhere (not just in the GC heap). Mark
the GT_IND node so that we use the correct write barrier helper in case
the field is a GC ref.
*/
// If we have no information about the destination, we have to assume it could
// live anywhere (not just in the GC heap).
// Mark the GT_IND node so that we use the correct write barrier helper in case
// the field is a GC ref.

dest->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE);
if (!fgIsIndirOfAddrOfLocal(dest))
{
dest->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE);
}

_DoneDest:;

Expand Down Expand Up @@ -8063,10 +8071,19 @@ _DoneDest:;
}
}

/* Indirect the src node */
// Indirect the src node.

src = gtNewOperNode(GT_IND, type, src);
src->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE);

// If we have no information about the src, we have to assume it could
// live anywhere (not just in the GC heap).
// Mark the GT_IND node so that we use the correct write barrier helper in case
// the field is a GC ref.

if (!fgIsIndirOfAddrOfLocal(src))
{
src->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE);
}

_DoneSrc:;

Expand Down Expand Up @@ -11397,6 +11414,16 @@ CM_OVF_OP :
fgAddCodeRef(compCurBB, bbThrowIndex(compCurBB), SCK_ARITH_EXCPN, fgPtrArgCntCur);
break;

case GT_OBJ:
// If we have GT_OBJ(GT_ADDR(X)) and X has GTF_GLOB_REF, we must set GTF_GLOB_REF on
// the GT_OBJ. Note that the GTF_GLOB_REF will have been cleared on ADDR(X) where X
// is a local or clsVar, even if it has been address-exposed.
if (op1->OperGet() == GT_ADDR)
{
tree->gtFlags |= (op1->gtGetOp1()->gtFlags & GTF_GLOB_REF);
}
break;

case GT_IND:

// Can not remove a GT_IND if it is currently a CSE candidate.
Expand Down
2 changes: 1 addition & 1 deletion src/jit/regalloc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4683,7 +4683,7 @@ regMaskTP Compiler::rpPredictTreeRegUse(GenTreePtr tree,
}
}

if (promotedStructLocal != NULL)
if ((promotedStructLocal != NULL) && (curArgMask != RBM_NONE))
{
// All or a portion of this struct will be placed in the argument registers indicated by "curArgMask".
// We build in knowledge of the order in which the code is generated here, so that the second arg to be evaluated
Expand Down