Skip to content

Commit 41ab200

Browse files
SingleAccretionSingleAccretion
authored
Do not number partial definitions and ARGPLACE nodes (dotnet#64898)
* Refactor LCL_VAR/FLD numbering Do not number "partial definitions" - it is simply wasted work. * Don't create unnecessary unique VNs For ARGPLACE nodes, they will be overwritten when numbering the call. For nodes that do not produce values, they serve no purpose, and just waste memory. Also, delete RET_EXPR/FTN_ADDR handling. They will never appear in VN. * Fix printing for ASG(struct, CALL)
1 parent 537d607 commit 41ab200

File tree

1 file changed

+72
-141
lines changed

1 file changed

+72
-141
lines changed

‎src/coreclr/jit/valuenum.cpp‎

Lines changed: 72 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -8169,15 +8169,15 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
81698169
// Should not have been recorded as updating the GC heap.
81708170
assert(!GetMemorySsaMap(GcHeap)->Lookup(tree));
81718171

8172-
unsigned lhsLclNum = lclVarTree->GetLclNum();
8173-
unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree);
8172+
unsigned lhsLclNum = lclVarTree->GetLclNum();
8173+
unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree);
8174+
LclVarDsc* lhsVarDsc = lvaGetDesc(lhsLclNum);
81748175

81758176
// Ignore vars that we excluded from SSA (for example, because they're address-exposed). They don't have
81768177
// SSA names in which to store VN's on defs. We'll yield unique VN's when we read from them.
81778178
if (lclDefSsaNum != SsaConfig::RESERVED_SSA_NUM)
81788179
{
81798180
FieldSeqNode* lhsFldSeq = nullptr;
8180-
LclVarDsc* lhsVarDsc = lvaGetDesc(lhsLclNum);
81818181

81828182
if (lhs->IsLocalExpr(this, &lclVarTree, &lhsFldSeq))
81838183
{
@@ -8260,7 +8260,25 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
82608260
}
82618261
#endif // DEBUG
82628262
}
8263-
else if (lvaVarAddrExposed(lhsLclNum))
8263+
else if (lclVarTree->HasSsaName())
8264+
{
8265+
// The local wasn't in SSA, the tree is still an SSA def. There is only one
8266+
// case when this can happen - a promoted "CanBeReplacedWithItsField" struct.
8267+
assert((lhs == lclVarTree) && rhs->IsCall() && isEntire);
8268+
assert(lhsVarDsc->CanBeReplacedWithItsField(this));
8269+
// Give a new, unique, VN to the field.
8270+
LclVarDsc* fieldVarDsc = lvaGetDesc(lhsVarDsc->lvFieldLclStart);
8271+
LclSsaVarDsc* fieldVarSsaDsc = fieldVarDsc->GetPerSsaData(lclVarTree->GetSsaNum());
8272+
ValueNum newUniqueVN = vnStore->VNForExpr(compCurBB, fieldVarDsc->TypeGet());
8273+
8274+
fieldVarSsaDsc->m_vnPair.SetBoth(newUniqueVN);
8275+
8276+
JITDUMP("Tree [%06u] assigned VN to the only field V%02u/%u of promoted struct V%02u: new uniq ",
8277+
dspTreeID(tree), lhsVarDsc->lvFieldLclStart, lclVarTree->GetSsaNum(), lhsLclNum);
8278+
JITDUMPEXEC(vnPrint(newUniqueVN, 1));
8279+
JITDUMP("\n");
8280+
}
8281+
else if (lhsVarDsc->IsAddressExposed())
82648282
{
82658283
fgMutateAddressExposedLocal(tree DEBUGARG("INITBLK/COPYBLK - address-exposed local"));
82668284
}
@@ -8413,172 +8431,85 @@ void Compiler::fgValueNumberTree(GenTree* tree)
84138431
unsigned lclNum = lcl->GetLclNum();
84148432
LclVarDsc* varDsc = lvaGetDesc(lclNum);
84158433

