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

Revert "Less conservative gt obj" #6227

Merged
merged 1 commit into from
Jul 19, 2016
Merged

Revert "Less conservative gt obj" #6227

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: 7 additions & 11 deletions src/jit/codegenlegacy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17725,7 +17725,7 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,
fieldSize,
fieldVarDsc->lvRegNum,
compiler->lvaOutgoingArgSpaceVar,
argOffset + fieldArgOffset);
fieldArgOffset);

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

Expand All @@ -17838,7 +17838,7 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,
fieldSize,
regTmp,
compiler->lvaOutgoingArgSpaceVar,
argOffset + fieldArgOffset + TARGET_POINTER_SIZE);
fieldArgOffset + TARGET_POINTER_SIZE);
// Record the fact that we filled in an extra stack slot
filledExtraSlot = true;
}
Expand Down Expand Up @@ -17885,8 +17885,6 @@ 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 @@ -17902,7 +17900,7 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,
fieldSize,
fieldVarDsc->lvRegNum,
compiler->lvaOutgoingArgSpaceVar,
argOffset + fieldArgOffset);
fieldArgOffset);
}
}
else
Expand All @@ -17922,8 +17920,6 @@ 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 @@ -17938,7 +17934,7 @@ bool CodeGen::genFillSlotFromPromotedStruct(GenTreePtr arg,
fieldSize,
regTmp,
compiler->lvaOutgoingArgSpaceVar,
argOffset + fieldArgOffset);
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 @@ -6824,7 +6824,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.
static GenTreePtr fgIsIndirOfAddrOfLocal(GenTreePtr tree);
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: 6 additions & 8 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 @@ -20592,15 +20592,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 [%06d]: ", dspTreeID(tree));
printf("Missing flags on tree [%X]: ", 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 [%06d]: ", dspTreeID(tree));
printf("Missing flags on tree [%X]: ", tree);
GenTree::gtDispFlags(chkFlags & ~treeFlags, GTF_DEBUG_NONE);
printf("\n");
gtDispTree(tree);
Expand Down Expand Up @@ -22330,11 +22330,9 @@ GenTreePtr Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
!(argSingleUseNode->gtFlags & GTF_VAR_CLONED) &&
!inlArgInfo[argNum].argHasLdargaOp)
{
// 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);
/* Change the temp in-place to the actual argument */

argSingleUseNode->CopyFrom(inlArgInfo[argNum].argNode, this);
continue;
}
else
Expand Down
19 changes: 2 additions & 17 deletions src/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5313,14 +5313,12 @@ 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 @@ -6047,16 +6045,7 @@ GenTreeObj* Compiler::gtNewObjNode(CORINFO_CLASS_HANDLE structHnd, GenTree* addr
{
var_types nodeType = impNormStructType(structHnd);
assert(varTypeIsStruct(nodeType));
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;
return new (this, GT_OBJ) GenTreeObj(nodeType, addr, structHnd);
}

// Creates a new CpObj node.
Expand Down Expand Up @@ -14278,8 +14267,6 @@ 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 @@ -14293,8 +14280,6 @@ 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: 1 addition & 2 deletions src/jit/gentree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3329,8 +3329,7 @@ struct GenTreeObj: public GenTreeUnOp
GenTreeUnOp(GT_OBJ, type, addr),
gtClass(cls)
{
// By default, an OBJ is assumed to be a global reference.
gtFlags |= GTF_GLOB_REF;
gtFlags |= GTF_GLOB_REF; // An Obj is always a global reference.
}

#if DEBUGGABLE_GENTREE
Expand Down
27 changes: 12 additions & 15 deletions src/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,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 @@ -1512,10 +1512,9 @@ 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.
if (!lvaIsImplicitByRefLocal(structVal->AsLclVarCommon()->gtLclNum))
{
structObj->gtFlags &= ~GTF_GLOB_REF;
}
//
// TODO-CQ: Clear the GTF_GLOB_REF flag on structObj as well
// but this needs additional work when inlining.
}
else
{
Expand Down Expand Up @@ -16799,17 +16798,15 @@ 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 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 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 (!varTypeIsStruct(lclTyp) &&
(!inlArgInfo[lclNum].argHasSideEff) &&
if ((!inlArgInfo[lclNum].argHasSideEff) &&
(!inlArgInfo[lclNum].argHasGlobRef))
{
/* Get a *LARGE* LCL_VAR node */
Expand Down
48 changes: 14 additions & 34 deletions src/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2067,9 +2067,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.
// gtNewObjNode will set the GTF_EXCEPT flag if this is not a local stack object.
// Get a new Obj node temp to use it as a call argument
arg = gtNewObjNode(lvaGetStruct(tmpVarNum), arg);
arg->gtFlags |= GTF_EXCEPT;

#endif // not (_TARGET_AMD64_ or _TARGET_ARM64_)

Expand Down Expand Up @@ -3522,8 +3522,7 @@ 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] We should only be copying when roundupSize > originalSize.
// See also the TODO-CQ just before the call to fgMakeOutgoingStructArgCopy().
// TODO-X86-CQ: [1091733] Revisit for small structs, we should use push instruction
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 @@ -4238,6 +4237,8 @@ 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 @@ -8069,19 +8070,17 @@ GenTreePtr Compiler::fgMorphOneAsgBlockOp(GenTreePtr tree)
}
}

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

dest = gtNewOperNode(GT_IND, type, dest);

// 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.
/* 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 (!fgIsIndirOfAddrOfLocal(dest))
{
dest->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE);
}
dest->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE);

_DoneDest:;

Expand Down Expand Up @@ -8129,19 +8128,10 @@ _DoneDest:;
}
}

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

src = gtNewOperNode(GT_IND, type, src);

// 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);
}
src->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF | GTF_IND_TGTANYWHERE);

_DoneSrc:;

Expand Down Expand Up @@ -11452,16 +11442,6 @@ 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) && (curArgMask != RBM_NONE))
if (promotedStructLocal != NULL)
{
// 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