From 779de9bfd7520cf24f3ed6599c26172474e68141 Mon Sep 17 00:00:00 2001 From: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> Date: Fri, 1 Apr 2022 22:35:27 +0300 Subject: [PATCH] Use a simpler representation for array addresses after morph (#64581) * Do not copy flags in ADDR/COMMA opt We only need to update the side effects, and gtUpdateNodeSideEffects already takes care of that. Will avoid copying GTF_NO_CSE unnecessarily. No diffs. * Add GT_ARR_ADDR * Generate ARR_ADDR in morph * gtSetEvalOrder tuning * Delete the ArrayInfo map * Delete GTF_IND_ARR_INDEX * Make ARR_ADDR non-null This preserves the non-faultness annotation for the parent indir when ADDR(IND(ARR_ADDR)) -> ARR_ADDR folding happends. * Simplify the array address parsing We don't need the field sequence or the complex parsing. A few diffs because previos code didn't handle COMMAs. (The new code doesn't either, but they are skipped automatically by the optComputeLoopSideEffectsOfBlock). * Clean ParseArrayAddress up Move it to GenTreeArrAddr, stop using ArrayInfo. * More ParseArrayAddress cleanup FldSeq no loger expected or needed. We will be attaching it to ARR_ADDR directly in the future. * Delete index labeling And associated code. No longer needed, and we will use a different mechanism for ARR_ADDR. No diffs. * Delete the last pseudo-field No longer needed... * Delete ArrayInfo * Fix formatting --- src/coreclr/jit/assertionprop.cpp | 11 - src/coreclr/jit/compiler.cpp | 13 +- src/coreclr/jit/compiler.h | 74 +--- src/coreclr/jit/compiler.hpp | 17 +- src/coreclr/jit/compmemkind.h | 2 +- src/coreclr/jit/earlyprop.cpp | 10 - src/coreclr/jit/fgdiagnostic.cpp | 3 +- src/coreclr/jit/flowgraph.cpp | 4 + src/coreclr/jit/gentree.cpp | 589 ++++++++---------------------- src/coreclr/jit/gentree.h | 152 ++++---- src/coreclr/jit/gtlist.h | 1 + src/coreclr/jit/gtstructs.h | 1 + src/coreclr/jit/lclmorph.cpp | 14 +- src/coreclr/jit/morph.cpp | 248 ++----------- src/coreclr/jit/morphblock.cpp | 3 - src/coreclr/jit/objectalloc.cpp | 14 +- src/coreclr/jit/optimizer.cpp | 76 ++-- src/coreclr/jit/rationalize.cpp | 3 +- src/coreclr/jit/valuenum.cpp | 169 +++------ 19 files changed, 384 insertions(+), 1020 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 1e5d32e1f4884..ad127b334d007 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3225,19 +3225,8 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, } else { - bool isArrIndex = ((tree->gtFlags & GTF_VAR_ARR_INDEX) != 0); - assert(varTypeIsIntegralOrI(tree)); - newTree->BashToConst(curAssertion->op2.u1.iconVal, genActualType(tree)); - - // If we're doing an array index address, assume any constant propagated contributes to the index. - if (isArrIndex) - { - newTree->AsIntCon()->gtFieldSeq = - GetFieldSeqStore()->CreateSingleton(FieldSeqStore::ConstantIndexPseudoField); - } - newTree->gtFlags &= ~GTF_VAR_ARR_INDEX; } break; diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index d577b1cfe4242..5d7b518aadd4f 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -1916,7 +1916,6 @@ void Compiler::compInit(ArenaAllocator* pAlloc, m_blockToEHPreds = nullptr; m_fieldSeqStore = nullptr; m_zeroOffsetFieldMap = nullptr; - m_arrayInfoMap = nullptr; m_refAnyClass = nullptr; for (MemoryKind memoryKind : allMemoryKinds()) { @@ -9161,10 +9160,6 @@ void cTreeFlags(Compiler* comp, GenTree* tree) { chars += printf("[VAR_DEATH]"); } - if (tree->gtFlags & GTF_VAR_ARR_INDEX) - { - chars += printf("[VAR_ARR_INDEX]"); - } #if defined(DEBUG) if (tree->gtDebugFlags & GTF_DEBUG_VAR_CSE_REF) { @@ -9310,8 +9305,14 @@ void cTreeFlags(Compiler* comp, GenTree* tree) } break; - case GT_CNS_INT: + case GT_ARR_ADDR: + if (tree->gtFlags & GTF_ARR_ADDR_NONNULL) + { + chars += printf("[ARR_ADDR_NONNULL]"); + } + break; + case GT_CNS_INT: { GenTreeFlags handleKind = (tree->gtFlags & GTF_ICON_HDL_MASK); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 427796912e528..4daea0334752c 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1377,27 +1377,6 @@ class LinearScanInterface LinearScanInterface* getLinearScanAllocator(Compiler* comp); -// Information about arrays: their element type and size, and the offset of the first element. -// We label GT_IND's that are array indices with GTF_IND_ARR_INDEX, and, for such nodes, -// associate an array info via the map retrieved by GetArrayInfoMap(). This information is used, -// for example, in value numbering of array index expressions. -struct ArrayInfo -{ - var_types m_elemType; - CORINFO_CLASS_HANDLE m_elemStructType; - unsigned m_elemSize; - unsigned m_elemOffset; - - ArrayInfo() : m_elemType(TYP_UNDEF), m_elemStructType(nullptr), m_elemSize(0), m_elemOffset(0) - { - } - - ArrayInfo(var_types elemType, unsigned elemSize, unsigned elemOffset, CORINFO_CLASS_HANDLE elemStructType) - : m_elemType(elemType), m_elemStructType(elemStructType), m_elemSize(elemSize), m_elemOffset(elemOffset) - { - } -}; - // This enumeration names the phases into which we divide compilation. The phases should completely // partition a compilation. enum Phases @@ -5604,6 +5583,8 @@ class Compiler // Does value-numbering for an intrinsic tree. void fgValueNumberIntrinsic(GenTree* tree); + void fgValueNumberArrIndexAddr(GenTreeArrAddr* arrAddr); + #ifdef FEATURE_SIMD // Does value-numbering for a GT_SIMD tree void fgValueNumberSimd(GenTreeSIMD* tree); @@ -11220,52 +11201,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX // CoreRT. Such case is handled same as the default case. void fgAddFieldSeqForZeroOffset(GenTree* op1, FieldSeqNode* fieldSeq); - typedef JitHashTable, ArrayInfo> NodeToArrayInfoMap; - NodeToArrayInfoMap* m_arrayInfoMap; - - NodeToArrayInfoMap* GetArrayInfoMap() - { - Compiler* compRoot = impInlineRoot(); - if (compRoot->m_arrayInfoMap == nullptr) - { - // Create a CompAllocator that labels sub-structure with CMK_ArrayInfoMap, and use that for allocation. - CompAllocator ialloc(getAllocator(CMK_ArrayInfoMap)); - compRoot->m_arrayInfoMap = new (ialloc) NodeToArrayInfoMap(ialloc); - } - return compRoot->m_arrayInfoMap; - } - - //----------------------------------------------------------------------------------------------------------------- - // Compiler::TryGetArrayInfo: - // Given an indirection node, checks to see whether or not that indirection represents an array access, and - // if so returns information about the array. - // - // Arguments: - // indir - The `GT_IND` node. - // arrayInfo (out) - Information about the accessed array if this function returns true. Undefined otherwise. - // - // Returns: - // True if the `GT_IND` node represents an array access; false otherwise. - bool TryGetArrayInfo(GenTreeIndir* indir, ArrayInfo* arrayInfo) - { - if ((indir->gtFlags & GTF_IND_ARR_INDEX) == 0) - { - return false; - } - - if (indir->gtOp1->OperIs(GT_INDEX_ADDR)) - { - GenTreeIndexAddr* const indexAddr = indir->gtOp1->AsIndexAddr(); - *arrayInfo = ArrayInfo(indexAddr->gtElemType, indexAddr->gtElemSize, indexAddr->gtElemOffset, - indexAddr->gtStructElemClass); - return true; - } - - bool found = GetArrayInfoMap()->Lookup(indir, arrayInfo); - assert(found); - return true; - } - NodeToUnsignedMap* m_memorySsaMap[MemoryKindCount]; // In some cases, we want to assign intermediate SSA #'s to memory states, and know what nodes create those memory @@ -11283,8 +11218,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Compiler* compRoot = impInlineRoot(); if (compRoot->m_memorySsaMap[memoryKind] == nullptr) { - // Create a CompAllocator that labels sub-structure with CMK_ArrayInfoMap, and use that for allocation. - CompAllocator ialloc(getAllocator(CMK_ArrayInfoMap)); + // Create a CompAllocator that labels sub-structure with CMK_MemorySsaMap, and use that for allocation. + CompAllocator ialloc(getAllocator(CMK_MemorySsaMap)); compRoot->m_memorySsaMap[memoryKind] = new (ialloc) NodeToUnsignedMap(ialloc); } return compRoot->m_memorySsaMap[memoryKind]; @@ -11552,6 +11487,7 @@ class GenTreeVisitor case GT_RETURN: case GT_RETFILT: case GT_RUNTIMELOOKUP: + case GT_ARR_ADDR: case GT_KEEPALIVE: case GT_INC_SATURATE: { diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index c0a08412a10ad..882685d8bb67b 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -844,19 +844,14 @@ inline GenTree* Compiler::gtNewOperNode(genTreeOps oper, var_types type, GenTree if (doSimplifications) { - // We do some simplifications here. - // If this gets to be too many, try a switch... - // TODO-Cleanup: With the factoring out of array bounds checks, it should not be the - // case that we need to check for the array index case here, but without this check - // we get failures (see for example jit\Directed\Languages\Python\test_methods_d.exe) + // We do some simplifications here. If this gets to be too many, try a switch... if (oper == GT_IND) { // IND(ADDR(IND(x)) == IND(x) - if (op1->gtOper == GT_ADDR) + if (op1->OperIs(GT_ADDR)) { - GenTreeUnOp* addr = op1->AsUnOp(); - GenTree* indir = addr->gtGetOp1(); - if (indir->OperIs(GT_IND) && ((indir->gtFlags & GTF_IND_ARR_INDEX) == 0)) + GenTree* indir = op1->AsUnOp()->gtGetOp1(); + if (indir->OperIs(GT_IND)) { op1 = indir->AsIndir()->Addr(); } @@ -864,8 +859,7 @@ inline GenTree* Compiler::gtNewOperNode(genTreeOps oper, var_types type, GenTree } else if (oper == GT_ADDR) { - // if "x" is not an array index, ADDR(IND(x)) == x - if (op1->gtOper == GT_IND && (op1->gtFlags & GTF_IND_ARR_INDEX) == 0) + if (op1->OperIs(GT_IND)) { return op1->AsOp()->gtOp1; } @@ -4231,6 +4225,7 @@ void GenTree::VisitOperands(TVisitor visitor) case GT_ALLOCOBJ: case GT_INIT_VAL: case GT_RUNTIMELOOKUP: + case GT_ARR_ADDR: case GT_JTRUE: case GT_SWITCH: case GT_NULLCHECK: diff --git a/src/coreclr/jit/compmemkind.h b/src/coreclr/jit/compmemkind.h index 969b0c3dc2a7b..8810d4dec21d7 100644 --- a/src/coreclr/jit/compmemkind.h +++ b/src/coreclr/jit/compmemkind.h @@ -36,7 +36,7 @@ CompMemKindMacro(Generic) CompMemKindMacro(LocalAddressVisitor) CompMemKindMacro(FieldSeqStore) CompMemKindMacro(ZeroOffsetFieldMap) -CompMemKindMacro(ArrayInfoMap) +CompMemKindMacro(MemorySsaMap) CompMemKindMacro(MemoryPhiArg) CompMemKindMacro(CSE) CompMemKindMacro(GC) diff --git a/src/coreclr/jit/earlyprop.cpp b/src/coreclr/jit/earlyprop.cpp index 283c9c8901ae7..f09a2ffb68185 100644 --- a/src/coreclr/jit/earlyprop.cpp +++ b/src/coreclr/jit/earlyprop.cpp @@ -344,16 +344,6 @@ GenTree* Compiler::optEarlyPropRewriteTree(GenTree* tree, LocalNumberToNullCheck actualValClone->gtType = tree->gtType; } - // Propagating a constant into an array index expression requires calling - // LabelIndex to update the FieldSeq annotations. EarlyProp may replace - // array length expressions with constants, so check if this is an array - // length operator that is part of an array index expression. - bool isIndexExpr = (tree->OperGet() == GT_ARR_LENGTH && ((tree->gtFlags & GTF_ARRLEN_ARR_IDX) != 0)); - if (isIndexExpr) - { - actualValClone->LabelIndex(this); - } - // actualValClone has small tree node size, it is safe to use CopyFrom here. tree->ReplaceWith(actualValClone, this); diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 0e4f7ec3ec487..066ef8bfae197 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -2951,8 +2951,7 @@ void Compiler::fgDebugCheckFlags(GenTree* tree) expectedFlags |= GTF_CALL; } - // We reuse GTF_REVERSE_OPS as GTF_VAR_ARR_INDEX for LCL_VAR nodes. - if (((tree->gtFlags & GTF_REVERSE_OPS) != 0) && !tree->OperIs(GT_LCL_VAR)) + if ((tree->gtFlags & GTF_REVERSE_OPS) != 0) { assert(tree->OperSupportsReverseOpEvalOrder(this)); } diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index c63cf49f5e27d..349a1419c7ea7 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -948,6 +948,10 @@ bool Compiler::fgAddrCouldBeNull(GenTree* addr) { return false; } + else if (addr->OperIs(GT_ARR_ADDR)) + { + return (addr->gtFlags & GTF_ARR_ADDR_NONNULL) == 0; + } else if (addr->gtOper == GT_LCL_VAR) { unsigned varNum = addr->AsLclVarCommon()->GetLclNum(); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 5d921014cb58d..454fd39956652 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -406,14 +406,6 @@ void GenTree::ReplaceWith(GenTree* src, Compiler* comp) #ifdef DEBUG gtSeqNum = 0; #endif - // Transfer any annotations. - if (src->OperGet() == GT_IND && src->gtFlags & GTF_IND_ARR_INDEX) - { - ArrayInfo arrInfo; - bool b = comp->GetArrayInfoMap()->Lookup(src, &arrInfo); - assert(b); - comp->GetArrayInfoMap()->Set(this, arrInfo); - } DEBUG_DESTROY_NODE(src); } @@ -1617,6 +1609,7 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK) // For the ones below no extra argument matters for comparison. case GT_BOX: case GT_RUNTIMELOOKUP: + case GT_ARR_ADDR: break; default: @@ -2059,6 +2052,7 @@ unsigned Compiler::gtHashValue(GenTree* tree) // For the ones below no extra argument matters for comparison. case GT_BOX: + case GT_ARR_ADDR: break; default: @@ -3991,6 +3985,18 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) costSz = 2 * 2; break; + case GT_ARR_ADDR: + costEx = 0; + costSz = 0; + + // To preserve previous behavior, we will always use "gtMarkAddrMode" for ARR_ADDR. + if (op1->OperIs(GT_ADD) && gtMarkAddrMode(op1, &costEx, &costSz, tree->AsArrAddr()->GetElemType())) + { + op1->SetCosts(costEx, costSz); + goto DONE; + } + break; + case GT_BLK: case GT_IND: @@ -4036,28 +4042,22 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) GenTree* addr = op1->gtEffectiveVal(); bool doAddrMode = true; - // See if we can form a complex addressing mode. - // Always use an addrMode for an array index indirection. - // TODO-1stClassStructs: Always do this, but first make sure it's - // done in Lowering as well. - if ((tree->gtFlags & GTF_IND_ARR_INDEX) == 0) + // TODO-1stClassStructs: Always do this, but first make sure it's done in Lowering as well. + if (tree->TypeGet() == TYP_STRUCT) { - if (tree->TypeGet() == TYP_STRUCT) - { - doAddrMode = false; - } - else if (varTypeIsStruct(tree)) + doAddrMode = false; + } + else if (varTypeIsStruct(tree)) + { + // This is a heuristic attempting to match prior behavior when indirections + // under a struct assignment would not be considered for addressing modes. + if (compCurStmt != nullptr) { - // This is a heuristic attempting to match prior behavior when indirections - // under a struct assignment would not be considered for addressing modes. - if (compCurStmt != nullptr) + GenTree* expr = compCurStmt->GetRootNode(); + if ((expr->OperGet() == GT_ASG) && + ((expr->gtGetOp1() == tree) || (expr->gtGetOp2() == tree))) { - GenTree* expr = compCurStmt->GetRootNode(); - if ((expr->OperGet() == GT_ASG) && - ((expr->gtGetOp1() == tree) || (expr->gtGetOp2() == tree))) - { - doAddrMode = false; - } + doAddrMode = false; } } } @@ -5028,6 +5028,7 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse) case GT_BOX: case GT_ALLOCOBJ: case GT_RUNTIMELOOKUP: + case GT_ARR_ADDR: case GT_INIT_VAL: case GT_JTRUE: case GT_SWITCH: @@ -7427,10 +7428,6 @@ GenTree* Compiler::gtCloneExpr( if (tree->AsLclVarCommon()->GetLclNum() == varNum) { copy = gtNewIconNode(varVal, tree->gtType); - if (tree->gtFlags & GTF_VAR_ARR_INDEX) - { - copy->LabelIndex(this); - } } else { @@ -7606,15 +7603,19 @@ GenTree* Compiler::gtCloneExpr( } break; + case GT_ARR_ADDR: + copy = new (this, GT_ARR_ADDR) + GenTreeArrAddr(tree->AsArrAddr()->Addr(), tree->AsArrAddr()->GetElemType(), + tree->AsArrAddr()->GetElemClassHandle(), tree->AsArrAddr()->GetFirstElemOffset()); + break; + case GT_ARR_LENGTH: copy = gtNewArrLen(tree->TypeGet(), tree->AsOp()->gtOp1, tree->AsArrLen()->ArrLenOffset(), nullptr); break; case GT_ARR_INDEX: copy = new (this, GT_ARR_INDEX) - GenTreeArrIndex(tree->TypeGet(), - gtCloneExpr(tree->AsArrIndex()->ArrObj(), addFlags, deepVarNum, deepVarVal), - gtCloneExpr(tree->AsArrIndex()->IndexExpr(), addFlags, deepVarNum, deepVarVal), + GenTreeArrIndex(tree->TypeGet(), tree->AsArrIndex()->ArrObj(), tree->AsArrIndex()->IndexExpr(), tree->AsArrIndex()->gtCurrDim, tree->AsArrIndex()->gtArrRank, tree->AsArrIndex()->gtArrElemType); break; @@ -7723,26 +7724,6 @@ GenTree* Compiler::gtCloneExpr( /* Flags */ addFlags |= tree->gtFlags; - // Copy any node annotations, if necessary. - switch (tree->gtOper) - { - case GT_STOREIND: - case GT_IND: - case GT_OBJ: - case GT_STORE_OBJ: - { - ArrayInfo arrInfo; - if (!tree->AsIndir()->gtOp1->OperIs(GT_INDEX_ADDR) && TryGetArrayInfo(tree->AsIndir(), &arrInfo)) - { - GetArrayInfoMap()->Set(copy, arrInfo); - } - } - break; - - default: - break; - } - #ifdef DEBUG /* GTF_NODE_MASK should not be propagated from 'tree' to 'copy' */ addFlags &= ~GTF_NODE_MASK; @@ -8480,6 +8461,7 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node) case GT_BOX: case GT_ALLOCOBJ: case GT_RUNTIMELOOKUP: + case GT_ARR_ADDR: case GT_INIT_VAL: case GT_JTRUE: case GT_SWITCH: @@ -9481,12 +9463,6 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ --msgLength; break; } - if (tree->gtFlags & GTF_IND_ARR_INDEX) - { - printf("a"); - --msgLength; - break; - } if (tree->gtFlags & GTF_IND_NONFAULTING) { printf("n"); // print a n for non-faulting @@ -9624,12 +9600,6 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ --msgLength; break; } - if (tree->gtFlags & GTF_VAR_ARR_INDEX) - { - printf("i"); - --msgLength; - break; - } if (tree->gtFlags & GTF_VAR_CONTEXT) { printf("!"); @@ -9717,7 +9687,7 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ else // !(tree->OperIsBinary() || tree->OperIsMultiOp()) { // the GTF_REVERSE flag only applies to binary operations (which some MultiOp nodes are). - flags &= ~GTF_REVERSE_OPS; // we use this value for GTF_VAR_ARR_INDEX above + flags &= ~GTF_REVERSE_OPS; } msgLength -= GenTree::gtDispFlags(flags, tree->gtDebugFlags); @@ -9821,22 +9791,6 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ printf("<%S, %u>", shortClassName, classSize); } } - else if (tree->OperIsIndir()) - { - ArrayInfo arrInfo; - if (TryGetArrayInfo(tree->AsIndir(), &arrInfo)) - { - if (varTypeIsStruct(arrInfo.m_elemType)) - { - CORINFO_CLASS_HANDLE clsHnd = arrInfo.m_elemStructType; - // We could create a layout with `typGetObjLayout(asInd->gtStructElemClass)` but we - // don't want to affect the layout table. - const unsigned classSize = info.compCompHnd->getClassSize(clsHnd); - const char16_t* shortClassName = eeGetShortClassName(clsHnd); - printf("<%S, %u>", shortClassName, classSize); - } - } - } if (layout != nullptr) { @@ -9844,6 +9798,18 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ } } + if (tree->OperIs(GT_ARR_ADDR)) + { + if (tree->AsArrAddr()->GetElemClassHandle() != NO_CLASS_HANDLE) + { + printf("%S[]", eeGetShortClassName(tree->AsArrAddr()->GetElemClassHandle())); + } + else + { + printf("%s[]", varTypeName(tree->AsArrAddr()->GetElemType())); + } + } + if (tree->gtOper == GT_LCL_VAR || tree->gtOper == GT_STORE_LCL_VAR) { LclVarDsc* varDsc = lvaGetDesc(tree->AsLclVarCommon()); @@ -10480,21 +10446,12 @@ void Compiler::gtDispFieldSeq(FieldSeqNode* pfsn) while (pfsn != nullptr) { assert(pfsn != FieldSeqStore::NotAField()); // Can't exist in a field sequence list except alone - CORINFO_FIELD_HANDLE fldHnd = pfsn->GetFieldHandleValue(); - // First check the "pseudo" field handles... - if (fldHnd == FieldSeqStore::FirstElemPseudoField) - { - printf("#FirstElem"); - } - else if (fldHnd == FieldSeqStore::ConstantIndexPseudoField) - { - printf("#ConstantIndex"); - } - else - { - printf("%s", eeGetFieldName(fldHnd)); - } + + CORINFO_FIELD_HANDLE fldHnd = pfsn->GetFieldHandle(); + printf("%s", eeGetFieldName(fldHnd)); + pfsn = pfsn->GetNext(); + if (pfsn != nullptr) { printf(", "); @@ -10968,14 +10925,7 @@ void Compiler::gtDispTree(GenTree* tree, if (tree->OperIs(GT_FIELD)) { - if (FieldSeqStore::IsPseudoField(tree->AsField()->gtFldHnd)) - { - printf(" #PseudoField:0x%x", tree->AsField()->gtFldOffset); - } - else - { - printf(" %s", eeGetFieldName(tree->AsField()->gtFldHnd), 0); - } + printf(" %s", eeGetFieldName(tree->AsField()->gtFldHnd), 0); } if (tree->gtOper == GT_INTRINSIC) @@ -13386,13 +13336,6 @@ GenTree* Compiler::gtFoldExprConst(GenTree* tree) i1 = (INT32)op1->AsIntCon()->IconValue(); - // If we fold a unary oper, then the folded constant - // is considered a ConstantIndexField if op1 was one. - if ((op1->AsIntCon()->gtFieldSeq != nullptr) && op1->AsIntCon()->gtFieldSeq->IsConstantIndexFieldSeq()) - { - fieldSeq = op1->AsIntCon()->gtFieldSeq; - } - switch (tree->OperGet()) { case GT_NOT: @@ -13885,22 +13828,6 @@ GenTree* Compiler::gtFoldExprConst(GenTree* tree) { goto INTEGRAL_OVF; } - - // For the very particular case of the "constant array index" pseudo-field, we - // assume that multiplication is by the field width, and preserves that field. - // This could obviously be made more robust by a more complicated set of annotations... - if ((op1->AsIntCon()->gtFieldSeq != nullptr) && - op1->AsIntCon()->gtFieldSeq->IsConstantIndexFieldSeq()) - { - assert(op2->AsIntCon()->gtFieldSeq == FieldSeqStore::NotAField()); - fieldSeq = op1->AsIntCon()->gtFieldSeq; - } - else if ((op2->AsIntCon()->gtFieldSeq != nullptr) && - op2->AsIntCon()->gtFieldSeq->IsConstantIndexFieldSeq()) - { - assert(op1->AsIntCon()->gtFieldSeq == FieldSeqStore::NotAField()); - fieldSeq = op2->AsIntCon()->gtFieldSeq; - } i1 = itemp; break; @@ -16268,7 +16195,7 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pF assert((fldSeq != nullptr) && (baseAddr != nullptr)); - if ((fldSeq == FieldSeqStore::NotAField()) || fldSeq->IsPseudoField()) + if (fldSeq == FieldSeqStore::NotAField()) { return false; } @@ -16635,33 +16562,29 @@ CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleIfPresent(GenTree* tree) { // Attempt to find a handle for this expression. // We can do this for an array element indirection, or for a field indirection. - ArrayInfo arrInfo; - if (TryGetArrayInfo(tree->AsIndir(), &arrInfo)) + GenTree* addr = tree->AsIndir()->Addr(); + if (addr->OperIs(GT_ARR_ADDR)) + { + structHnd = addr->AsArrAddr()->GetElemClassHandle(); + break; + } + + FieldSeqNode* fieldSeq = nullptr; + if ((addr->OperGet() == GT_ADD) && addr->gtGetOp2()->OperIs(GT_CNS_INT)) { - structHnd = arrInfo.m_elemStructType; + fieldSeq = addr->gtGetOp2()->AsIntCon()->gtFieldSeq; } else { - GenTree* addr = tree->AsIndir()->Addr(); - FieldSeqNode* fieldSeq = nullptr; - if ((addr->OperGet() == GT_ADD) && addr->gtGetOp2()->OperIs(GT_CNS_INT)) - { - fieldSeq = addr->gtGetOp2()->AsIntCon()->gtFieldSeq; - } - else - { - GetZeroOffsetFieldMap()->Lookup(addr, &fieldSeq); - } - if (fieldSeq != nullptr) - { - fieldSeq = fieldSeq->GetTail(); + GetZeroOffsetFieldMap()->Lookup(addr, &fieldSeq); + } - if (fieldSeq != FieldSeqStore::NotAField() && !fieldSeq->IsPseudoField()) - { - // Note we may have a primitive here (and correctly fail to obtain the handle) - eeGetFieldType(fieldSeq->GetFieldHandle(), &structHnd); - } - } + if ((fieldSeq != nullptr) && (fieldSeq != FieldSeqStore::NotAField())) + { + fieldSeq = fieldSeq->GetTail(); + + // Note we may have a primitive here (and correctly fail to obtain the handle) + eeGetFieldType(fieldSeq->GetFieldHandle(), &structHnd); } } break; @@ -17267,15 +17190,24 @@ bool Compiler::gtIsStaticGCBaseHelperCall(GenTree* tree) return false; } -void GenTree::ParseArrayAddress( - Compiler* comp, ArrayInfo* arrayInfo, GenTree** pArr, ValueNum* pInxVN, FieldSeqNode** pFldSeq) +//------------------------------------------------------------------------ +// ParseArrayAddress: Rehydrate the array and index expression from ARR_ADDR. +// +// Arguments: +// comp - The Compiler instance +// pArr - [out] parameter for the tree representing the array instance +// (either an array object pointer, or perhaps a byref to the some element) +// pInxVN - [out] parameter for the value number representing the index +// +// Return Value: +// Will set "*pArr" to "nullptr" if this array address is not parseable. +// +void GenTreeArrAddr::ParseArrayAddress(Compiler* comp, GenTree** pArr, ValueNum* pInxVN) { *pArr = nullptr; ValueNum inxVN = ValueNumStore::NoVN; target_ssize_t offset = 0; - FieldSeqNode* fldSeq = nullptr; - - ParseArrayAddressWork(comp, 1, pArr, &inxVN, &offset, &fldSeq); + ParseArrayAddressWork(this->Addr(), comp, 1, pArr, &inxVN, &offset); // If we didn't find an array reference (perhaps it is the constant null?) we will give up. if (*pArr == nullptr) @@ -17284,74 +17216,29 @@ void GenTree::ParseArrayAddress( } // OK, new we have to figure out if any part of the "offset" is a constant contribution to the index. - // First, sum the offsets of any fields in fldSeq. - unsigned fieldOffsets = 0; - FieldSeqNode* fldSeqIter = fldSeq; - // Also, find the first non-pseudo field... - assert(*pFldSeq == nullptr); - while (fldSeqIter != nullptr) - { - if (fldSeqIter == FieldSeqStore::NotAField()) - { - // TODO-Review: A NotAField here indicates a failure to properly maintain the field sequence - // See test case self_host_tests_x86\jit\regression\CLR-x86-JIT\v1-m12-beta2\ b70992\ b70992.exe - // Safest thing to do here is to drop back to MinOpts - CLANG_FORMAT_COMMENT_ANCHOR; + target_ssize_t elemOffset = GetFirstElemOffset(); + unsigned elemSizeUn = (GetElemType() == TYP_STRUCT) ? comp->typGetObjLayout(GetElemClassHandle())->GetSize() + : genTypeSize(GetElemType()); -#ifdef DEBUG - if (comp->opts.optRepeat) - { - // We don't guarantee preserving these annotations through the entire optimizer, so - // just conservatively return null if under optRepeat. - *pArr = nullptr; - return; - } -#endif // DEBUG - noway_assert(!"fldSeqIter is NotAField() in ParseArrayAddress"); - } + assert(FitsIn(elemSizeUn)); + target_ssize_t elemSize = static_cast(elemSizeUn); + target_ssize_t constIndexOffset = offset - elemOffset; - if (!FieldSeqStore::IsPseudoField(fldSeqIter->GetFieldHandleValue())) - { - if (*pFldSeq == nullptr) - { - *pFldSeq = fldSeqIter; - } - CORINFO_CLASS_HANDLE fldCls = nullptr; - noway_assert(fldSeqIter->GetFieldHandle() != NO_FIELD_HANDLE); - CorInfoType cit = comp->info.compCompHnd->getFieldType(fldSeqIter->GetFieldHandle(), &fldCls); - fieldOffsets += comp->compGetTypeSize(cit, fldCls); - } - fldSeqIter = fldSeqIter->GetNext(); - } - - // Is there some portion of the "offset" beyond the first-elem offset and the struct field suffix we just computed? - if (!FitsIn(fieldOffsets + arrayInfo->m_elemOffset) || - !FitsIn(arrayInfo->m_elemSize)) - { - // This seems unlikely, but no harm in being safe... - *pInxVN = comp->GetValueNumStore()->VNForExpr(nullptr, TYP_INT); - return; - } - // Otherwise... - target_ssize_t offsetAccountedFor = static_cast(fieldOffsets + arrayInfo->m_elemOffset); - target_ssize_t elemSize = static_cast(arrayInfo->m_elemSize); - - target_ssize_t constIndOffset = offset - offsetAccountedFor; // This should be divisible by the element size... - assert((constIndOffset % elemSize) == 0); - target_ssize_t constInd = constIndOffset / elemSize; + assert((constIndexOffset % elemSize) == 0); + target_ssize_t constIndex = constIndexOffset / elemSize; ValueNumStore* vnStore = comp->GetValueNumStore(); if (inxVN == ValueNumStore::NoVN) { // Must be a constant index. - *pInxVN = vnStore->VNForPtrSizeIntCon(constInd); + *pInxVN = vnStore->VNForPtrSizeIntCon(constIndex); } else { // - // Perform ((inxVN / elemSizeVN) + vnForConstInd) + // Perform ((inxVN / elemSizeVN) + vnForConstIndex) // // The value associated with the index value number (inxVN) is the offset into the array, @@ -17360,7 +17247,7 @@ void GenTree::ParseArrayAddress( { target_ssize_t index = vnStore->CoercedConstantValue(inxVN); noway_assert(elemSize > 0 && ((index % elemSize) == 0)); - *pInxVN = vnStore->VNForPtrSizeIntCon((index / elemSize) + constInd); + *pInxVN = vnStore->VNForPtrSizeIntCon((index / elemSize) + constIndex); } else { @@ -17387,7 +17274,7 @@ void GenTree::ParseArrayAddress( } } - // Perform ((inxVN / elemSizeVN) + vnForConstInd) + // Perform ((inxVN / elemSizeVN) + vnForConstIndex) if (!canFoldDiv) { ValueNum vnForElemSize = vnStore->VNForPtrSizeIntCon(elemSize); @@ -17395,50 +17282,44 @@ void GenTree::ParseArrayAddress( *pInxVN = vnForScaledInx; } - if (constInd != 0) + if (constIndex != 0) { - ValueNum vnForConstInd = comp->GetValueNumStore()->VNForPtrSizeIntCon(constInd); - VNFunc vnFunc = VNFunc(GT_ADD); + ValueNum vnForConstIndex = vnStore->VNForPtrSizeIntCon(constIndex); - *pInxVN = comp->GetValueNumStore()->VNForFunc(TYP_I_IMPL, vnFunc, *pInxVN, vnForConstInd); + *pInxVN = comp->GetValueNumStore()->VNForFunc(TYP_I_IMPL, VNFunc(GT_ADD), *pInxVN, vnForConstIndex); } } } } -void GenTree::ParseArrayAddressWork(Compiler* comp, - target_ssize_t inputMul, - GenTree** pArr, - ValueNum* pInxVN, - target_ssize_t* pOffset, - FieldSeqNode** pFldSeq) +/* static */ void GenTreeArrAddr::ParseArrayAddressWork( + GenTree* tree, Compiler* comp, target_ssize_t inputMul, GenTree** pArr, ValueNum* pInxVN, target_ssize_t* pOffset) { - if (TypeGet() == TYP_REF) + if (tree->TypeIs(TYP_REF)) { // This must be the array pointer. - *pArr = this; + *pArr = tree; assert(inputMul == 1); // Can't multiply the array pointer by anything. } else { - switch (OperGet()) + switch (tree->OperGet()) { case GT_CNS_INT: - *pFldSeq = comp->GetFieldSeqStore()->Append(*pFldSeq, AsIntCon()->gtFieldSeq); - assert(!AsIntCon()->ImmedValNeedsReloc(comp)); + assert(!tree->AsIntCon()->ImmedValNeedsReloc(comp)); // TODO-CrossBitness: we wouldn't need the cast below if GenTreeIntCon::gtIconVal had target_ssize_t // type. - *pOffset += (inputMul * (target_ssize_t)(AsIntCon()->gtIconVal)); + *pOffset += (inputMul * (target_ssize_t)(tree->AsIntCon()->gtIconVal)); return; case GT_ADD: case GT_SUB: - AsOp()->gtOp1->ParseArrayAddressWork(comp, inputMul, pArr, pInxVN, pOffset, pFldSeq); - if (OperGet() == GT_SUB) + ParseArrayAddressWork(tree->AsOp()->gtOp1, comp, inputMul, pArr, pInxVN, pOffset); + if (tree->OperIs(GT_SUB)) { inputMul = -inputMul; } - AsOp()->gtOp2->ParseArrayAddressWork(comp, inputMul, pArr, pInxVN, pOffset, pFldSeq); + ParseArrayAddressWork(tree->AsOp()->gtOp2, comp, inputMul, pArr, pInxVN, pOffset); return; case GT_MUL: @@ -17446,39 +17327,39 @@ void GenTree::ParseArrayAddressWork(Compiler* comp, // If one op is a constant, continue parsing down. target_ssize_t subMul = 0; GenTree* nonConst = nullptr; - if (AsOp()->gtOp1->IsCnsIntOrI()) + if (tree->AsOp()->gtOp1->IsCnsIntOrI()) { // If the other arg is an int constant, and is a "not-a-field", choose // that as the multiplier, thus preserving constant index offsets... - if (AsOp()->gtOp2->OperGet() == GT_CNS_INT && - AsOp()->gtOp2->AsIntCon()->gtFieldSeq == FieldSeqStore::NotAField()) + if (tree->AsOp()->gtOp2->OperGet() == GT_CNS_INT && + tree->AsOp()->gtOp2->AsIntCon()->gtFieldSeq == FieldSeqStore::NotAField()) { - assert(!AsOp()->gtOp2->AsIntCon()->ImmedValNeedsReloc(comp)); + assert(!tree->AsOp()->gtOp2->AsIntCon()->ImmedValNeedsReloc(comp)); // TODO-CrossBitness: we wouldn't need the cast below if GenTreeIntConCommon::gtIconVal had // target_ssize_t type. - subMul = (target_ssize_t)AsOp()->gtOp2->AsIntConCommon()->IconValue(); - nonConst = AsOp()->gtOp1; + subMul = (target_ssize_t)tree->AsOp()->gtOp2->AsIntConCommon()->IconValue(); + nonConst = tree->AsOp()->gtOp1; } else { - assert(!AsOp()->gtOp1->AsIntCon()->ImmedValNeedsReloc(comp)); + assert(!tree->AsOp()->gtOp1->AsIntCon()->ImmedValNeedsReloc(comp)); // TODO-CrossBitness: we wouldn't need the cast below if GenTreeIntConCommon::gtIconVal had // target_ssize_t type. - subMul = (target_ssize_t)AsOp()->gtOp1->AsIntConCommon()->IconValue(); - nonConst = AsOp()->gtOp2; + subMul = (target_ssize_t)tree->AsOp()->gtOp1->AsIntConCommon()->IconValue(); + nonConst = tree->AsOp()->gtOp2; } } - else if (AsOp()->gtOp2->IsCnsIntOrI()) + else if (tree->AsOp()->gtOp2->IsCnsIntOrI()) { - assert(!AsOp()->gtOp2->AsIntCon()->ImmedValNeedsReloc(comp)); + assert(!tree->AsOp()->gtOp2->AsIntCon()->ImmedValNeedsReloc(comp)); // TODO-CrossBitness: we wouldn't need the cast below if GenTreeIntConCommon::gtIconVal had // target_ssize_t type. - subMul = (target_ssize_t)AsOp()->gtOp2->AsIntConCommon()->IconValue(); - nonConst = AsOp()->gtOp1; + subMul = (target_ssize_t)tree->AsOp()->gtOp2->AsIntConCommon()->IconValue(); + nonConst = tree->AsOp()->gtOp1; } if (nonConst != nullptr) { - nonConst->ParseArrayAddressWork(comp, inputMul * subMul, pArr, pInxVN, pOffset, pFldSeq); + ParseArrayAddressWork(nonConst, comp, inputMul * subMul, pArr, pInxVN, pOffset); return; } // Otherwise, exit the switch, treat as a contribution to the index. @@ -17487,14 +17368,14 @@ void GenTree::ParseArrayAddressWork(Compiler* comp, case GT_LSH: // If one op is a constant, continue parsing down. - if (AsOp()->gtOp2->IsCnsIntOrI()) + if (tree->AsOp()->gtOp2->IsCnsIntOrI()) { - assert(!AsOp()->gtOp2->AsIntCon()->ImmedValNeedsReloc(comp)); + assert(!tree->AsOp()->gtOp2->AsIntCon()->ImmedValNeedsReloc(comp)); // TODO-CrossBitness: we wouldn't need the cast below if GenTreeIntCon::gtIconVal had target_ssize_t // type. - target_ssize_t shiftVal = (target_ssize_t)AsOp()->gtOp2->AsIntConCommon()->IconValue(); + target_ssize_t shiftVal = (target_ssize_t)tree->AsOp()->gtOp2->AsIntConCommon()->IconValue(); target_ssize_t subMul = target_ssize_t{1} << shiftVal; - AsOp()->gtOp1->ParseArrayAddressWork(comp, inputMul * subMul, pArr, pInxVN, pOffset, pFldSeq); + ParseArrayAddressWork(tree->AsOp()->gtOp1, comp, inputMul * subMul, pArr, pInxVN, pOffset); return; } // Otherwise, exit the switch, treat as a contribution to the index. @@ -17502,9 +17383,9 @@ void GenTree::ParseArrayAddressWork(Compiler* comp, case GT_COMMA: // We don't care about exceptions for this purpose. - if (AsOp()->gtOp1->OperIs(GT_BOUNDS_CHECK) || AsOp()->gtOp1->IsNothingNode()) + if (tree->AsOp()->gtOp1->OperIs(GT_BOUNDS_CHECK) || tree->AsOp()->gtOp1->IsNothingNode()) { - AsOp()->gtOp2->ParseArrayAddressWork(comp, inputMul, pArr, pInxVN, pOffset, pFldSeq); + ParseArrayAddressWork(tree->AsOp()->gtOp2, comp, inputMul, pArr, pInxVN, pOffset); return; } break; @@ -17513,11 +17394,11 @@ void GenTree::ParseArrayAddressWork(Compiler* comp, break; } // If we didn't return above, must be a contribution to the non-constant part of the index VN. - ValueNum vn = comp->GetValueNumStore()->VNLiberalNormalValue(gtVNPair); + ValueNum vn = comp->GetValueNumStore()->VNLiberalNormalValue(tree->gtVNPair); if (inputMul != 1) { ValueNum mulVN = comp->GetValueNumStore()->VNForLongCon(inputMul); - vn = comp->GetValueNumStore()->VNForFunc(TypeGet(), VNFunc(GT_MUL), mulVN, vn); + vn = comp->GetValueNumStore()->VNForFunc(tree->TypeGet(), VNFunc(GT_MUL), mulVN, vn); } if (*pInxVN == ValueNumStore::NoVN) { @@ -17525,159 +17406,39 @@ void GenTree::ParseArrayAddressWork(Compiler* comp, } else { - *pInxVN = comp->GetValueNumStore()->VNForFunc(TypeGet(), VNFunc(GT_ADD), *pInxVN, vn); + *pInxVN = comp->GetValueNumStore()->VNForFunc(tree->TypeGet(), VNFunc(GT_ADD), *pInxVN, vn); } } } -bool GenTree::ParseArrayElemForm(Compiler* comp, ArrayInfo* arrayInfo, FieldSeqNode** pFldSeq) -{ - if (OperIsIndir()) - { - if (gtFlags & GTF_IND_ARR_INDEX) - { - bool b = comp->GetArrayInfoMap()->Lookup(this, arrayInfo); - assert(b); - return true; - } - - // Otherwise... - GenTree* addr = AsIndir()->Addr(); - return addr->ParseArrayElemAddrForm(comp, arrayInfo, pFldSeq); - } - else - { - return false; - } -} - -bool GenTree::ParseArrayElemAddrForm(Compiler* comp, ArrayInfo* arrayInfo, FieldSeqNode** pFldSeq) +//------------------------------------------------------------------------ +// IsArrayAddr: Is "this" an expression for an array address? +// +// Recognizes the following patterns: +// this: ARR_ADDR +// this: ADD(ARR_ADDR, CONST) +// +// Arguments: +// pArrAddr - [out] parameter for the found ARR_ADDR node +// +// Return Value: +// Whether "this" matches the pattern denoted above. +// +bool GenTree::IsArrayAddr(GenTreeArrAddr** pArrAddr) { - switch (OperGet()) + GenTree* addr = this; + if (addr->OperIs(GT_ADD) && addr->AsOp()->gtGetOp2()->IsCnsIntOrI()) { - case GT_ADD: - { - GenTree* arrAddr = nullptr; - GenTree* offset = nullptr; - if (AsOp()->gtOp1->TypeGet() == TYP_BYREF) - { - arrAddr = AsOp()->gtOp1; - offset = AsOp()->gtOp2; - } - else if (AsOp()->gtOp2->TypeGet() == TYP_BYREF) - { - arrAddr = AsOp()->gtOp2; - offset = AsOp()->gtOp1; - } - else - { - return false; - } - if (!offset->ParseOffsetForm(comp, pFldSeq)) - { - return false; - } - return arrAddr->ParseArrayElemAddrForm(comp, arrayInfo, pFldSeq); - } - - case GT_ADDR: - { - GenTree* addrArg = AsOp()->gtOp1; - if (addrArg->OperGet() != GT_IND) - { - return false; - } - else - { - // The "Addr" node might be annotated with a zero-offset field sequence. - FieldSeqNode* zeroOffsetFldSeq = nullptr; - if (comp->GetZeroOffsetFieldMap()->Lookup(this, &zeroOffsetFldSeq)) - { - *pFldSeq = comp->GetFieldSeqStore()->Append(*pFldSeq, zeroOffsetFldSeq); - } - return addrArg->ParseArrayElemForm(comp, arrayInfo, pFldSeq); - } - } - - default: - return false; + addr = addr->AsOp()->gtGetOp1(); } -} -bool GenTree::ParseOffsetForm(Compiler* comp, FieldSeqNode** pFldSeq) -{ - switch (OperGet()) + if (addr->OperIs(GT_ARR_ADDR)) { - case GT_CNS_INT: - { - GenTreeIntCon* icon = AsIntCon(); - *pFldSeq = comp->GetFieldSeqStore()->Append(*pFldSeq, icon->gtFieldSeq); - return true; - } - - case GT_ADD: - if (!AsOp()->gtOp1->ParseOffsetForm(comp, pFldSeq)) - { - return false; - } - return AsOp()->gtOp2->ParseOffsetForm(comp, pFldSeq); - - default: - return false; + *pArrAddr = addr->AsArrAddr(); + return true; } -} - -void GenTree::LabelIndex(Compiler* comp, bool isConst) -{ - switch (OperGet()) - { - case GT_CNS_INT: - // If we got here, this is a contribution to the constant part of the index. - if (isConst) - { - AsIntCon()->gtFieldSeq = - comp->GetFieldSeqStore()->CreateSingleton(FieldSeqStore::ConstantIndexPseudoField); - } - return; - - case GT_LCL_VAR: - gtFlags |= GTF_VAR_ARR_INDEX; - return; - - case GT_ADD: - case GT_SUB: - AsOp()->gtOp1->LabelIndex(comp, isConst); - AsOp()->gtOp2->LabelIndex(comp, isConst); - break; - - case GT_CAST: - AsOp()->gtOp1->LabelIndex(comp, isConst); - break; - - case GT_ARR_LENGTH: - gtFlags |= GTF_ARRLEN_ARR_IDX; - return; - default: - // For all other operators, peel off one constant; and then label the other if it's also a constant. - if (OperIsArithmetic() || OperIsCompare()) - { - if (AsOp()->gtOp2->OperGet() == GT_CNS_INT) - { - AsOp()->gtOp1->LabelIndex(comp, isConst); - break; - } - else if (AsOp()->gtOp1->OperGet() == GT_CNS_INT) - { - AsOp()->gtOp2->LabelIndex(comp, isConst); - break; - } - // Otherwise continue downward on both, labeling vars. - AsOp()->gtOp1->LabelIndex(comp, false); - AsOp()->gtOp2->LabelIndex(comp, false); - } - break; - } + return false; } // Note that the value of the below field doesn't matter; it exists only to provide a distinguished address. @@ -17724,13 +17485,6 @@ FieldSeqNode* FieldSeqStore::Append(FieldSeqNode* a, FieldSeqNode* b) else if (b == NotAField()) { return NotAField(); - // Extremely special case for ConstantIndex pseudo-fields -- appending consecutive such - // together collapse to one. - } - else if (a->GetNext() == nullptr && a->GetFieldHandleValue() == ConstantIndexPseudoField && - b->GetFieldHandleValue() == ConstantIndexPseudoField) - { - return b; } else { @@ -17754,15 +17508,6 @@ FieldSeqNode* FieldSeqStore::Append(FieldSeqNode* a, FieldSeqNode* b) } } -// Static vars. -int FieldSeqStore::FirstElemPseudoFieldStruct; -int FieldSeqStore::ConstantIndexPseudoFieldStruct; - -CORINFO_FIELD_HANDLE FieldSeqStore::FirstElemPseudoField = - (CORINFO_FIELD_HANDLE)&FieldSeqStore::FirstElemPseudoFieldStruct; -CORINFO_FIELD_HANDLE FieldSeqStore::ConstantIndexPseudoField = - (CORINFO_FIELD_HANDLE)&FieldSeqStore::ConstantIndexPseudoFieldStruct; - FieldSeqNode::FieldSeqNode(CORINFO_FIELD_HANDLE fieldHnd, FieldSeqNode* next, FieldKind fieldKind) : m_next(next) { uintptr_t handleValue = reinterpret_cast(fieldHnd); @@ -17770,33 +17515,17 @@ FieldSeqNode::FieldSeqNode(CORINFO_FIELD_HANDLE fieldHnd, FieldSeqNode* next, Fi assert((handleValue & FIELD_KIND_MASK) == 0); m_fieldHandleAndKind = handleValue | static_cast(fieldKind); - if (!FieldSeqStore::IsPseudoField(fieldHnd) && (fieldHnd != NO_FIELD_HANDLE)) + if (fieldHnd != NO_FIELD_HANDLE) { assert(JitTls::GetCompiler()->eeIsFieldStatic(fieldHnd) == IsStaticField()); } else { - // Use the default for pseudo-fields. + // Use the default for NotAField. assert(fieldKind == FieldKind::Instance); } } -bool FieldSeqNode::IsFirstElemFieldSeq() const -{ - return GetFieldHandleValue() == FieldSeqStore::FirstElemPseudoField; -} - -bool FieldSeqNode::IsConstantIndexFieldSeq() const -{ - return GetFieldHandleValue() == FieldSeqStore::ConstantIndexPseudoField; -} - -bool FieldSeqNode::IsPseudoField() const -{ - return (GetFieldHandleValue() == FieldSeqStore::FirstElemPseudoField) || - (GetFieldHandleValue() == FieldSeqStore::ConstantIndexPseudoField); -} - #ifdef FEATURE_SIMD GenTreeSIMD* Compiler::gtNewSIMDNode( var_types type, GenTree* op1, SIMDIntrinsicID simdIntrinsicID, CorInfoType simdBaseJitType, unsigned simdSize) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index a514de9c8f593..86d7f5bb102e6 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -254,7 +254,7 @@ class FieldSeqNode CORINFO_FIELD_HANDLE GetFieldHandle() const { - assert(!IsPseudoField() && (GetFieldHandleValue() != NO_FIELD_HANDLE)); + assert(GetFieldHandleValue() != NO_FIELD_HANDLE); return GetFieldHandleValue(); } @@ -263,14 +263,10 @@ class FieldSeqNode return CORINFO_FIELD_HANDLE(m_fieldHandleAndKind & ~FIELD_KIND_MASK); } - // returns true when this is the pseudo #FirstElem field sequence - bool IsFirstElemFieldSeq() const; - - // returns true when this is the pseudo #ConstantIndex field sequence - bool IsConstantIndexFieldSeq() const; - - // returns true when this is the the pseudo #FirstElem field sequence or the pseudo #ConstantIndex field sequence - bool IsPseudoField() const; + FieldSeqNode* GetNext() const + { + return m_next; + } bool IsStaticField() const { @@ -282,11 +278,6 @@ class FieldSeqNode return GetKind() == FieldKind::SharedStatic; } - FieldSeqNode* GetNext() const - { - return m_next; - } - FieldSeqNode* GetTail() { FieldSeqNode* tail = this; @@ -321,10 +312,6 @@ class FieldSeqStore static FieldSeqNode s_notAField; // No value, just exists to provide an address. - // Dummy variables to provide the addresses for the "pseudo field handle" statics below. - static int FirstElemPseudoFieldStruct; - static int ConstantIndexPseudoFieldStruct; - public: FieldSeqStore(CompAllocator alloc); @@ -345,22 +332,6 @@ class FieldSeqStore // they are the results of CreateSingleton, NotAField, or Append calls. If either of the arguments // are the "NotAField" value, so is the result. FieldSeqNode* Append(FieldSeqNode* a, FieldSeqNode* b); - - // We have a few "pseudo" field handles: - - // This treats the constant offset of the first element of something as if it were a field. - // Works for method table offsets of boxed structs, or first elem offset of arrays/strings. - static CORINFO_FIELD_HANDLE FirstElemPseudoField; - - // If there is a constant index, we make a psuedo field to correspond to the constant added to - // offset of the indexed field. This keeps the field sequence structure "normalized", especially in the - // case where the element type is a struct, so we might add a further struct field offset. - static CORINFO_FIELD_HANDLE ConstantIndexPseudoField; - - static bool IsPseudoField(CORINFO_FIELD_HANDLE hnd) - { - return hnd == FirstElemPseudoField || hnd == ConstantIndexPseudoField; - } }; class GenTreeUseEdgeIterator; @@ -500,9 +471,6 @@ enum GenTreeFlags : unsigned int // on their parents in post-order morph. // Relevant for inlining optimizations (see fgInlinePrependStatements) - GTF_VAR_ARR_INDEX = 0x00000020, // The variable is part of (the index portion of) an array index expression. - // Shares a value with GTF_REVERSE_OPS, which is meaningless for local var. - // For additional flags for GT_CALL node see GTF_CALL_M_* GTF_CALL_UNMANAGED = 0x80000000, // GT_CALL -- direct call to unmanaged code @@ -543,11 +511,10 @@ enum GenTreeFlags : unsigned int GTF_IND_UNALIGNED = 0x02000000, // GT_IND -- the load or store is unaligned (we assume worst case // alignment of 1 byte) GTF_IND_INVARIANT = 0x01000000, // GT_IND -- the target is invariant (a prejit indirection) - GTF_IND_ARR_INDEX = 0x00800000, // GT_IND -- the indirection represents an (SZ) array index GTF_IND_NONNULL = 0x00400000, // GT_IND -- the indirection never returns null (zero) - GTF_IND_FLAGS = GTF_IND_VOLATILE | GTF_IND_TGTANYWHERE | GTF_IND_NONFAULTING | GTF_IND_TLS_REF | \ - GTF_IND_UNALIGNED | GTF_IND_INVARIANT | GTF_IND_NONNULL | GTF_IND_ARR_INDEX | GTF_IND_TGT_NOT_HEAP, + GTF_IND_FLAGS = GTF_IND_VOLATILE | GTF_IND_TGTANYWHERE | GTF_IND_NONFAULTING | GTF_IND_TLS_REF | + GTF_IND_UNALIGNED | GTF_IND_INVARIANT | GTF_IND_NONNULL | GTF_IND_TGT_NOT_HEAP, GTF_CLS_VAR_VOLATILE = 0x40000000, // GT_FIELD/GT_CLS_VAR -- same as GTF_IND_VOLATILE GTF_CLS_VAR_INITCLASS = 0x20000000, // GT_FIELD/GT_CLS_VAR -- same as GTF_FLD_INITCLASS @@ -576,6 +543,8 @@ enum GenTreeFlags : unsigned int GTF_BOX_VALUE = 0x80000000, // GT_BOX -- "box" is on a value type + GTF_ARR_ADDR_NONNULL = 0x80000000, // GT_ARR_ADDR -- this array's address is not null + GTF_ICON_HDL_MASK = 0xFF000000, // Bits used by handle types below GTF_ICON_SCOPE_HDL = 0x01000000, // GT_CNS_INT -- constant is a scope handle GTF_ICON_CLASS_HDL = 0x02000000, // GT_CNS_INT -- constant is a class handle @@ -614,9 +583,8 @@ enum GenTreeFlags : unsigned int GTF_DIV_BY_CNS_OPT = 0x80000000, // GT_DIV -- Uses the division by constant optimization to compute this division - GTF_CHK_INDEX_INBND = 0x80000000, // GT_BOUNDS_CHECK -- have proved this check is always in-bounds + GTF_CHK_INDEX_INBND = 0x80000000, // GT_BOUNDS_CHECK -- have proven this check is always in-bounds - GTF_ARRLEN_ARR_IDX = 0x80000000, // GT_ARR_LENGTH -- Length which feeds into an array index expression GTF_ARRLEN_NONFAULTING = 0x20000000, // GT_ARR_LENGTH -- An array length operation that cannot fault. Same as GT_IND_NONFAULTING. GTF_SIMDASHW_OP = 0x80000000, // GT_HWINTRINSIC -- Indicates that the structHandle should be gotten from gtGetStructHandleForSIMD @@ -1962,41 +1930,7 @@ struct GenTree bool IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pFldSeq); - // Requires "this" to be the address of an array (the child of a GT_IND labeled with GTF_IND_ARR_INDEX). - // Sets "pArr" to the node representing the array (either an array object pointer, or perhaps a byref to the some - // element). - // Sets "*pArrayType" to the class handle for the array type. - // Sets "*inxVN" to the value number inferred for the array index. - // Sets "*pFldSeq" to the sequence, if any, of struct fields used to index into the array element. - void ParseArrayAddress( - Compiler* comp, struct ArrayInfo* arrayInfo, GenTree** pArr, ValueNum* pInxVN, FieldSeqNode** pFldSeq); - - // Helper method for the above. - void ParseArrayAddressWork(Compiler* comp, - target_ssize_t inputMul, - GenTree** pArr, - ValueNum* pInxVN, - target_ssize_t* pOffset, - FieldSeqNode** pFldSeq); - - // Requires "this" to be a GT_IND. Requires the outermost caller to set "*pFldSeq" to nullptr. - // Returns true if it is an array index expression, or access to a (sequence of) struct field(s) - // within a struct array element. If it returns true, sets *arrayInfo to the array information, and sets *pFldSeq - // to the sequence of struct field accesses. - bool ParseArrayElemForm(Compiler* comp, ArrayInfo* arrayInfo, FieldSeqNode** pFldSeq); - - // Requires "this" to be the address of a (possible) array element (or struct field within that). - // If it is, sets "*arrayInfo" to the array access info, "*pFldSeq" to the sequence of struct fields - // accessed within the array element, and returns true. If not, returns "false". - bool ParseArrayElemAddrForm(Compiler* comp, ArrayInfo* arrayInfo, FieldSeqNode** pFldSeq); - - // Requires "this" to be an int expression. If it is a sequence of one or more integer constants added together, - // returns true and sets "*pFldSeq" to the sequence of fields with which those constants are annotated. - bool ParseOffsetForm(Compiler* comp, FieldSeqNode** pFldSeq); - - // Labels "*this" as an array index expression: label all constants and variables that could contribute, as part of - // an affine expression, to the value of the of the index. - void LabelIndex(Compiler* comp, bool isConst = true); + bool IsArrayAddr(GenTreeArrAddr** pArrAddr); // Assumes that "this" occurs in a context where it is being dereferenced as the LHS of an assignment-like // statement (assignment, initblk, or copyblk). The "width" should be the number of bytes copied by the @@ -5836,6 +5770,70 @@ struct GenTreeIndexAddr : public GenTreeOp #endif }; +// GenTreeArrAddr - GT_ARR_ADDR, carries information about the array type from morph to VN. +// This node is just a wrapper (similar to GenTreeBox), the real address +// expression is contained in its first operand. +// +struct GenTreeArrAddr : GenTreeUnOp +{ +private: + CORINFO_CLASS_HANDLE m_elemClassHandle; // The array element class. Currently only used for arrays of TYP_STRUCT. + var_types m_elemType; // The normalized (TYP_SIMD != TYP_STRUCT) array element type. + uint8_t m_firstElemOffset; // Offset to the first element of the array. + +public: + GenTreeArrAddr(GenTree* addr, var_types elemType, CORINFO_CLASS_HANDLE elemClassHandle, uint8_t firstElemOffset) + : GenTreeUnOp(GT_ARR_ADDR, TYP_BYREF, addr DEBUGARG(/* largeNode */ false)) + , m_elemClassHandle(elemClassHandle) + , m_elemType(elemType) + , m_firstElemOffset(firstElemOffset) + { + assert(addr->TypeIs(TYP_BYREF)); + assert(((elemType == TYP_STRUCT) && (elemClassHandle != NO_CLASS_HANDLE)) || + (elemClassHandle == NO_CLASS_HANDLE)); + + // We will only consider "addr" for CSE. This is more profitable and precise + // because ARR_ADDR can get its VN "polluted" by zero-offset field sequences. + SetDoNotCSE(); + } + +#if DEBUGGABLE_GENTREE + GenTreeArrAddr() : GenTreeUnOp() + { + } +#endif + + GenTree*& Addr() + { + return gtOp1; + } + + CORINFO_CLASS_HANDLE GetElemClassHandle() const + { + return m_elemClassHandle; + } + + var_types GetElemType() const + { + return m_elemType; + } + + uint8_t GetFirstElemOffset() const + { + return m_firstElemOffset; + } + + void ParseArrayAddress(Compiler* comp, GenTree** pArr, ValueNum* pInxVN); + +private: + static void ParseArrayAddressWork(GenTree* tree, + Compiler* comp, + target_ssize_t inputMul, + GenTree** pArr, + ValueNum* pInxVN, + target_ssize_t* pOffset); +}; + /* gtArrLen -- array length (GT_ARR_LENGTH) GT_ARR_LENGTH is used for "arr.length" */ diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index 1d0be8bd16766..c902b21ec72f4 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -101,6 +101,7 @@ GTNODE(INIT_VAL , GenTreeOp ,0,GTK_UNOP) // Initialization valu GTNODE(BOX , GenTreeBox ,0,GTK_UNOP|GTK_EXOP|DBK_NOTLIR) // Marks its first operands (a local) as being a box GTNODE(PUTARG_TYPE , GenTreeOp ,0,GTK_UNOP|DBK_NOTLIR) // Saves argument type between importation and morph GTNODE(RUNTIMELOOKUP , GenTreeRuntimeLookup, 0,GTK_UNOP|GTK_EXOP|DBK_NOTLIR) // Runtime handle lookup +GTNODE(ARR_ADDR , GenTreeArrAddr ,0,GTK_UNOP|GTK_EXOP|DBK_NOTLIR) // Wraps an array address expression GTNODE(BSWAP , GenTreeOp ,0,GTK_UNOP) // Byte swap (32-bit or 64-bit) GTNODE(BSWAP16 , GenTreeOp ,0,GTK_UNOP) // Byte swap (16-bit) diff --git a/src/coreclr/jit/gtstructs.h b/src/coreclr/jit/gtstructs.h index 6373011779020..2507c633be15c 100644 --- a/src/coreclr/jit/gtstructs.h +++ b/src/coreclr/jit/gtstructs.h @@ -115,6 +115,7 @@ GTSTRUCT_1(HWIntrinsic , GT_HWINTRINSIC) #endif // FEATURE_HW_INTRINSICS GTSTRUCT_1(AllocObj , GT_ALLOCOBJ) GTSTRUCT_1(RuntimeLookup, GT_RUNTIMELOOKUP) +GTSTRUCT_1(ArrAddr , GT_ARR_ADDR) GTSTRUCT_2(CC , GT_JCC, GT_SETCC) #if defined(TARGET_X86) GTSTRUCT_1(MultiRegOp , GT_MUL_LONG) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index c6c0bd3bac3b3..3a6d6d1fae57f 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -259,12 +259,6 @@ class LocalAddressVisitor final : public GenTreeVisitor { haveCorrectFieldForVN = false; } - else if (FieldSeqStore::IsPseudoField(field->gtFldHnd)) - { - assert(compiler->compObjectStackAllocation()); - // We use PseudoFields when accessing stack allocated classes. - haveCorrectFieldForVN = false; - } else if (val.m_fieldSeq == nullptr) { @@ -279,7 +273,7 @@ class LocalAddressVisitor final : public GenTreeVisitor { FieldSeqNode* lastSeqNode = val.m_fieldSeq->GetTail(); assert(lastSeqNode != nullptr); - if (lastSeqNode->IsPseudoField() || lastSeqNode == FieldSeqStore::NotAField()) + if (lastSeqNode == FieldSeqStore::NotAField()) { haveCorrectFieldForVN = false; } @@ -1026,11 +1020,7 @@ class LocalAddressVisitor final : public GenTreeVisitor if ((fieldSeq != nullptr) && (fieldSeq != FieldSeqStore::NotAField())) { - // TODO-ADDR: ObjectAllocator produces FIELD nodes with FirstElemPseudoField as field - // handle so we cannot use FieldSeqNode::GetFieldHandle() because it asserts on such - // handles. ObjectAllocator should be changed to create LCL_FLD nodes directly. - assert(!indir->OperIs(GT_FIELD) || - (indir->AsField()->gtFldHnd == fieldSeq->GetTail()->GetFieldHandleValue())); + assert(!indir->OperIs(GT_FIELD) || (indir->AsField()->gtFldHnd == fieldSeq->GetTail()->GetFieldHandle())); } else { diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 8121191b95977..ce6711d13c3e5 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5237,7 +5237,7 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) unsigned elemSize = asIndex->gtIndElemSize; CORINFO_CLASS_HANDLE elemStructType = asIndex->gtStructElemClass; - noway_assert(elemTyp != TYP_STRUCT || elemStructType != nullptr); + noway_assert(elemTyp != TYP_STRUCT || elemStructType != NO_CLASS_HANDLE); // Fold "cns_str"[cns_index] to ushort constant // NOTE: don't do it for empty string, the operation will fail anyway @@ -5273,7 +5273,7 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) // This is the new type of the node. tree->gtType = elemTyp; // Now set elemStructType to null so that we don't confuse value numbering. - elemStructType = nullptr; + elemStructType = NO_CLASS_HANDLE; } } #endif // FEATURE_SIMD @@ -5281,7 +5281,7 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) // Set up the array length's offset into lenOffs // And the first element's offset into elemOffs ssize_t lenOffs; - ssize_t elemOffs; + uint8_t elemOffs; if (tree->gtFlags & GTF_INX_STRING_LAYOUT) { lenOffs = OFFSETOF__CORINFO_String__stringLen; @@ -5308,8 +5308,10 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) // side-effecting. // 3. Perform an explicit bounds check: GT_BOUNDS_CHECK(index, GT_ARR_LENGTH(array)) // 4. Compute the address of the element that will be accessed: - // GT_ADD(GT_ADD(array, firstElementOffset), GT_MUL(index, elementSize)) - // 5. Dereference the address with a GT_IND. + // GT_ADD(GT_ADD(array, firstElementOffset), GT_MUL(index, elementSize)) OR + // GT_ADD(GT_ADD(array, GT_ADD(GT_MUL(index, elementSize), firstElementOffset))) + // 5. Wrap the address in a GT_ADD_ADDR (the information saved there will later be used by VN). + // 6. Dereference the address with a GT_IND. // // This expansion explicitly exposes the bounds check and the address calculation to the optimizer, which allows // for more straightforward bounds-check removal, CSE, etc. @@ -5318,9 +5320,8 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) GenTree* const array = fgMorphTree(asIndex->Arr()); GenTree* const index = fgMorphTree(asIndex->Index()); - GenTreeIndexAddr* const indexAddr = - new (this, GT_INDEX_ADDR) GenTreeIndexAddr(array, index, elemTyp, elemStructType, elemSize, - static_cast(lenOffs), static_cast(elemOffs)); + GenTreeIndexAddr* const indexAddr = new (this, GT_INDEX_ADDR) + GenTreeIndexAddr(array, index, elemTyp, elemStructType, elemSize, static_cast(lenOffs), elemOffs); indexAddr->gtFlags |= (array->gtFlags | index->gtFlags) & GTF_ALL_EFFECT; // Mark the indirection node as needing a range check if necessary. @@ -5343,7 +5344,7 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) GenTreeIndir* const indir = tree->AsIndir(); indir->Addr() = indexAddr; bool canCSE = indir->CanCSE(); - indir->gtFlags = GTF_IND_ARR_INDEX | (indexAddr->gtFlags & GTF_ALL_EFFECT); + indir->gtFlags = indexAddr->gtFlags & GTF_ALL_EFFECT; if (!canCSE) { indir->SetDoNotCSE(); @@ -5530,8 +5531,7 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) addr = gtNewOperNode(GT_ADD, TYP_BYREF, arrRef, addr); } - assert(((tree->gtDebugFlags & GTF_DEBUG_NODE_LARGE) != 0) || - (GenTree::s_gtNodeSizes[GT_IND] == TREE_NODE_SZ_SMALL)); + addr = new (this, GT_ARR_ADDR) GenTreeArrAddr(addr, elemTyp, elemStructType, elemOffs); // Change the orginal GT_INDEX node into a GT_IND node tree->SetOper(GT_IND); @@ -5549,13 +5549,11 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) tree->AsOp()->gtOp1 = addr; - // This is an array index expression. - tree->gtFlags |= GTF_IND_ARR_INDEX; - // If there's a bounds check, the indir won't fault. if (bndsChk || indexNonFaulting) { tree->gtFlags |= GTF_IND_NONFAULTING; + addr->gtFlags |= GTF_ARR_ADDR_NONNULL; } else { @@ -5567,13 +5565,6 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) tree->gtFlags |= GTF_DONT_CSE; } - // Store information about it. - GetArrayInfoMap()->Set(tree, ArrayInfo(elemTyp, elemSize, (int)elemOffs, elemStructType)); - - // Remember this 'indTree' that we just created, as we still need to attach the fieldSeq information to it. - - GenTree* indTree = tree; - // Did we create a bndsChk tree? if (bndsChk) { @@ -5601,116 +5592,11 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) JITDUMP("fgMorphArrayIndex (before remorph):\n") DISPTREE(tree) - // Currently we morph the tree to perform some folding operations prior - // to attaching fieldSeq info and labeling constant array index contributions - // tree = fgMorphTree(tree); JITDUMP("fgMorphArrayIndex (after remorph):\n") DISPTREE(tree) - // Ideally we just want to proceed to attaching fieldSeq info and labeling the - // constant array index contributions, but the morphing operation may have changed - // the 'tree' into something that now unconditionally throws an exception. - // - // In such case the gtEffectiveVal could be a new tree or it's gtOper could be modified - // or it could be left unchanged. If it is unchanged then we should not return, - // instead we should proceed to attaching fieldSeq info, etc... - // - GenTree* arrElem = tree->gtEffectiveVal(); - - if (fgIsCommaThrow(tree)) - { - if ((arrElem != indTree) || // A new tree node may have been created - (!indTree->OperIs(GT_IND))) // The GT_IND may have been changed to a GT_CNS_INT - { - return tree; // Just return the Comma-Throw, don't try to attach the fieldSeq info, etc.. - } - } - - assert(!fgGlobalMorph || (arrElem->gtDebugFlags & GTF_DEBUG_NODE_MORPHED)); - DBEXEC(fgGlobalMorph && (arrElem == tree), tree->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED) - - addr = arrElem->gtGetOp1(); - - GenTree* cnsOff = nullptr; - if (addr->OperIs(GT_ADD)) - { - GenTree* addrOp1 = addr->gtGetOp1(); - if (groupArrayRefWithElemOffset) - { - if (addrOp1->OperIs(GT_ADD) && addrOp1->gtGetOp2()->IsCnsIntOrI()) - { - assert(addrOp1->gtGetOp1()->TypeIs(TYP_REF)); - cnsOff = addrOp1->gtGetOp2(); - addr = addr->gtGetOp2(); - // Label any constant array index contributions with #ConstantIndex and any LclVars with - // GTF_VAR_ARR_INDEX - addr->LabelIndex(this); - } - else - { - assert(addr->gtGetOp2()->IsCnsIntOrI()); - cnsOff = addr->gtGetOp2(); - addr = nullptr; - } - } - else - { - assert(addr->TypeIs(TYP_BYREF)); - assert(addr->gtGetOp1()->TypeIs(TYP_REF)); - addr = addr->gtGetOp2(); - - // Look for the constant [#FirstElem] node here, or as the RHS of an ADD. - if (addr->IsCnsIntOrI()) - { - cnsOff = addr; - addr = nullptr; - } - else - { - if ((addr->OperIs(GT_ADD)) && addr->gtGetOp2()->IsCnsIntOrI()) - { - cnsOff = addr->gtGetOp2(); - addr = addr->gtGetOp1(); - } - // Label any constant array index contributions with #ConstantIndex and any LclVars with - // GTF_VAR_ARR_INDEX - addr->LabelIndex(this); - } - } - } - else if (addr->IsCnsIntOrI()) - { - cnsOff = addr; - } - - FieldSeqNode* firstElemFseq = GetFieldSeqStore()->CreateSingleton(FieldSeqStore::FirstElemPseudoField); - - if ((cnsOff != nullptr) && (cnsOff->AsIntCon()->gtIconVal == elemOffs)) - { - // Assign it the [#FirstElem] field sequence - // - cnsOff->AsIntCon()->gtFieldSeq = firstElemFseq; - } - else // We have folded the first element's offset with the index expression - { - // Build the [#ConstantIndex, #FirstElem] field sequence - // - FieldSeqNode* constantIndexFseq = GetFieldSeqStore()->CreateSingleton(FieldSeqStore::ConstantIndexPseudoField); - FieldSeqNode* fieldSeq = GetFieldSeqStore()->Append(constantIndexFseq, firstElemFseq); - - if (cnsOff == nullptr) // It must have folded into a zero offset - { - // Record in the general zero-offset map. - fgAddFieldSeqForZeroOffset(addr, fieldSeq); - } - else - { - cnsOff->AsIntCon()->gtFieldSeq = fieldSeq; - } - } - return tree; } @@ -10402,22 +10288,10 @@ GenTree* Compiler::fgMorphGetStructAddr(GenTree** pTree, CORINFO_CLASS_HANDLE cl { GenTree* addr; GenTree* tree = *pTree; - // If this is an indirection, we can return its op1, unless it's a GTF_IND_ARR_INDEX, in which case we - // need to hang onto that for the purposes of value numbering. + // If this is an indirection, we can return its address. if (tree->OperIsIndir()) { - if ((tree->gtFlags & GTF_IND_ARR_INDEX) == 0) - { - addr = tree->AsOp()->gtOp1; - } - else - { - if (isRValue && tree->OperIsBlk()) - { - tree->ChangeOper(GT_IND); - } - addr = gtNewOperNode(GT_ADDR, TYP_BYREF, tree); - } + addr = tree->AsOp()->gtOp1; } else if (tree->gtOper == GT_COMMA) { @@ -12835,14 +12709,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) commaNode->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; #endif } - bool wasArrIndex = (tree->gtFlags & GTF_IND_ARR_INDEX) != 0; - ArrayInfo arrInfo; - if (wasArrIndex) - { - bool b = GetArrayInfoMap()->Lookup(tree, &arrInfo); - assert(b); - GetArrayInfoMap()->Remove(tree); - } + tree = op1; GenTree* addr = commaNode->AsOp()->gtOp2; // TODO-1stClassStructs: we often create a struct IND without a handle, fix it. @@ -12851,10 +12718,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) op1->gtFlags |= treeFlags & ~GTF_ALL_EFFECT & ~GTF_IND_NONFAULTING; op1->gtFlags |= (addr->gtFlags & GTF_ALL_EFFECT); - if (wasArrIndex) - { - GetArrayInfoMap()->Set(op1, arrInfo); - } #ifdef DEBUG op1->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; #endif @@ -12876,32 +12739,29 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) if (op1->OperGet() == GT_IND) { - if ((op1->gtFlags & GTF_IND_ARR_INDEX) == 0) + // Can not remove a GT_ADDR if it is currently a CSE candidate. + if (gtIsActiveCSE_Candidate(tree)) { - // Can not remove a GT_ADDR if it is currently a CSE candidate. - if (gtIsActiveCSE_Candidate(tree)) - { - break; - } + break; + } - // Perform the transform ADDR(IND(...)) == (...). - GenTree* addr = op1->AsOp()->gtOp1; + // Perform the transform ADDR(IND(...)) == (...). + GenTree* addr = op1->AsOp()->gtOp1; - // If tree has a zero field sequence annotation, update the annotation - // on addr node. - FieldSeqNode* zeroFieldSeq = nullptr; - if (GetZeroOffsetFieldMap()->Lookup(tree, &zeroFieldSeq)) - { - fgAddFieldSeqForZeroOffset(addr, zeroFieldSeq); - } + // If tree has a zero field sequence annotation, update the annotation + // on addr node. + FieldSeqNode* zeroFieldSeq = nullptr; + if (GetZeroOffsetFieldMap()->Lookup(tree, &zeroFieldSeq)) + { + fgAddFieldSeqForZeroOffset(addr, zeroFieldSeq); + } - noway_assert(varTypeIsGC(addr->gtType) || addr->gtType == TYP_I_IMPL); + noway_assert(varTypeIsGC(addr->gtType) || addr->gtType == TYP_I_IMPL); - DEBUG_DESTROY_NODE(op1); - DEBUG_DESTROY_NODE(tree); + DEBUG_DESTROY_NODE(op1); + DEBUG_DESTROY_NODE(tree); - return addr; - } + return addr; } else if (op1->OperGet() == GT_OBJ) { @@ -12966,15 +12826,11 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) // Originally, I gave all the comma nodes type "byref". But the ADDR(IND(x)) == x transform // might give op1 a type different from byref (like, say, native int). So now go back and give // all the comma nodes the type of op1. - // TODO: the comma flag update below is conservative and can be improved. - // For example, if we made the ADDR(IND(x)) == x transformation, we may be able to - // get rid of some of the IND flags on the COMMA nodes (e.g., GTF_GLOB_REF). while (!commas.Empty()) { GenTree* comma = commas.Pop(); comma->gtType = op1->gtType; - comma->gtFlags |= op1->gtFlags; #ifdef DEBUG comma->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; #endif @@ -13911,11 +13767,7 @@ GenTree* Compiler::fgOptimizeMultiply(GenTreeOp* mul) if (op2->IsIntegralConst()) { - ssize_t mult = op2->AsIntConCommon()->IconValue(); - bool op2IsConstIndex = op2->OperGet() == GT_CNS_INT && op2->AsIntCon()->gtFieldSeq != nullptr && - op2->AsIntCon()->gtFieldSeq->IsConstantIndexFieldSeq(); - - assert(!op2IsConstIndex || op2->AsIntCon()->gtFieldSeq->GetNext() == nullptr); + ssize_t mult = op2->AsIntConCommon()->IconValue(); if (mult == 0) { @@ -13956,22 +13808,6 @@ GenTree* Compiler::fgOptimizeMultiply(GenTreeOp* mul) fgMorphTreeDone(op1); } - // If "op2" is a constant array index, the other multiplicand must be a constant. - // Transfer the annotation to the other one. - if (op2->OperGet() == GT_CNS_INT && op2->AsIntCon()->gtFieldSeq != nullptr && - op2->AsIntCon()->gtFieldSeq->IsConstantIndexFieldSeq()) - { - assert(op2->AsIntCon()->gtFieldSeq->GetNext() == nullptr); - GenTree* otherOp = op1; - if (otherOp->OperGet() == GT_NEG) - { - otherOp = otherOp->AsOp()->gtOp1; - } - assert(otherOp->OperGet() == GT_CNS_INT); - assert(otherOp->AsIntCon()->gtFieldSeq == FieldSeqStore::NotAField()); - otherOp->AsIntCon()->gtFieldSeq = op2->AsIntCon()->gtFieldSeq; - } - if (abs_mult == 1) { DEBUG_DESTROY_NODE(op2); @@ -13998,15 +13834,8 @@ GenTree* Compiler::fgOptimizeMultiply(GenTreeOp* mul) fgMorphTreeDone(op1); } - GenTree* factorIcon = gtNewIconNode(factor, mul->TypeGet()); - if (op2IsConstIndex) - { - factorIcon->AsIntCon()->gtFieldSeq = - GetFieldSeqStore()->CreateSingleton(FieldSeqStore::ConstantIndexPseudoField); - } - // change the multiplication into a smaller multiplication (by 3, 5 or 9) and a shift - op1 = gtNewOperNode(GT_MUL, mul->TypeGet(), op1, factorIcon); + op1 = gtNewOperNode(GT_MUL, mul->TypeGet(), op1, gtNewIconNode(factor, mul->TypeGet())); mul->gtOp1 = op1; fgMorphTreeDone(op1); @@ -14617,16 +14446,7 @@ GenTree* Compiler::fgMorphSmpOpOptional(GenTreeOp* tree) // we are reusing the shift amount node here, but the type we want is that of the shift result op2->gtType = op1->gtType; op2->AsIntConCommon()->SetValueTruncating(iadd << ishf); - - if (cns->gtOper == GT_CNS_INT && cns->AsIntCon()->gtFieldSeq != nullptr && - cns->AsIntCon()->gtFieldSeq->IsConstantIndexFieldSeq()) - { - assert(cns->AsIntCon()->gtFieldSeq->GetNext() == nullptr); - op2->AsIntCon()->gtFieldSeq = cns->AsIntCon()->gtFieldSeq; - } - op1->ChangeOper(GT_LSH); - cns->AsIntConCommon()->SetIconValue(ishf); } } diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index f1a950d66d887..5b28585d5a70d 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -263,9 +263,6 @@ void MorphInitBlockHelper::PrepareDst() noway_assert(dstAddr->TypeIs(TYP_BYREF, TYP_I_IMPL)); if (dstAddr->IsLocalAddrExpr(m_comp, &m_dstLclNode, &m_dstFldSeq, &m_dstAddOff)) { - // Don't expect `IsLocalAddrExpr` to pass `INDEX_ADDR`. - assert((m_dst->gtFlags & GTF_IND_ARR_INDEX) == 0); - // Note that lclNode can be a field, like `BLK<4> struct(ADD(ADDR(LCL_FLD int), CNST_INT))`. m_dstVarDsc = m_comp->lvaGetDesc(m_dstLclNode); } diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 0c66dd39a1773..3a03220ddb0e3 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -549,19 +549,13 @@ unsigned int ObjectAllocator::MorphAllocObjNodeIntoStackAlloc(GenTreeAllocObj* a //------------------------------------------------------------------------ // STMTx (IL 0x... ???) // * ASG long - // +--* FIELD long #PseudoField:0x0 - // | \--* ADDR byref - // | \--* LCL_VAR struct + // +--* LCL_FLD long // \--* CNS_INT(h) long //------------------------------------------------------------------------ - // Create a local representing the object - GenTree* tree = comp->gtNewLclvNode(lclNum, TYP_STRUCT); - - // Add a pseudo-field for the method table pointer and initialize it - tree = comp->gtNewOperNode(GT_ADDR, TYP_BYREF, tree); - tree = comp->gtNewFieldRef(TYP_I_IMPL, FieldSeqStore::FirstElemPseudoField, tree, 0); - tree = comp->gtNewAssignNode(tree, allocObj->gtGetOp1()); + // Initialize the method table pointer. + GenTree* tree = comp->gtNewLclFldNode(lclNum, TYP_I_IMPL, 0); + tree = comp->gtNewAssignNode(tree, allocObj->gtGetOp1()); Statement* newStmt = comp->gtNewStmt(tree); diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 31c2eb672dafe..8b8e7895a4f93 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -7804,8 +7804,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) if (lhs->OperGet() == GT_IND) { - GenTree* arg = lhs->AsOp()->gtOp1->gtEffectiveVal(/*commaOnly*/ true); - FieldSeqNode* fldSeqArrElem = nullptr; + GenTree* arg = lhs->AsOp()->gtOp1->gtEffectiveVal(/*commaOnly*/ true); if ((tree->gtFlags & GTF_IND_VOLATILE) != 0) { @@ -7813,8 +7812,6 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) continue; } - ArrayInfo arrInfo; - if (arg->TypeGet() == TYP_BYREF && arg->OperGet() == GT_LCL_VAR) { // If it's a local byref for which we recorded a value number, use that... @@ -7840,24 +7837,25 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) // Otherwise... memoryHavoc |= memoryKindSet(GcHeap, ByrefExposed); } - // Is the LHS an array index expression? - else if (lhs->ParseArrayElemForm(this, &arrInfo, &fldSeqArrElem)) - { - // We actually ignore "fldSeq" -- any modification to an S[], at any - // field of "S", will lose all information about the array type. - CORINFO_CLASS_HANDLE elemTypeEq = EncodeElemType(arrInfo.m_elemType, arrInfo.m_elemStructType); - AddModifiedElemTypeAllContainingLoops(mostNestedLoop, elemTypeEq); - // Conservatively assume byrefs may alias this array element - memoryHavoc |= memoryKindSet(ByrefExposed); - } else { - GenTree* baseAddr = nullptr; - FieldSeqNode* fldSeq = nullptr; - if (arg->IsFieldAddr(this, &baseAddr, &fldSeq)) + GenTreeArrAddr* arrAddr = nullptr; + GenTree* baseAddr = nullptr; + FieldSeqNode* fldSeq = nullptr; + + if (arg->IsArrayAddr(&arrAddr)) { - assert((fldSeq != nullptr) && (fldSeq != FieldSeqStore::NotAField()) && - !fldSeq->IsPseudoField()); + // We will not collect "fldSeq" -- any modification to an S[], at + // any field of "S", will lose all information about the array type. + CORINFO_CLASS_HANDLE elemTypeEq = + EncodeElemType(arrAddr->GetElemType(), arrAddr->GetElemClassHandle()); + AddModifiedElemTypeAllContainingLoops(mostNestedLoop, elemTypeEq); + // Conservatively assume byrefs may alias this array element + memoryHavoc |= memoryKindSet(ByrefExposed); + } + else if (arg->IsFieldAddr(this, &baseAddr, &fldSeq)) + { + assert((fldSeq != nullptr) && (fldSeq != FieldSeqStore::NotAField())); FieldKindForVN fieldKind = (baseAddr != nullptr) ? FieldKindForVN::WithBaseAddr : FieldKindForVN::SimpleStatic; @@ -7924,32 +7922,20 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) tree->gtVNPair = tree->AsOp()->gtOp2->gtVNPair; break; - case GT_ADDR: - // Is it an addr of a array index expression? - { - GenTree* addrArg = tree->AsOp()->gtOp1; - if (addrArg->OperGet() == GT_IND) - { - // Is the LHS an array index expression? - if (addrArg->gtFlags & GTF_IND_ARR_INDEX) - { - ArrayInfo arrInfo; - bool b = GetArrayInfoMap()->Lookup(addrArg, &arrInfo); - assert(b); - CORINFO_CLASS_HANDLE elemTypeEq = - EncodeElemType(arrInfo.m_elemType, arrInfo.m_elemStructType); - ValueNum elemTypeEqVN = - vnStore->VNForHandle(ssize_t(elemTypeEq), GTF_ICON_CLASS_HDL); - ValueNum ptrToArrElemVN = - vnStore->VNForFunc(TYP_BYREF, VNF_PtrToArrElem, elemTypeEqVN, - // The rest are dummy arguments. - vnStore->VNForNull(), vnStore->VNForNull(), - vnStore->VNForNull()); - tree->gtVNPair.SetBoth(ptrToArrElemVN); - } - } - } - break; + // Is it an addr of an array index expression? + case GT_ARR_ADDR: + { + CORINFO_CLASS_HANDLE elemTypeEq = + EncodeElemType(tree->AsArrAddr()->GetElemType(), tree->AsArrAddr()->GetElemClassHandle()); + ValueNum elemTypeEqVN = vnStore->VNForHandle(ssize_t(elemTypeEq), GTF_ICON_CLASS_HDL); + + // Label this with a "dummy" PtrToArrElem so that we pick it up when looking at the ASG. + ValueNum ptrToArrElemVN = + vnStore->VNForFunc(TYP_BYREF, VNF_PtrToArrElem, elemTypeEqVN, vnStore->VNForNull(), + vnStore->VNForNull(), vnStore->VNForNull()); + tree->gtVNPair.SetBoth(ptrToArrElemVN); + } + break; #ifdef FEATURE_HW_INTRINSICS case GT_HWINTRINSIC: diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index 9b357e2668592..43a0f71806b44 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -595,7 +595,8 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge break; case GT_BOX: - // GT_BOX at this level just passes through so get rid of it + case GT_ARR_ADDR: + // BOX/ARR_ADDR at this level are just NOPs. use.ReplaceWith(node->gtGetOp1()); BlockRange().Remove(node); break; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 681a54eed8a8b..27d0059552241 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4011,7 +4011,6 @@ ValueNum ValueNumStore::VNApplySelectors(ValueNumKind vnk, for (FieldSeqNode* field = fieldSeq; field != nullptr; field = field->GetNext()) { assert(field != FieldSeqStore::NotAField()); - assert(!field->IsPseudoField()); JITDUMP(" VNApplySelectors:\n"); var_types fieldType; @@ -4170,7 +4169,6 @@ ValueNum ValueNumStore::VNApplySelectorsAssign( else { assert(fieldSeq != FieldSeqStore::NotAField()); - assert(!fieldSeq->IsPseudoField()); if (fieldSeq->GetNext() == nullptr) { @@ -6383,7 +6381,7 @@ static genTreeOps genTreeOpsIllegalAsVNFunc[] = {GT_IND, // When we do heap memo // These need special semantics: GT_COMMA, // == second argument (but with exception(s) from first). - GT_ADDR, GT_BOUNDS_CHECK, + GT_ADDR, GT_ARR_ADDR, GT_BOUNDS_CHECK, GT_OBJ, // May reference heap memory. GT_BLK, // May reference heap memory. GT_INIT_VAL, // Not strictly a pass-through. @@ -7955,47 +7953,11 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree) ValueNum heapVN = fgValueNumberArrIndexAssign(elemTypeEq, arrVN, inxVN, fldSeq, rhsVNPair.GetLiberal(), lhs->TypeGet()); - recordGcHeapStore(tree, heapVN DEBUGARG("ArrIndexAssign (case 1)")); - } - // It may be that we haven't parsed it yet. Try. - else if (lhs->gtFlags & GTF_IND_ARR_INDEX) - { - ArrayInfo arrInfo; - bool b = GetArrayInfoMap()->Lookup(lhs, &arrInfo); - assert(b); - ValueNum arrVN = ValueNumStore::NoVN; - ValueNum inxVN = ValueNumStore::NoVN; - FieldSeqNode* fldSeq = nullptr; - - // Try to parse it. - GenTree* arr = nullptr; - arg->ParseArrayAddress(this, &arrInfo, &arr, &inxVN, &fldSeq); - if (arr == nullptr) - { - fgMutateGcHeap(tree DEBUGARG("assignment to unparseable array expression")); - return; - } - // Otherwise, parsing succeeded. - - // Need to form H[arrType][arr][ind][fldSeq] = rhsVNPair.GetLiberal() - - // Get the element type equivalence class representative. - CORINFO_CLASS_HANDLE elemTypeEq = EncodeElemType(arrInfo.m_elemType, arrInfo.m_elemStructType); - arrVN = arr->gtVNPair.GetLiberal(); - - FieldSeqNode* zeroOffsetFldSeq = nullptr; - if (GetZeroOffsetFieldMap()->Lookup(arg, &zeroOffsetFldSeq)) - { - fldSeq = GetFieldSeqStore()->Append(fldSeq, zeroOffsetFldSeq); - } - - ValueNum heapVN = fgValueNumberArrIndexAssign(elemTypeEq, arrVN, inxVN, fldSeq, - rhsVNPair.GetLiberal(), lhs->TypeGet()); - recordGcHeapStore(tree, heapVN DEBUGARG("ArrIndexAssign (case 2)")); + recordGcHeapStore(tree, heapVN DEBUGARG("ArrIndexAssign")); } else if (arg->IsFieldAddr(this, &baseAddr, &fldSeq)) { - assert((fldSeq != nullptr) && (fldSeq != FieldSeqStore::NotAField()) && !fldSeq->IsPseudoField()); + assert((fldSeq != nullptr) && (fldSeq != FieldSeqStore::NotAField())); // The value number from the rhs of the assignment ValueNum storeVal = rhsVNPair.GetLiberal(); @@ -8798,81 +8760,6 @@ void Compiler::fgValueNumberTree(GenTree* tree) ValueNum newUniq = vnStore->VNForExpr(compCurBB, tree->TypeGet()); tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(newUniq, newUniq), addrXvnp); } - // We always want to evaluate the LHS when the GT_IND node is marked with GTF_IND_ARR_INDEX - // as this will relabel the GT_IND child correctly using the VNF_PtrToArrElem - else if ((tree->gtFlags & GTF_IND_ARR_INDEX) != 0) - { - ArrayInfo arrInfo; - bool b = GetArrayInfoMap()->Lookup(tree, &arrInfo); - assert(b); - - ValueNum inxVN = ValueNumStore::NoVN; - FieldSeqNode* fldSeq = nullptr; - - // Try to parse it. - GenTree* arr = nullptr; - addr->ParseArrayAddress(this, &arrInfo, &arr, &inxVN, &fldSeq); - if (arr != nullptr) - { - assert(fldSeq != FieldSeqStore::NotAField()); - - // Need to form H[arrType][arr][ind][fldSeq] - // Get the array element type equivalence class rep. - CORINFO_CLASS_HANDLE elemTypeEq = EncodeElemType(arrInfo.m_elemType, arrInfo.m_elemStructType); - ValueNum elemTypeEqVN = vnStore->VNForHandle(ssize_t(elemTypeEq), GTF_ICON_CLASS_HDL); - JITDUMP(" VNForHandle(arrElemType: %s) is " FMT_VN "\n", - (arrInfo.m_elemType == TYP_STRUCT) ? eeGetClassName(arrInfo.m_elemStructType) - : varTypeName(arrInfo.m_elemType), - elemTypeEqVN); - - // We take the "VNNormalValue"s here, because if either has exceptional outcomes, they will - // be captured as part of the value of the composite "addr" operation... - ValueNum arrVN = vnStore->VNLiberalNormalValue(arr->gtVNPair); - inxVN = vnStore->VNNormalValue(inxVN); - - // Additionally, relabel the address with a PtrToArrElem value number. - ValueNum fldSeqVN = vnStore->VNForFieldSeq(fldSeq); - ValueNum elemAddr = - vnStore->VNForFunc(TYP_BYREF, VNF_PtrToArrElem, elemTypeEqVN, arrVN, inxVN, fldSeqVN); - - // The aggregate "addr" VN should have had all the exceptions bubble up... - addr->gtVNPair = vnStore->VNPWithExc(ValueNumPair(elemAddr, elemAddr), addrXvnp); -#ifdef DEBUG - ValueNum elemAddrWithExc = addr->gtVNPair.GetLiberal(); - if (verbose) - { - printf(" Relabeled IND_ARR_INDEX address node "); - Compiler::printTreeID(addr); - printf(" with l:" FMT_VN ": ", elemAddrWithExc); - vnStore->vnDump(this, elemAddrWithExc); - printf("\n"); - if (elemAddrWithExc != elemAddr) - { - printf(" [" FMT_VN " is: ", elemAddr); - vnStore->vnDump(this, elemAddr); - printf("]\n"); - } - } -#endif // DEBUG - - // We now need to retrieve the value number for the array element value - // and give this value number to the GT_IND node 'tree' - // We do this whenever we have an rvalue, but we don't do it for a - // normal LHS assignment into an array element. - // - if ((tree->gtFlags & GTF_IND_ASG_LHS) == 0) - { - fgValueNumberArrIndexVal(tree, elemTypeEq, arrVN, inxVN, addrXvnp, fldSeq); - } - } - else // An unparseable array expression. - { - if ((tree->gtFlags & GTF_IND_ASG_LHS) == 0) - { - tree->gtVNPair = vnStore->VNPUniqueWithExc(tree->TypeGet(), addrXvnp); - } - } - } // In general we skip GT_IND nodes on that are the LHS of an assignment. (We labeled these earlier.) // We will "evaluate" this as part of the assignment. else if ((tree->gtFlags & GTF_IND_ASG_LHS) == 0) @@ -8899,7 +8786,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) } tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp); } - else if (vnStore->GetVNFunc(addrNvnp.GetLiberal(), &funcApp) && funcApp.m_func == VNF_PtrToStatic) + else if (vnStore->GetVNFunc(addrNvnp.GetLiberal(), &funcApp) && (funcApp.m_func == VNF_PtrToStatic)) { var_types indType = tree->TypeGet(); ValueNum fieldSeqVN = funcApp.m_args[1]; @@ -8933,7 +8820,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) } else if (addr->IsFieldAddr(this, &baseAddr, &fldSeq)) { - assert((fldSeq != nullptr) && (fldSeq != FieldSeqStore::NotAField()) && !fldSeq->IsPseudoField()); + assert((fldSeq != nullptr) && (fldSeq != FieldSeqStore::NotAField())); // The size of the ultimate value we will select, if it is of a struct type. size_t structSize = 0; @@ -9109,6 +8996,10 @@ void Compiler::fgValueNumberTree(GenTree* tree) } break; + case GT_ARR_ADDR: + fgValueNumberArrIndexAddr(tree->AsArrAddr()); + break; + case GT_BOUNDS_CHECK: { ValueNumPair vnpIndex = tree->AsBoundsChk()->GetIndex()->gtVNPair; @@ -9390,6 +9281,48 @@ void Compiler::fgValueNumberIntrinsic(GenTree* tree) } } +//------------------------------------------------------------------------ +// fgValueNumberArrIndexAddr: Numbers the ARR_ADDR tree using VNF_PtrToArrElem. +// +// Arguments: +// arrAddr - The GT_ARR_ADDR tree to number +// +void Compiler::fgValueNumberArrIndexAddr(GenTreeArrAddr* arrAddr) +{ + GenTree* arr = nullptr; + ValueNum inxVN = ValueNumStore::NoVN; + arrAddr->ParseArrayAddress(this, &arr, &inxVN); + + if (arr == nullptr) + { + JITDUMP(" *** ARR_ADDR -- an unparseable array expression, assigning a new, unique VN\n"); + arrAddr->gtVNPair = vnStore->VNPUniqueWithExc(TYP_BYREF, vnStore->VNPExceptionSet(arrAddr->Addr()->gtVNPair)); + return; + } + + // Get the element type equivalence class representative. + var_types elemType = arrAddr->GetElemType(); + CORINFO_CLASS_HANDLE elemStructType = arrAddr->GetElemClassHandle(); + CORINFO_CLASS_HANDLE elemTypeEq = EncodeElemType(elemType, elemStructType); + ValueNum elemTypeEqVN = vnStore->VNForHandle(ssize_t(elemTypeEq), GTF_ICON_CLASS_HDL); + JITDUMP(" VNForHandle(arrElemType: %s) is " FMT_VN "\n", + (elemType == TYP_STRUCT) ? eeGetClassName(elemStructType) : varTypeName(elemType), elemTypeEqVN); + + ValueNum arrVN = vnStore->VNNormalValue(arr->GetVN(VNK_Liberal)); + inxVN = vnStore->VNNormalValue(inxVN); + + // Look up if we have any zero-offset sequences... + assert(!GetZeroOffsetFieldMap()->Lookup(arrAddr->Addr())); + + FieldSeqNode* fldSeq = nullptr; + GetZeroOffsetFieldMap()->Lookup(arrAddr, &fldSeq); + ValueNum fldSeqVN = vnStore->VNForFieldSeq(fldSeq); + + ValueNum arrAddrVN = vnStore->VNForFunc(TYP_BYREF, VNF_PtrToArrElem, elemTypeEqVN, arrVN, inxVN, fldSeqVN); + ValueNumPair arrAddrVNP = ValueNumPair(arrAddrVN, arrAddrVN); + arrAddr->gtVNPair = vnStore->VNPWithExc(arrAddrVNP, vnStore->VNPExceptionSet(arrAddr->Addr()->gtVNPair)); +} + #ifdef FEATURE_SIMD // Does value-numbering for a GT_SIMD node. void Compiler::fgValueNumberSimd(GenTreeSIMD* tree)