8416-
if (varDsc->CanBeReplacedWithItsField(this))
8417-
{
8418-
lclNum = varDsc->lvFieldLclStart;
8419-
varDsc = lvaGetDesc(lclNum);
8420-
}
8421-
8422-
// Do we have a Use (read) of the LclVar?
8423-
//
8424-
if ((lcl->gtFlags & GTF_VAR_DEF) == 0 ||
8425-
(lcl->gtFlags & GTF_VAR_USEASG)) // If it is a "pure" def, will handled as part of the assignment.
8434+
// Do we have a Use (read) of the LclVar (defs will be handled at assignments)?
8435+
// Note that this a weak test, as we can have nodes under ADDRs that will be labeled as "uses".
8436+
if ((lcl->gtFlags & GTF_VAR_DEF) == 0)
84268437
{
8427-
bool generateUniqueVN = false;
8428-
FieldSeqNode* zeroOffsetFldSeq = nullptr;
8429-
8430-
// When we have a TYP_BYREF LclVar it can have a zero offset field sequence that needs to be added
8431-
if (typ == TYP_BYREF)
8432-
{
8433-
GetZeroOffsetFieldMap()->Lookup(tree, &zeroOffsetFldSeq);
8434-
}
8435-
8436-
if (varDsc->lvPromoted && varDsc->lvFieldCnt == 1)
8437-
{
8438-
// If the promoted var has only one field var, treat like a use of the field var.
8439-
lclNum = varDsc->lvFieldLclStart;
8440-
}
8441-
8442-
if (lcl->GetSsaNum() == SsaConfig::RESERVED_SSA_NUM)
8438+
if (lcl->HasSsaName())
84438439
{
8444-
// Not an SSA variable.
8440+
// We expect all uses of promoted structs to be replaced with uses of their fields.
8441+
assert(lvaInSsa(lclNum) && !varDsc->CanBeReplacedWithItsField(this));
84458442

8446-
if (lvaVarAddrExposed(lclNum))
8447-
{
8448-
// Address-exposed locals are part of ByrefExposed.
8449-
ValueNum addrVN = vnStore->VNForFunc(TYP_BYREF, VNF_PtrToLoc, vnStore->VNForIntCon(lclNum),
8450-
vnStore->VNForFieldSeq(nullptr));
8451-
ValueNum loadVN = fgValueNumberByrefExposedLoad(typ, addrVN);
8452-
8453-
lcl->gtVNPair.SetBoth(loadVN);
8454-
}
8455-
else
8456-
{
8457-
// Assign odd cases a new, unique, VN.
8458-
generateUniqueVN = true;
8459-
}
8460-
}
8461-
else
8462-
{
84638443
ValueNumPair wholeLclVarVNP = varDsc->GetPerSsaData(lcl->GetSsaNum())->m_vnPair;
8444+
assert(wholeLclVarVNP.BothDefined());
84648445

8465-
// Check for mismatched LclVar size
8466-
//
8467-
unsigned typSize = genTypeSize(genActualType(typ));
8468-
unsigned varSize = genTypeSize(genActualType(varDsc->TypeGet()));
8469-
8470-
if (typSize == varSize)
8471-
{
8472-
lcl->gtVNPair = wholeLclVarVNP;
8473-
}
8474-
else // mismatched LclVar definition and LclVar use size
8446+
// Account for type mismatches.
8447+
if (genActualType(varDsc) != genActualType(lcl))
84758448
{
8476-
if (typSize < varSize)
8449+
if (genTypeSize(varDsc) != genTypeSize(lcl))
84778450
{
8478-
// the indirection is reading less that the whole LclVar
8479-
// create a new VN that represent the partial value
8480-
//
8481-
ValueNumPair partialLclVarVNP =
8482-
vnStore->VNPairForCast(wholeLclVarVNP, typ, varDsc->TypeGet());
8483-
lcl->gtVNPair = partialLclVarVNP;
8451+
assert((varDsc->TypeGet() == TYP_LONG) && lcl->TypeIs(TYP_INT));
8452+
lcl->gtVNPair =
8453+
vnStore->VNPairForCast(wholeLclVarVNP, lcl->TypeGet(), varDsc->TypeGet());
84848454
}
84858455
else
84868456
{
8487-
assert(typSize > varSize);
8488-
// the indirection is reading beyond the end of the field
8489-
//
8490-
generateUniqueVN = true;
8457+
assert((varDsc->TypeGet() == TYP_I_IMPL) && lcl->TypeIs(TYP_BYREF));
8458+
lcl->gtVNPair = wholeLclVarVNP;
84918459
}
84928460
}
8461+
else
8462+
{
8463+
lcl->gtVNPair = wholeLclVarVNP;
8464+
}
84938465
}
8494-
8495-
if (!generateUniqueVN)
8466+
else if (varDsc->IsAddressExposed())
84968467
{
8497-
// There are a couple of cases where we haven't assigned a valid value number to 'lcl'
8498-
//
8499-
if (lcl->gtVNPair.GetLiberal() == ValueNumStore::NoVN)
8500-
{
8501-
#ifdef DEBUG
8502-
8503-
// So far, we know about three of these cases:
8504-
// Case 1) We have a local var who has never been defined but it's seen as a use.
8505-
// This is the case of storeIndir(addr(lclvar)) = expr. In this case since we only
8506-
// take the address of the variable, this doesn't mean it's a use nor we have to
8507-
// initialize it, so in this very rare case, we fabricate a value number;
8508-
// Case 2) Local variables that represent structs which are assigned using CpBlk;
8509-
// Case 3) Local variable was written using a partial write,
8510-
// for example, BLK<1>(ADDR(LCL_VAR int)) = 1, it will change only the first byte.
8511-
// Check that there was ld-addr-op on the local.
8512-
//
8513-
// Make sure we have one of these cases.
8514-
//
8515-
const GenTree* nextNode = lcl->gtNext;
8516-
const LclVarDsc* varDsc = lvaGetDesc(lcl);
8517-
8518-
assert((nextNode->gtOper == GT_ADDR && nextNode->AsOp()->gtOp1 == lcl) ||
8519-
varTypeIsStruct(lcl->TypeGet()) || varDsc->lvHasLdAddrOp);
8520-
#endif // DEBUG
8468+
// Address-exposed locals are part of ByrefExposed.
8469+
ValueNum addrVN = vnStore->VNForFunc(TYP_BYREF, VNF_PtrToLoc, vnStore->VNForIntCon(lclNum),
8470+
vnStore->VNForFieldSeq(nullptr));
8471+
ValueNum loadVN = fgValueNumberByrefExposedLoad(lcl->TypeGet(), addrVN);
85218472

8522-
// We will assign a unique value number for these
8523-
//
8524-
generateUniqueVN = true;
8525-
}
8473+
lcl->gtVNPair.SetBoth(loadVN);
8474+
}
8475+
else
8476+
{
8477+
// An untracked local, and other odd cases.
8478+
lcl->gtVNPair.SetBoth(vnStore->VNForExpr(compCurBB, lcl->TypeGet()));
85268479
}
85278480

8528-
if (!generateUniqueVN && (zeroOffsetFldSeq != nullptr))
8481+
// When we have a TYP_BYREF LclVar it can have a zero offset field sequence that needs to be added.
8482+
FieldSeqNode* zeroOffsetFldSeq = nullptr;
8483+
if ((typ == TYP_BYREF) && GetZeroOffsetFieldMap()->Lookup(tree, &zeroOffsetFldSeq))
85298484
{
85308485
ValueNum addrExtended = vnStore->ExtendPtrVN(lcl, zeroOffsetFldSeq);
85318486
if (addrExtended != ValueNumStore::NoVN)
85328487
{
8488+
assert(lcl->gtVNPair.BothEqual());
85338489
lcl->gtVNPair.SetBoth(addrExtended);
85348490
}
85358491
}
8536-
8537-
if (generateUniqueVN)
8538-
{
8539-
ValueNum uniqVN = vnStore->VNForExpr(compCurBB, lcl->TypeGet());
8540-
lcl->gtVNPair.SetBoth(uniqVN);
8541-
}
85428492
}
8543-
else if ((lcl->gtFlags & GTF_VAR_DEF) != 0)
8493+
else
85448494
{
8545-
// We have a Def (write) of the LclVar
8546-
8547-
// The below block ensures we give VNs to the fields of
8548-
// "CanBeReplacedWithItsField" struct locals. To the numbering
8549-
// of block assignments, those appear as untracked locals, but
8550-
// we need to give the SSA defs they represent a VN.
8551-
if (lcl->GetSsaNum() != SsaConfig::RESERVED_SSA_NUM)
8552-
{
8553-
ValueNum uniqVN = vnStore->VNForExpr(compCurBB, lcl->TypeGet());
8554-
varDsc->GetPerSsaData(lcl->GetSsaNum())->m_vnPair.SetBoth(uniqVN);
8555-
}
8495+
assert((lcl->gtFlags & GTF_VAR_DEF) != 0);
85568496

8557-
// Location nodes get VNForVoid (no exceptions needed).
8558-
lcl->gtVNPair = vnStore->VNPForVoid();
8497+
// Give location nodes VNForVoid. Note that for non-struct
8498+
// locals, this will get overriden in "fgValueNumberAssignment",
8499+
// because some code (notably copy propagation) depends on that.
8500+
// TODO-Cleanup: get rid of this dependency.
8501+
lcl->SetVNs(vnStore->VNPForVoid());
85598502
}
85608503
}
85618504
break;
85628505

