diff --git a/src/jit/compiler.hpp b/src/jit/compiler.hpp index 9b1054272274..d6894a5f69b2 100644 --- a/src/jit/compiler.hpp +++ b/src/jit/compiler.hpp @@ -1451,9 +1451,24 @@ inline void GenTree::ChangeOper(genTreeOps oper, ValueNumberUpdate vnUpdate) switch (oper) { case GT_LCL_FLD: + { + // The original GT_LCL_VAR might be annotated with a zeroOffset field. + FieldSeqNode* zeroFieldSeq = nullptr; + Compiler* compiler = JitTls::GetCompiler(); + bool isZeroOffset = compiler->GetZeroOffsetFieldMap()->Lookup(this, &zeroFieldSeq); + gtLclFld.gtLclOffs = 0; gtLclFld.gtFieldSeq = FieldSeqStore::NotAField(); + + if (zeroFieldSeq != nullptr) + { + // Set the zeroFieldSeq in the GT_LCL_FLD node + gtLclFld.gtFieldSeq = zeroFieldSeq; + // and remove the annotation from the ZeroOffsetFieldMap + compiler->GetZeroOffsetFieldMap()->Remove(this); + } break; + } default: break; } diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index 9e58db0df4fe..f0dfbc7ee5a6 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -7415,7 +7415,7 @@ GenTree* Compiler::gtCloneExpr( FieldSeqNode* fldSeq = nullptr; if (GetZeroOffsetFieldMap()->Lookup(tree, &fldSeq)) { - GetZeroOffsetFieldMap()->Set(copy, fldSeq); + fgAddFieldSeqForZeroOffset(copy, fldSeq); } } @@ -17628,6 +17628,9 @@ FieldSeqNode* FieldSeqStore::Append(FieldSeqNode* a, FieldSeqNode* b) } else { + // We should never add a duplicate FieldSeqNode + assert(a != b); + FieldSeqNode* tmp = Append(a->m_next, b); FieldSeqNode fsn(a->m_fieldHnd, tmp); FieldSeqNode* res = nullptr; diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index db6b6df5718f..2c57bd85c10f 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -1330,7 +1330,8 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, assert(OFFSETOF__CORINFO_TypedReference__dataPtr == 0); assert(destAddr->gtType == TYP_I_IMPL || destAddr->gtType == TYP_BYREF); - GetZeroOffsetFieldMap()->Set(destAddr, GetFieldSeqStore()->CreateSingleton(GetRefanyDataField())); + fgAddFieldSeqForZeroOffset(destAddr, GetFieldSeqStore()->CreateSingleton(GetRefanyDataField())); + GenTree* ptrSlot = gtNewOperNode(GT_IND, TYP_I_IMPL, destAddr); GenTreeIntCon* typeFieldOffset = gtNewIconNode(OFFSETOF__CORINFO_TypedReference__type, TYP_I_IMPL); typeFieldOffset->gtFieldSeq = GetFieldSeqStore()->CreateSingleton(GetRefanyTypeField()); diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index 6e4c8a836642..985fdb3ef7e6 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -5975,7 +5975,7 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) if (cnsOff == nullptr) // It must have folded into a zero offset { // Record in the general zero-offset map. - GetZeroOffsetFieldMap()->Set(addr, fieldSeq); + fgAddFieldSeqForZeroOffset(addr, fieldSeq); } else { @@ -6398,14 +6398,6 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) addr = gtNewLclvNode(lclNum, objRefType); // Use "tmpLcl" to create "addr" node. } - else if (fldOffset == 0) - { - // Generate the "addr" node. - addr = objRef; - FieldSeqNode* fieldSeq = - fieldMayOverlap ? FieldSeqStore::NotAField() : GetFieldSeqStore()->CreateSingleton(symHnd); - GetZeroOffsetFieldMap()->Set(addr, fieldSeq); - } else { addr = objRef; @@ -6647,19 +6639,42 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) } noway_assert(tree->gtOper == GT_IND); - // Pass down the current mac; if non null we are computing an address - GenTree* res = fgMorphSmpOp(tree, mac); - - if (fldOffset == 0 && res->OperGet() == GT_IND) + if (fldOffset == 0) { - GenTree* addr = res->gtOp.gtOp1; + GenTree* addr = tree->gtOp.gtOp1; + + // 'addr' may be a GT_COMMA. Skip over any comma nodes + addr = addr->gtEffectiveVal(); + +#ifdef DEBUG + if (verbose) + { + printf("\nBefore calling fgAddFieldSeqForZeroOffset:\n"); + gtDispTree(tree); + } +#endif + + // We expect 'addr' to be an address at this point. + assert(addr->TypeGet() == TYP_BYREF || addr->TypeGet() == TYP_I_IMPL); + // Since we don't make a constant zero to attach the field sequence to, associate it with the "addr" node. FieldSeqNode* fieldSeq = fieldMayOverlap ? FieldSeqStore::NotAField() : GetFieldSeqStore()->CreateSingleton(symHnd); fgAddFieldSeqForZeroOffset(addr, fieldSeq); } - return res; + // Pass down the current mac; if non null we are computing an address + GenTree* result = fgMorphSmpOp(tree, mac); + +#ifdef DEBUG + if (verbose) + { + printf("\nFinal value of Compiler::fgMorphField after calling fgMorphSmpOp:\n"); + gtDispTree(result); + } +#endif + + return result; } //------------------------------------------------------------------------------ @@ -12897,7 +12912,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) /* Negate the constant and change the node to be "+" */ op2->gtIntConCommon.SetIconValue(-op2->gtIntConCommon.IconValue()); - oper = GT_ADD; + op2->gtIntCon.gtFieldSeq = FieldSeqStore::NotAField(); + oper = GT_ADD; tree->ChangeOper(oper); goto CM_ADD_OP; } @@ -13462,23 +13478,38 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) // lclVar and must not extend beyond the end of the lclVar. if ((ival1 >= 0) && ((ival1 + genTypeSize(typ)) <= varSize)) { + GenTreeLclFld* lclFld; + // We will turn a GT_LCL_VAR into a GT_LCL_FLD with an gtLclOffs of 'ival' // or if we already have a GT_LCL_FLD we will adjust the gtLclOffs by adding 'ival' // Then we change the type of the GT_LCL_FLD to match the orginal GT_IND type. // if (temp->OperGet() == GT_LCL_FLD) { - temp->AsLclFld()->gtLclOffs += (unsigned short)ival1; - temp->AsLclFld()->gtFieldSeq = - GetFieldSeqStore()->Append(temp->AsLclFld()->gtFieldSeq, fieldSeq); + lclFld = temp->AsLclFld(); + lclFld->gtLclOffs += (unsigned short)ival1; + lclFld->gtFieldSeq = GetFieldSeqStore()->Append(lclFld->gtFieldSeq, fieldSeq); } - else + else // we have a GT_LCL_VAR { - temp->ChangeOper(GT_LCL_FLD); // Note that this makes the gtFieldSeq "NotAField"... - temp->AsLclFld()->gtLclOffs = (unsigned short)ival1; - if (fieldSeq != nullptr) - { // If it does represent a field, note that. - temp->AsLclFld()->gtFieldSeq = fieldSeq; + assert(temp->OperGet() == GT_LCL_VAR); + temp->ChangeOper(GT_LCL_FLD); // Note that this typically makes the gtFieldSeq "NotAField", + // unless there is a zero filed offset associated with 'temp'. + lclFld = temp->AsLclFld(); + lclFld->gtLclOffs = (unsigned short)ival1; + + if (lclFld->gtFieldSeq == FieldSeqStore::NotAField()) + { + if (fieldSeq != nullptr) + { + // If it does represent a field, note that. + lclFld->gtFieldSeq = fieldSeq; + } + } + else + { + // Append 'fieldSeq' to the existing one + lclFld->gtFieldSeq = GetFieldSeqStore()->Append(lclFld->gtFieldSeq, fieldSeq); } } temp->gtType = tree->gtType; @@ -13689,7 +13720,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) zeroFieldSeq = GetFieldSeqStore()->Append(existingZeroOffsetFldSeq, zeroFieldSeq); } // Transfer the annotation to the new GT_ADDR node. - GetZeroOffsetFieldMap()->Set(op1, zeroFieldSeq, NodeToFieldSeqMap::Overwrite); + fgAddFieldSeqForZeroOffset(op1, zeroFieldSeq); } commaNode->gtOp.gtOp2 = op1; // Originally, I gave all the comma nodes type "byref". But the ADDR(IND(x)) == x transform @@ -18703,66 +18734,122 @@ class LocalAddressVisitor final : public GenTreeVisitor } }; -void Compiler::fgAddFieldSeqForZeroOffset(GenTree* op1, FieldSeqNode* fieldSeq) +//------------------------------------------------------------------------ +// fgAddFieldSeqForZeroOffset: +// Associate a fieldSeq (with a zero offset) with the GenTree node 'addr' +// +// Arguments: +// addr - A GenTree node +// fieldSeqZero - a fieldSeq (with a zero offset) +// +// Notes: +// Some GenTree nodes have internal fields that record the field sequence. +// If we have one of these nodes: GT_CNS_INT, GT_LCL_FLD +// we can append the field sequence using the gtFieldSeq +// If we have a GT_ADD of a GT_CNS_INT we can use the +// fieldSeq from child node. +// Otherwise we record 'fieldSeqZero' in the GenTree node using +// a Map: GetFieldSeqStore() +// When doing so we take care to preserve any existing zero field sequence +// +void Compiler::fgAddFieldSeqForZeroOffset(GenTree* addr, FieldSeqNode* fieldSeqZero) { - assert(op1->TypeGet() == TYP_BYREF || op1->TypeGet() == TYP_I_IMPL || op1->TypeGet() == TYP_REF); + // We expect 'addr' to be an address at this point. + assert(addr->TypeGet() == TYP_BYREF || addr->TypeGet() == TYP_I_IMPL); - switch (op1->OperGet()) + FieldSeqNode* fieldSeqUpdate = fieldSeqZero; + GenTree* fieldSeqNode = addr; + bool fieldSeqRecorded = false; + bool isMapAnnotation = false; + +#ifdef DEBUG + if (verbose) { + printf("\nfgAddFieldSeqForZeroOffset for"); + gtDispFieldSeq(fieldSeqZero); + + printf("\naddr (Before)\n"); + gtDispNode(addr, nullptr, nullptr, false); + gtDispCommonEndLine(addr); + } +#endif // DEBUG + + switch (addr->OperGet()) + { + case GT_CNS_INT: + fieldSeqUpdate = GetFieldSeqStore()->Append(addr->gtIntCon.gtFieldSeq, fieldSeqZero); + addr->gtIntCon.gtFieldSeq = fieldSeqUpdate; + fieldSeqRecorded = true; + break; + + case GT_LCL_FLD: + { + GenTreeLclFld* lclFld = addr->AsLclFld(); + fieldSeqUpdate = GetFieldSeqStore()->Append(lclFld->gtFieldSeq, fieldSeqZero); + lclFld->gtFieldSeq = fieldSeqUpdate; + fieldSeqRecorded = true; + break; + } + case GT_ADDR: - if (op1->gtOp.gtOp1->OperGet() == GT_LCL_FLD) + if (addr->gtOp.gtOp1->OperGet() == GT_LCL_FLD) { - GenTreeLclFld* lclFld = op1->gtOp.gtOp1->AsLclFld(); - lclFld->gtFieldSeq = GetFieldSeqStore()->Append(lclFld->gtFieldSeq, fieldSeq); + fieldSeqNode = addr->gtOp.gtOp1; + + GenTreeLclFld* lclFld = addr->gtOp.gtOp1->AsLclFld(); + fieldSeqUpdate = GetFieldSeqStore()->Append(lclFld->gtFieldSeq, fieldSeqZero); + lclFld->gtFieldSeq = fieldSeqUpdate; + fieldSeqRecorded = true; } break; case GT_ADD: - if (op1->gtOp.gtOp1->OperGet() == GT_CNS_INT) + if (addr->gtOp.gtOp1->OperGet() == GT_CNS_INT) { - FieldSeqNode* op1Fs = op1->gtOp.gtOp1->gtIntCon.gtFieldSeq; - if (op1Fs != nullptr) - { - op1Fs = GetFieldSeqStore()->Append(op1Fs, fieldSeq); - op1->gtOp.gtOp1->gtIntCon.gtFieldSeq = op1Fs; - } + fieldSeqNode = addr->gtOp.gtOp1; + + fieldSeqUpdate = GetFieldSeqStore()->Append(addr->gtOp.gtOp1->gtIntCon.gtFieldSeq, fieldSeqZero); + addr->gtOp.gtOp1->gtIntCon.gtFieldSeq = fieldSeqUpdate; + fieldSeqRecorded = true; } - else if (op1->gtOp.gtOp2->OperGet() == GT_CNS_INT) + else if (addr->gtOp.gtOp2->OperGet() == GT_CNS_INT) { - FieldSeqNode* op2Fs = op1->gtOp.gtOp2->gtIntCon.gtFieldSeq; - if (op2Fs != nullptr) - { - op2Fs = GetFieldSeqStore()->Append(op2Fs, fieldSeq); - op1->gtOp.gtOp2->gtIntCon.gtFieldSeq = op2Fs; - } + fieldSeqNode = addr->gtOp.gtOp2; + + fieldSeqUpdate = GetFieldSeqStore()->Append(addr->gtOp.gtOp2->gtIntCon.gtFieldSeq, fieldSeqZero); + addr->gtOp.gtOp2->gtIntCon.gtFieldSeq = fieldSeqUpdate; + fieldSeqRecorded = true; } break; - case GT_CNS_INT: + default: + break; + } + + if (fieldSeqRecorded == false) + { + // Record in the general zero-offset map. + + // The "addr" node might already be annotated with a zero-offset field sequence. + FieldSeqNode* existingFieldSeq = nullptr; + if (GetZeroOffsetFieldMap()->Lookup(addr, &existingFieldSeq)) { - FieldSeqNode* op1Fs = op1->gtIntCon.gtFieldSeq; - if (op1Fs != nullptr) - { - op1Fs = GetFieldSeqStore()->Append(op1Fs, fieldSeq); - op1->gtIntCon.gtFieldSeq = op1Fs; - } + // Append the zero field sequences + fieldSeqUpdate = GetFieldSeqStore()->Append(existingFieldSeq, fieldSeqZero); } - break; - - default: - // Record in the general zero-offset map. + // Overwrite the field sequence annotation for op1 + GetZeroOffsetFieldMap()->Set(addr, fieldSeqUpdate, NodeToFieldSeqMap::Overwrite); + fieldSeqRecorded = true; + } - // The "op1" node might already be annotated with a zero-offset field sequence. - FieldSeqNode* existingZeroOffsetFldSeq = nullptr; - if (GetZeroOffsetFieldMap()->Lookup(op1, &existingZeroOffsetFldSeq)) - { - // Append the zero field sequences - fieldSeq = GetFieldSeqStore()->Append(existingZeroOffsetFldSeq, fieldSeq); - } - // Set the new field sequence annotation for op1 - GetZeroOffsetFieldMap()->Set(op1, fieldSeq, NodeToFieldSeqMap::Overwrite); - break; +#ifdef DEBUG + if (verbose) + { + printf(" (After)\n"); + gtDispNode(fieldSeqNode, nullptr, nullptr, false); + gtDispCommonEndLine(fieldSeqNode); } +#endif // DEBUG } //------------------------------------------------------------------------ diff --git a/src/jit/optcse.cpp b/src/jit/optcse.cpp index 31789328c3d1..9dc421b3fda4 100644 --- a/src/jit/optcse.cpp +++ b/src/jit/optcse.cpp @@ -2351,7 +2351,7 @@ class CSE_Heuristic // If it has a zero-offset field seq, copy annotation to the ref if (hasZeroMapAnnotation) { - m_pCompiler->GetZeroOffsetFieldMap()->Set(ref, fldSeq); + m_pCompiler->fgAddFieldSeqForZeroOffset(ref, fldSeq); } /* Create a comma node for the CSE assignment */ @@ -2392,7 +2392,7 @@ class CSE_Heuristic // If it has a zero-offset field seq, copy annotation. if (hasZeroMapAnnotation) { - m_pCompiler->GetZeroOffsetFieldMap()->Set(cse, fldSeq); + m_pCompiler->fgAddFieldSeqForZeroOffset(cse, fldSeq); } assert(m_pCompiler->fgRemoveRestOfBlock == false); diff --git a/src/jit/valuenum.cpp b/src/jit/valuenum.cpp index 03d6bac90083..a15a1cbef1aa 100644 --- a/src/jit/valuenum.cpp +++ b/src/jit/valuenum.cpp @@ -7510,17 +7510,6 @@ void Compiler::fgValueNumberTree(GenTree* tree) ValueNum arrVN = funcApp.m_args[1]; ValueNum inxVN = funcApp.m_args[2]; FieldSeqNode* fldSeq = vnStore->FieldSeqVNToFieldSeq(funcApp.m_args[3]); - - if (arg->gtOper != GT_LCL_VAR) - { - // Does the child of the GT_IND 'arg' have an associated zero-offset field sequence? - FieldSeqNode* addrFieldSeq = nullptr; - if (GetZeroOffsetFieldMap()->Lookup(arg, &addrFieldSeq)) - { - fldSeq = GetFieldSeqStore()->Append(addrFieldSeq, fldSeq); - } - } - #ifdef DEBUG if (verbose) {