8563-
case GT_FTN_ADDR:
8564-
// Use the value of the function pointer (actually, a method handle.)
8565-
tree->gtVNPair.SetBoth(
8566-
vnStore->VNForHandle(ssize_t(tree->AsFptrVal()->gtFptrMethod), GTF_ICON_METHOD_HDL));
8567-
break;
8568-
8569-
// This group passes through a value from a child node.
8570-
case GT_RET_EXPR:
8571-
tree->SetVNsFromNode(tree->AsRetExpr()->gtInlineCandidate);
8572-
break;
8573-
85748506
case GT_LCL_FLD:
85758507
{
85768508
GenTreeLclFld* lclFld = tree->AsLclFld();
85778509
assert(!lvaInSsa(lclFld->GetLclNum()) || (lclFld->GetFieldSeq() != nullptr));
8578-
// If this is a (full) def, then the variable will be labeled with the new SSA number,
8579-
// which will not have a value. We skip; it will be handled by one of the assignment-like
8580-
// forms (assignment, or initBlk or copyBlk).
8581-
if (((lclFld->gtFlags & GTF_VAR_DEF) == 0) || (lclFld->gtFlags & GTF_VAR_USEASG))
8510+
8511+
// If this is a (full or partial) def we skip; it will be handled as part of the assignment.
8512+
if ((lclFld->gtFlags & GTF_VAR_DEF) == 0)
85828513
{
85838514
unsigned ssaNum = lclFld->GetSsaNum();
85848515
LclVarDsc* varDsc = lvaGetDesc(lclFld);
@@ -8616,7 +8547,6 @@ void Compiler::fgValueNumberTree(GenTree* tree)
86168547
}
86178548
break;
86188549

8619-
// The ones below here all get a new unique VN -- but for various reasons, explained after each.
86208550
case GT_CATCH_ARG:
86218551
// We know nothing about the value of a caught expression.
86228552
tree->gtVNPair.SetBoth(vnStore->VNForExpr(compCurBB, tree->TypeGet()));
@@ -8687,6 +8617,7 @@ void Compiler::fgValueNumberTree(GenTree* tree)
86878617
case GT_MEMORYBARRIER: // Leaf
86888618
// For MEMORYBARRIER add an arbitrary side effect on GcHeap/ByrefExposed.
86898619
fgMutateGcHeap(tree DEBUGARG("MEMORYBARRIER"));
8620+
tree->gtVNPair = vnStore->VNPForVoid();
86908621
break;
86918622

86928623
// These do not represent values.
@@ -8696,19 +8627,19 @@ void Compiler::fgValueNumberTree(GenTree* tree)
86968627
#if !defined(FEATURE_EH_FUNCLETS)
86978628
case GT_END_LFIN: // Control flow
86988629
#endif
8630+
tree->gtVNPair = vnStore->VNPForVoid();
8631+
break;
8632+
86998633
case GT_ARGPLACE:
87008634
// This node is a standin for an argument whose value will be computed later. (Perhaps it's
87018635
// a register argument, and we don't want to preclude use of the register in arg evaluation yet.)
8702-
// We give this a "fake" value number now; if the call in which it occurs cares about the
8703-
// value (e.g., it's a helper call whose result is a function of argument values) we'll reset
8704-
// this later, when the later args have been assigned VNs.
8705-
tree->gtVNPair.SetBoth(vnStore->VNForExpr(compCurBB, tree->TypeGet()));
8636+
// We defer giving this a value number now; we'll reset it later, when numbering the call.
87068637
break;
87078638

87088639
case GT_PHI_ARG:
87098640
// This one is special because we should never process it in this method: it should
87108641
// always be taken care of, when needed, during pre-processing of a blocks phi definitions.
8711-
assert(false);
8642+
assert(!"PHI_ARG in fgValueNumberTree");
87128643
break;
87138644

87148645
default:

0 commit comments

Comments
 (0)