From c56d519fb516b74d5eaae4a6817badc6cd4ee348 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Wed, 11 May 2022 21:51:52 +0300 Subject: [PATCH 1/4] Do not set NO_CSE on ARR_ADDRs It is effectively no-CSE already because of how "optIsCSECandidate" works. --- src/coreclr/jit/gentree.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index df6452fb4e72eb..5f34cce9bda79e 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -6363,10 +6363,6 @@ struct GenTreeArrAddr : GenTreeUnOp 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 From cb2c63712f8f9d3ac482caa8d1476faa16126890 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Wed, 2 Feb 2022 20:55:04 +0300 Subject: [PATCH 2/4] Delete GT_INDEX Instead: 1) For "ldelem", import "IND/OBJ(INDEX_ADDR)". 2) For "ldelema", import "INDEX_ADDR". This deletes two usages of "ADDR": 1) "OBJ(ADDR(INDEX))" from "ldelem". 2) "ADDR(INDEX)" from "ldelema". --- src/coreclr/jit/compiler.cpp | 7 - src/coreclr/jit/compiler.h | 17 +- src/coreclr/jit/compiler.hpp | 46 +++++- src/coreclr/jit/flowgraph.cpp | 5 +- src/coreclr/jit/gentree.cpp | 118 +++++++------- src/coreclr/jit/gentree.h | 72 +++------ src/coreclr/jit/gtlist.h | 4 +- src/coreclr/jit/gtstructs.h | 1 - src/coreclr/jit/importer.cpp | 181 ++++++--------------- src/coreclr/jit/lclmorph.cpp | 5 - src/coreclr/jit/liveness.cpp | 1 - src/coreclr/jit/loopcloning.cpp | 23 +-- src/coreclr/jit/loopcloning.h | 4 +- src/coreclr/jit/morph.cpp | 271 +++++++++++--------------------- src/coreclr/jit/simd.cpp | 55 +++---- 15 files changed, 315 insertions(+), 495 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 5702221c02294a..2201c3a7d1e8d9 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -9373,13 +9373,6 @@ void cTreeFlags(Compiler* comp, GenTree* tree) } break; - case GT_INDEX: - - if (tree->gtFlags & GTF_INX_STRING_LAYOUT) - { - chars += printf("[INX_STRING_LAYOUT]"); - } - FALLTHROUGH; case GT_INDEX_ADDR: if (tree->gtFlags & GTF_INX_RNGCHK) { diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 60c8041555930c..3a2d81b2905327 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2587,7 +2587,19 @@ class Compiler GenTreeField* gtNewFieldRef(var_types type, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj = nullptr, DWORD offset = 0); - GenTree* gtNewIndexRef(var_types typ, GenTree* arrayOp, GenTree* indexOp); + GenTreeIndexAddr* gtNewIndexAddr(GenTree* arrayOp, + GenTree* indexOp, + var_types elemType, + CORINFO_CLASS_HANDLE elemClassHandle, + unsigned firstElemOffset, + unsigned lengthOffset); + + GenTreeIndexAddr* gtNewArrayIndexAddr(GenTree* arrayOp, + GenTree* indexOp, + var_types elemType, + CORINFO_CLASS_HANDLE elemClassHandle); + + GenTreeIndir* gtNewIndexIndir(GenTreeIndexAddr* indexAddr); GenTreeArrLen* gtNewArrLen(var_types typ, GenTree* arrayOp, int lenOffset, BasicBlock* block); @@ -2758,6 +2770,7 @@ class Compiler GenTree* gtFoldExpr(GenTree* tree); GenTree* gtFoldExprConst(GenTree* tree); + GenTree* gtFoldIndirConst(GenTreeIndir* indir); GenTree* gtFoldExprSpecial(GenTree* tree); GenTree* gtFoldBoxNullable(GenTree* tree); GenTree* gtFoldExprCompare(GenTree* tree); @@ -5627,7 +5640,7 @@ class Compiler Statement* fgPreviousCandidateSIMDFieldAsgStmt; #endif // FEATURE_SIMD - GenTree* fgMorphArrayIndex(GenTree* tree); + GenTree* fgMorphIndexAddr(GenTreeIndexAddr* tree); GenTree* fgMorphExpandCast(GenTreeCast* tree); GenTreeFieldList* fgMorphLclArgToFieldlist(GenTreeLclVarCommon* lcl); GenTreeCall* fgMorphArgs(GenTreeCall* call); diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index bda0cc6d73b533..2fe7c10574c6b0 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -1145,16 +1145,48 @@ inline GenTreeField* Compiler::gtNewFieldRef(var_types type, CORINFO_FIELD_HANDL return fieldNode; } -/***************************************************************************** - * - * A little helper to create an array index node. - */ +inline GenTreeIndexAddr* Compiler::gtNewIndexAddr(GenTree* arrayOp, + GenTree* indexOp, + var_types elemType, + CORINFO_CLASS_HANDLE elemClassHandle, + unsigned firstElemOffset, + unsigned lengthOffset) +{ + unsigned elemSize = + (elemType == TYP_STRUCT) ? info.compCompHnd->getClassSize(elemClassHandle) : genTypeSize(elemType); + + GenTreeIndexAddr* indexAddr = new (this, GT_INDEX_ADDR) + GenTreeIndexAddr(arrayOp, indexOp, elemType, elemClassHandle, elemSize, lengthOffset, firstElemOffset); -inline GenTree* Compiler::gtNewIndexRef(var_types typ, GenTree* arrayOp, GenTree* indexOp) + return indexAddr; +} + +inline GenTreeIndexAddr* Compiler::gtNewArrayIndexAddr(GenTree* arrayOp, + GenTree* indexOp, + var_types elemType, + CORINFO_CLASS_HANDLE elemClassHandle) { - GenTreeIndex* gtIndx = new (this, GT_INDEX) GenTreeIndex(typ, arrayOp, indexOp, genTypeSize(typ)); + unsigned lengthOffset = OFFSETOF__CORINFO_Array__length; + unsigned firstElemOffset = OFFSETOF__CORINFO_Array__data; + + return gtNewIndexAddr(arrayOp, indexOp, elemType, elemClassHandle, firstElemOffset, lengthOffset); +} + +inline GenTreeIndir* Compiler::gtNewIndexIndir(GenTreeIndexAddr* indexAddr) +{ + GenTreeIndir* index; + if (varTypeIsStruct(indexAddr->gtElemType)) + { + index = gtNewObjNode(indexAddr->gtStructElemClass, indexAddr); + } + else + { + index = gtNewIndir(indexAddr->gtElemType, indexAddr); + } + + index->gtFlags |= GTF_GLOB_REF; - return gtIndx; + return index; } //------------------------------------------------------------------------------ diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 0f70f0d619df56..da1bcd79595aa9 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -4152,10 +4152,9 @@ void Compiler::fgSetBlockOrder(BasicBlock* block) } break; - case GT_INDEX: case GT_INDEX_ADDR: - // These two call CORINFO_HELP_RNGCHKFAIL for Debug code - if (tree->gtFlags & GTF_INX_RNGCHK) + // This calls CORINFO_HELP_RNGCHKFAIL for Debug code. + if (tree->AsIndexAddr()->IsBoundsChecked()) { return Compiler::WALK_ABORT; } diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 9e196da018b35e..143dadd23549fd 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -256,7 +256,6 @@ void GenTree::InitNodeSize() GenTree::s_gtNodeSizes[GT_CAST] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_FTN_ADDR] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_BOX] = TREE_NODE_SZ_LARGE; - GenTree::s_gtNodeSizes[GT_INDEX] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_INDEX_ADDR] = TREE_NODE_SZ_LARGE; GenTree::s_gtNodeSizes[GT_BOUNDS_CHECK] = TREE_NODE_SZ_SMALL; GenTree::s_gtNodeSizes[GT_ARR_ELEM] = TREE_NODE_SZ_LARGE; @@ -316,7 +315,6 @@ void GenTree::InitNodeSize() static_assert_no_msg(sizeof(GenTreeFptrVal) <= TREE_NODE_SZ_LARGE); // *** large node static_assert_no_msg(sizeof(GenTreeQmark) <= TREE_NODE_SZ_LARGE); // *** large node static_assert_no_msg(sizeof(GenTreeIntrinsic) <= TREE_NODE_SZ_LARGE); // *** large node - static_assert_no_msg(sizeof(GenTreeIndex) <= TREE_NODE_SZ_LARGE); // *** large node static_assert_no_msg(sizeof(GenTreeIndexAddr) <= TREE_NODE_SZ_LARGE); // *** large node static_assert_no_msg(sizeof(GenTreeArrLen) <= TREE_NODE_SZ_LARGE); // *** large node static_assert_no_msg(sizeof(GenTreeBoundsChk) <= TREE_NODE_SZ_SMALL); @@ -2495,12 +2493,6 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK) return false; } break; - case GT_INDEX: - if (op1->AsIndex()->gtIndElemSize != op2->AsIndex()->gtIndElemSize) - { - return false; - } - break; case GT_INDEX_ADDR: if (op1->AsIndexAddr()->gtElemSize != op2->AsIndexAddr()->gtElemSize) { @@ -2917,9 +2909,6 @@ unsigned Compiler::gtHashValue(GenTree* tree) case GT_CAST: hash ^= tree->AsCast()->gtCastType; break; - case GT_INDEX: - hash += tree->AsIndex()->gtIndElemSize; - break; case GT_INDEX_ADDR: hash += tree->AsIndexAddr()->gtElemSize; break; @@ -2989,7 +2978,6 @@ unsigned Compiler::gtHashValue(GenTree* tree) // For the ones below no extra argument matters for comparison. case GT_ARR_INDEX: case GT_QMARK: - case GT_INDEX: case GT_INDEX_ADDR: break; @@ -6397,7 +6385,6 @@ bool GenTree::OperMayThrow(Compiler* comp) case GT_ARR_OFFSET: case GT_LCLHEAP: case GT_CKFINITE: - case GT_INDEX: case GT_INDEX_ADDR: return true; @@ -8287,15 +8274,6 @@ GenTree* Compiler::gtCloneExpr( tree->AsCast()->gtCastType DEBUGARG(/*largeNode*/ TRUE)); break; - case GT_INDEX: - { - GenTreeIndex* asInd = tree->AsIndex(); - copy = new (this, GT_INDEX) - GenTreeIndex(asInd->TypeGet(), asInd->Arr(), asInd->Index(), asInd->gtIndElemSize); - copy->AsIndex()->gtStructElemClass = asInd->gtStructElemClass; - } - break; - case GT_INDEX_ADDR: { GenTreeIndexAddr* asIndAddr = tree->AsIndexAddr(); @@ -10171,8 +10149,6 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ } FALLTHROUGH; - case GT_INDEX: - case GT_INDEX_ADDR: case GT_FIELD: if (tree->gtFlags & GTF_IND_VOLATILE) { @@ -10468,19 +10444,6 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ { layout = tree->AsLclFld()->GetLayout(); } - else if (tree->OperIs(GT_INDEX)) - { - GenTreeIndex* asInd = tree->AsIndex(); - CORINFO_CLASS_HANDLE clsHnd = asInd->gtStructElemClass; - if (clsHnd != nullptr) - { - // 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) { @@ -10488,15 +10451,21 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ } } - if (tree->OperIs(GT_ARR_ADDR)) + if (tree->OperIs(GT_INDEX_ADDR, GT_ARR_ADDR)) { - if (tree->AsArrAddr()->GetElemClassHandle() != NO_CLASS_HANDLE) + var_types elemType = + tree->OperIs(GT_INDEX_ADDR) ? tree->AsIndexAddr()->gtElemType : tree->AsArrAddr()->GetElemType(); + + CORINFO_CLASS_HANDLE elemClsHnd = tree->OperIs(GT_INDEX_ADDR) ? tree->AsIndexAddr()->gtStructElemClass + : tree->AsArrAddr()->GetElemClassHandle(); + + if (varTypeIsStruct(elemType) && (elemClsHnd != NO_CLASS_HANDLE)) { - printf("%S[]", eeGetShortClassName(tree->AsArrAddr()->GetElemClassHandle())); + printf("%S[]", eeGetShortClassName(elemClsHnd)); } else { - printf("%s[]", varTypeName(tree->AsArrAddr()->GetElemType())); + printf("%s[]", varTypeName(elemType)); } } @@ -15108,6 +15077,48 @@ GenTree* Compiler::gtFoldExprConst(GenTree* tree) #pragma warning(pop) #endif +//------------------------------------------------------------------------ +// gtFoldIndirConst: Attempt to fold an "IND(addr)" expression to a constant. +// +// Currently handles the case of "addr" being "INDEX_ADDR(CNS_STR, CONST)". +// +// Arguments: +// indir - The IND node to attempt to fold +// +// Return Value: +// The new constant node if the folding was successful, "nullptr" otherwise. +// +GenTree* Compiler::gtFoldIndirConst(GenTreeIndir* indir) +{ + assert(opts.OptimizationEnabled() && !optValnumCSE_phase); + assert(indir->OperIs(GT_IND)); + + GenTree* addr = indir->Addr(); + + if (indir->TypeIs(TYP_USHORT) && addr->OperIs(GT_INDEX_ADDR) && addr->AsIndexAddr()->Arr()->OperIs(GT_CNS_STR)) + { + GenTreeStrCon* stringNode = addr->AsIndexAddr()->Arr()->AsStrCon(); + GenTree* indexNode = addr->AsIndexAddr()->Index(); + if (!stringNode->IsStringEmptyField() && indexNode->IsCnsIntOrI()) + { + int cnsIndex = static_cast(indexNode->AsIntConCommon()->IconValue()); + if (cnsIndex >= 0) + { + const int maxStrSize = 1024; + char16_t str[maxStrSize]; + int length = + info.compCompHnd->getStringLiteral(stringNode->gtScpHnd, stringNode->gtSconCPX, str, maxStrSize); + if (cnsIndex < length) + { + return gtNewIconNode(str[cnsIndex]); + } + } + } + } + + return nullptr; +} + //------------------------------------------------------------------------ // gtNewTempAssign: Create an assignment of the given value to a temp. // @@ -17173,9 +17184,6 @@ CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleIfPresent(GenTree* tree) case GT_RET_EXPR: structHnd = tree->AsRetExpr()->gtRetClsHnd; break; - case GT_INDEX: - structHnd = tree->AsIndex()->gtStructElemClass; - break; case GT_FIELD: info.compCompHnd->getFieldType(tree->AsField()->gtFldHnd, &structHnd); break; @@ -17515,13 +17523,19 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b objClass = lvaTable[objLcl].lvClassHnd; *pIsExact = lvaTable[objLcl].lvClassIsExact; } - else if (base->OperGet() == GT_ARR_ELEM) + else if (base->OperIs(GT_INDEX_ADDR, GT_ARR_ELEM)) { // indir(arr_elem(...)) -> array element type - GenTree* array = base->AsArrElem()->gtArrObj; + if (base->OperIs(GT_INDEX_ADDR)) + { + objClass = gtGetArrayElementClassHandle(base->AsIndexAddr()->Arr()); + } + else + { + objClass = gtGetArrayElementClassHandle(base->AsArrElem()->gtArrObj); + } - objClass = gtGetArrayElementClassHandle(array); *pIsExact = false; *pIsNonNull = false; } @@ -17577,16 +17591,6 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b break; } - case GT_INDEX: - { - GenTree* array = obj->AsIndex()->Arr(); - - objClass = gtGetArrayElementClassHandle(array); - *pIsExact = false; - *pIsNonNull = false; - break; - } - default: { break; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 5f34cce9bda79e..9aff6df4bed4c9 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -505,9 +505,8 @@ enum GenTreeFlags : unsigned int GTF_FLD_INITCLASS = 0x20000000, // GT_FIELD -- field access requires preceding class/static init helper GTF_FLD_TGT_HEAP = 0x10000000, // GT_FIELD -- same as GTF_IND_TGT_HEAP - GTF_INX_RNGCHK = 0x80000000, // GT_INDEX/GT_INDEX_ADDR -- the array reference should be range-checked. - GTF_INX_STRING_LAYOUT = 0x40000000, // GT_INDEX -- this uses the special string array layout - GTF_INX_NOFAULT = 0x20000000, // GT_INDEX -- the INDEX does not throw an exception (morph to GTF_IND_NONFAULTING) + GTF_INX_RNGCHK = 0x80000000, // GT_INDEX_ADDR -- this array address should be range-checked + GTF_INX_ADDR_NONNULL = 0x40000000, // GT_INDEX_ADDR -- this array address is not null GTF_IND_TGT_NOT_HEAP = 0x80000000, // GT_IND -- the target is not on the heap GTF_IND_VOLATILE = 0x40000000, // GT_IND -- the load or store must use volatile sematics (this is a nop on X86) @@ -2078,6 +2077,17 @@ struct GenTree gtFlags |= sourceFlags; } + void AddAllEffectsFlags(GenTree* firstSource, GenTree* secondSource) + { + AddAllEffectsFlags((firstSource->gtFlags | secondSource->gtFlags) & GTF_ALL_EFFECT); + } + + void AddAllEffectsFlags(GenTreeFlags sourceFlags) + { + assert((sourceFlags & ~GTF_ALL_EFFECT) == 0); + gtFlags |= sourceFlags; + } + inline bool IsCnsIntOrI() const; inline bool IsIntegralConst() const; @@ -6231,50 +6241,9 @@ struct GenTreeHWIntrinsic : public GenTreeJitIntrinsic }; #endif // FEATURE_HW_INTRINSICS -/* gtIndex -- array access */ - -struct GenTreeIndex : public GenTreeOp -{ - GenTree*& Arr() - { - return gtOp1; - } - GenTree*& Index() - { - return gtOp2; - } - - unsigned gtIndElemSize; // size of elements in the array - CORINFO_CLASS_HANDLE gtStructElemClass; // If the element type is a struct, this is the struct type. - - GenTreeIndex(var_types type, GenTree* arr, GenTree* ind, unsigned indElemSize) - : GenTreeOp(GT_INDEX, type, arr, ind) - , gtIndElemSize(indElemSize) - , gtStructElemClass(nullptr) // We always initialize this after construction. - { -#ifdef DEBUG - if (JitConfig.JitSkipArrayBoundCheck() == 1) - { - // Skip bounds check - } - else -#endif - { - // Do bounds check - gtFlags |= GTF_INX_RNGCHK; - } - - gtFlags |= GTF_EXCEPT | GTF_GLOB_REF; - } -#if DEBUGGABLE_GENTREE - GenTreeIndex() : GenTreeOp() - { - } -#endif -}; - -// gtIndexAddr: given an array object and an index, checks that the index is within the bounds of the array if -// necessary and produces the address of the value at that index of the array. +// GenTreeIndexAddr: Given an array object and an index, checks that the index is within the bounds of the array if +// necessary and produces the address of the value at that index of the array. +// struct GenTreeIndexAddr : public GenTreeOp { GenTree*& Arr() @@ -6310,6 +6279,7 @@ struct GenTreeIndexAddr : public GenTreeOp , gtLenOffset(lenOffset) , gtElemOffset(elemOffset) { + assert(!varTypeIsStruct(elemType) || (structElemClass != NO_CLASS_HANDLE)); #ifdef DEBUG if (JitConfig.JitSkipArrayBoundCheck() == 1) { @@ -6338,7 +6308,7 @@ struct GenTreeIndexAddr : public GenTreeOp bool IsNotNull() const { - return IsBoundsChecked(); + return IsBoundsChecked() || ((gtFlags & GTF_INX_ADDR_NONNULL) != 0); } }; @@ -6445,8 +6415,8 @@ struct GenTreeBoundsChk : public GenTreeOp SpecialCodeKind gtThrowKind; // Kind of throw block to branch to on failure // Store some information about the array element type that was in the GT_INDEX node before morphing. - // Note that this information is also stored in the m_arrayInfoMap of the morphed IND node (that - // is marked with GTF_IND_ARR_INDEX), but that can be hard to find. + // Note that this information is also stored in the ARR_ADDR node of the morphed tree, but that can + // be hard to find. var_types gtInxType; // Save the GT_INDEX type GenTreeBoundsChk(GenTree* index, GenTree* length, SpecialCodeKind kind) @@ -8483,7 +8453,6 @@ inline GenTree* GenTree::gtGetOp1() const case GT_RSZ: case GT_ROL: case GT_ROR: - case GT_INDEX: case GT_ASG: case GT_EQ: case GT_NE: @@ -8494,6 +8463,7 @@ inline GenTree* GenTree::gtGetOp1() const case GT_COMMA: case GT_QMARK: case GT_COLON: + case GT_INDEX_ADDR: case GT_MKREFANY: return true; default: diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index 0dfda4deeb4f98..a089124825ff93 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -148,9 +148,7 @@ GTNODE(COMMA , GenTreeOp ,0,GTK_BINOP|DBK_NOTLIR) GTNODE(QMARK , GenTreeQmark ,0,GTK_BINOP|GTK_EXOP|DBK_NOTLIR) GTNODE(COLON , GenTreeColon ,0,GTK_BINOP|DBK_NOTLIR) -GTNODE(INDEX , GenTreeIndex ,0,GTK_BINOP|GTK_EXOP|DBK_NOTLIR) // SZ-array-element. -GTNODE(INDEX_ADDR , GenTreeIndexAddr ,0,GTK_BINOP|GTK_EXOP) // Addr of SZ-array-element; used when aiming to minimize compile times. - +GTNODE(INDEX_ADDR , GenTreeIndexAddr ,0,GTK_BINOP|GTK_EXOP) // Address of SZ-array-element. GTNODE(MKREFANY , GenTreeOp ,0,GTK_BINOP|DBK_NOTLIR) GTNODE(LEA , GenTreeAddrMode ,0,GTK_BINOP|GTK_EXOP|DBK_NOTHIR) diff --git a/src/coreclr/jit/gtstructs.h b/src/coreclr/jit/gtstructs.h index 1c4c554a0526be..038164cb2f7b88 100644 --- a/src/coreclr/jit/gtstructs.h +++ b/src/coreclr/jit/gtstructs.h @@ -72,7 +72,6 @@ GTSTRUCT_1(FieldList , GT_FIELD_LIST) GTSTRUCT_1(Colon , GT_COLON) GTSTRUCT_1(FptrVal , GT_FTN_ADDR) GTSTRUCT_1(Intrinsic , GT_INTRINSIC) -GTSTRUCT_1(Index , GT_INDEX) GTSTRUCT_1(IndexAddr , GT_INDEX_ADDR) #if defined(FEATURE_HW_INTRINSICS) && defined(FEATURE_SIMD) GTSTRUCT_N(MultiOp , GT_SIMD, GT_HWINTRINSIC) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 564d0ee845a61e..8065b8f56128ce 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1137,7 +1137,7 @@ GenTree* Compiler::impAssignStruct(GenTree* dest, } assert(dest->gtOper == GT_LCL_VAR || dest->gtOper == GT_RETURN || dest->gtOper == GT_FIELD || - dest->gtOper == GT_IND || dest->gtOper == GT_OBJ || dest->gtOper == GT_INDEX); + dest->gtOper == GT_IND || dest->gtOper == GT_OBJ); // Return a NOP if this is a self-assignment. if (dest->OperGet() == GT_LCL_VAR && src->OperGet() == GT_LCL_VAR && @@ -1409,11 +1409,6 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, assert(src->AsObj()->GetLayout()->GetClassHandle() == structHnd); } } - else if (src->gtOper == GT_INDEX) - { - asgType = impNormStructType(structHnd); - assert(src->AsIndex()->gtStructElemClass == structHnd); - } else if (src->gtOper == GT_MKREFANY) { // Since we are assigning the result of a GT_MKREFANY, @@ -1493,9 +1488,9 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, if ((dest == nullptr) && (destAddr->OperGet() == GT_ADDR)) { GenTree* destNode = destAddr->gtGetOp1(); - // If the actual destination is a local, a GT_INDEX or a block node, or is a node that - // will be morphed, don't insert an OBJ(ADDR) if it already has the right type. - if (destNode->OperIs(GT_LCL_VAR, GT_INDEX) || destNode->OperIsBlk()) + // If the actual destination is a local, or a block node, + // don't insert an OBJ(ADDR) if it already has the right type. + if (destNode->OperIs(GT_LCL_VAR) || destNode->OperIsBlk()) { var_types destType = destNode->TypeGet(); // If one or both types are TYP_STRUCT (one may not yet be normalized), they are compatible @@ -1706,7 +1701,7 @@ var_types Compiler::impNormStructType(CORINFO_CLASS_HANDLE structHnd, CorInfoTyp // it is either: // - a known struct type (non-TYP_STRUCT, e.g. TYP_SIMD8) // - an OBJ or a MKREFANY node, or -// - a node (e.g. GT_INDEX) that will be morphed. +// - a node (e.g. GT_FIELD) that will be morphed. // If the node is a CALL or RET_EXPR, a copy will be made to a new temp. // GenTree* Compiler::impNormStructVal(GenTree* structVal, @@ -1745,13 +1740,6 @@ GenTree* Compiler::impNormStructVal(GenTree* structVal, makeTemp = true; break; - case GT_INDEX: - // This will be transformed to an OBJ later. - alreadyNormalized = true; - structVal->AsIndex()->gtStructElemClass = structHnd; - structVal->AsIndex()->gtIndElemSize = info.compCompHnd->getClassSize(structHnd); - break; - case GT_FIELD: // Wrap it in a GT_OBJ, if needed. structVal->gtType = structType; @@ -3910,10 +3898,11 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, case NI_System_String_get_Chars: { - GenTree* op2 = impPopStack().val; - GenTree* op1 = impPopStack().val; - retNode = gtNewIndexRef(TYP_USHORT, op1, op2); - retNode->gtFlags |= GTF_INX_STRING_LAYOUT; + GenTree* op2 = impPopStack().val; + GenTree* op1 = impPopStack().val; + GenTree* addr = gtNewIndexAddr(op1, op2, TYP_USHORT, NO_CLASS_HANDLE, OFFSETOF__CORINFO_String__chars, + OFFSETOF__CORINFO_String__stringLen); + retNode = gtNewIndexIndir(addr->AsIndexAddr()); break; } @@ -12867,7 +12856,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) } CORINFO_CLASS_HANDLE clsHnd = DUMMY_INIT(NULL); - CORINFO_CLASS_HANDLE ldelemClsHnd = DUMMY_INIT(NULL); + CORINFO_CLASS_HANDLE ldelemClsHnd = NO_CLASS_HANDLE; CORINFO_CLASS_HANDLE stelemClsHnd = DUMMY_INIT(NULL); var_types lclTyp, ovflType = TYP_UNKNOWN; @@ -12936,7 +12925,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) CORINFO_SIG_INFO sig; IL_OFFSET jmpAddr; bool ovfl, unordered, callNode; - bool ldstruct; CORINFO_CLASS_HANDLE tokenType; union { @@ -13634,13 +13622,9 @@ void Compiler::impImportBlockCode(BasicBlock* block) case CEE_LDELEM_I: lclTyp = TYP_I_IMPL; goto ARR_LD; - - // Should be UINT, but since no platform widens 4->8 bytes it doesn't matter - // and treating it as TYP_INT avoids other asserts. case CEE_LDELEM_U4: lclTyp = TYP_INT; goto ARR_LD; - case CEE_LDELEM_I4: lclTyp = TYP_INT; goto ARR_LD; @@ -13666,84 +13650,32 @@ void Compiler::impImportBlockCode(BasicBlock* block) ARR_LD: ARR_LD_POST_VERIFY: - /* Pull the index value and array address */ - op2 = impPopStack().val; - op1 = impPopStack().val; - assertImp(op1->gtType == TYP_REF); + op2 = impPopStack().val; // index + op1 = impPopStack().val; // array + assertImp(op1->TypeIs(TYP_REF)); - /* Check for null pointer - in the inliner case we simply abort */ - - if (compIsForInlining()) + // Check for null pointer - in the inliner case we simply abort. + if (compIsForInlining() && op1->IsCnsIntOrI()) { - if (op1->gtOper == GT_CNS_INT) - { - compInlineResult->NoteFatal(InlineObservation::CALLEE_HAS_NULL_FOR_LDELEM); - return; - } + compInlineResult->NoteFatal(InlineObservation::CALLEE_HAS_NULL_FOR_LDELEM); + return; } - /* Mark the block as containing an index expression */ + // Mark the block as containing an index expression. - if (op1->gtOper == GT_LCL_VAR) + if (op1->OperIs(GT_LCL_VAR) && op2->OperIs(GT_LCL_VAR, GT_CNS_INT, GT_ADD)) { - if (op2->gtOper == GT_LCL_VAR || op2->gtOper == GT_CNS_INT || op2->gtOper == GT_ADD) - { - block->bbFlags |= BBF_HAS_IDX_LEN; - optMethodFlags |= OMF_HAS_ARRAYREF; - } + block->bbFlags |= BBF_HAS_IDX_LEN; + optMethodFlags |= OMF_HAS_ARRAYREF; } - /* Create the index node and push it on the stack */ - - op1 = gtNewIndexRef(lclTyp, op1, op2); - - ldstruct = (opcode == CEE_LDELEM && lclTyp == TYP_STRUCT); + op1 = gtNewArrayIndexAddr(op1, op2, lclTyp, ldelemClsHnd); - if ((opcode == CEE_LDELEMA) || ldstruct || - (ldelemClsHnd != DUMMY_INIT(NULL) && eeIsValueClass(ldelemClsHnd))) + if (opcode != CEE_LDELEMA) { - assert(ldelemClsHnd != DUMMY_INIT(NULL)); - - // remember the element size - if (lclTyp == TYP_REF) - { - op1->AsIndex()->gtIndElemSize = TARGET_POINTER_SIZE; - } - else - { - // If ldElemClass is precisely a primitive type, use that, otherwise, preserve the struct type. - if (info.compCompHnd->getTypeForPrimitiveValueClass(ldelemClsHnd) == CORINFO_TYPE_UNDEF) - { - op1->AsIndex()->gtStructElemClass = ldelemClsHnd; - } - assert(lclTyp != TYP_STRUCT || op1->AsIndex()->gtStructElemClass != nullptr); - if (lclTyp == TYP_STRUCT) - { - size = info.compCompHnd->getClassSize(ldelemClsHnd); - op1->AsIndex()->gtIndElemSize = size; - op1->gtType = lclTyp; - } - } - - if ((opcode == CEE_LDELEMA) || ldstruct) - { - // wrap it in a & - lclTyp = TYP_BYREF; - - op1 = gtNewOperNode(GT_ADDR, lclTyp, op1); - } - else - { - assert(lclTyp != TYP_STRUCT); - } + op1 = gtNewIndexIndir(op1->AsIndexAddr()); } - if (ldstruct) - { - // Create an OBJ for the result - op1 = gtNewObjNode(ldelemClsHnd, op1); - op1->gtFlags |= GTF_EXCEPT; - } impPushOnStack(op1, tiRetVal); break; @@ -13768,7 +13700,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) { CorInfoType jitTyp = info.compCompHnd->asCorInfoType(stelemClsHnd); lclTyp = JITtype2varType(jitTyp); - goto ARR_ST_POST_VERIFY; + goto ARR_ST; } case CEE_STELEM_REF: @@ -13785,7 +13717,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) if (impCanSkipCovariantStoreCheck(value, array)) { lclTyp = TYP_REF; - goto ARR_ST_POST_VERIFY; + goto ARR_ST; } } @@ -13819,7 +13751,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) goto ARR_ST; ARR_ST: - ARR_ST_POST_VERIFY: + // TODO-Review: this comment is no longer correct. /* The strict order of evaluation is LHS-operands, RHS-operands, range-check, and then assignment. However, codegen currently does the range-check before evaluation the RHS-operands. So to @@ -13831,45 +13763,29 @@ void Compiler::impImportBlockCode(BasicBlock* block) "Strict ordering of exceptions for Array store")); } - /* Pull the new value from the stack */ + // Pull the new value from the stack. op2 = impPopStack().val; + impBashVarAddrsToI(op2); - /* Pull the index value */ + // Pull the index value. op1 = impPopStack().val; - /* Pull the array address */ + // Pull the array address. op3 = impPopStack().val; - - assertImp(op3->gtType == TYP_REF); - if (op2->IsLocalAddrExpr() != nullptr) - { - op2->gtType = TYP_I_IMPL; - } + assertImp(op3->TypeIs(TYP_REF)); // Mark the block as containing an index expression - - if (op3->gtOper == GT_LCL_VAR) + if (op3->OperIs(GT_LCL_VAR) && op1->OperIs(GT_LCL_VAR, GT_CNS_INT, GT_ADD)) { - if (op1->gtOper == GT_LCL_VAR || op1->gtOper == GT_CNS_INT || op1->gtOper == GT_ADD) - { - block->bbFlags |= BBF_HAS_IDX_LEN; - optMethodFlags |= OMF_HAS_ARRAYREF; - } + block->bbFlags |= BBF_HAS_IDX_LEN; + optMethodFlags |= OMF_HAS_ARRAYREF; } - /* Create the index node */ - - op1 = gtNewIndexRef(lclTyp, op3, op1); + // Create the index node. + op1 = gtNewArrayIndexAddr(op3, op1, lclTyp, stelemClsHnd); + op1 = gtNewIndexIndir(op1->AsIndexAddr()); - /* Create the assignment node and append it */ - - if (lclTyp == TYP_STRUCT) - { - assert(stelemClsHnd != DUMMY_INIT(NULL)); - - op1->AsIndex()->gtStructElemClass = stelemClsHnd; - op1->AsIndex()->gtIndElemSize = info.compCompHnd->getClassSize(stelemClsHnd); - } + // Create the assignment node and append it. if (varTypeIsStruct(op1)) { op1 = impAssignStruct(op1, op2, stelemClsHnd, (unsigned)CHECK_SPILL_ALL); @@ -13879,11 +13795,6 @@ void Compiler::impImportBlockCode(BasicBlock* block) op2 = impImplicitR4orR8Cast(op2, op1->TypeGet()); op1 = gtNewAssignNode(op1, op2); } - - /* Mark the expression as containing an assignment */ - - op1->gtFlags |= GTF_ASG; - goto SPILL_APPEND; case CEE_ADD: @@ -22640,14 +22551,14 @@ bool Compiler::impCanSkipCovariantStoreCheck(GenTree* value, GenTree* array) assert(opts.OptimizationEnabled()); // Check for assignment to same array, ie. arrLcl[i] = arrLcl[j] - if (value->OperIs(GT_INDEX) && array->OperIs(GT_LCL_VAR)) + if (value->OperIs(GT_IND) && value->AsIndir()->Addr()->OperIs(GT_INDEX_ADDR) && array->OperIs(GT_LCL_VAR)) { - GenTree* valueIndex = value->AsIndex()->Arr(); - if (valueIndex->OperIs(GT_LCL_VAR)) + GenTree* valueArray = value->AsIndir()->Addr()->AsIndexAddr()->Arr(); + if (valueArray->OperIs(GT_LCL_VAR)) { - unsigned valueLcl = valueIndex->AsLclVar()->GetLclNum(); - unsigned arrayLcl = array->AsLclVar()->GetLclNum(); - if ((valueLcl == arrayLcl) && !lvaGetDesc(arrayLcl)->IsAddressExposed()) + unsigned valueArrayLcl = valueArray->AsLclVar()->GetLclNum(); + unsigned arrayLcl = array->AsLclVar()->GetLclNum(); + if ((valueArrayLcl == arrayLcl) && !lvaGetDesc(arrayLcl)->IsAddressExposed()) { JITDUMP("\nstelem of ref from same array: skipping covariant store check\n"); return true; diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index ed40beb704cca2..25074f8827472e 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -796,17 +796,12 @@ class LocalAddressVisitor final : public GenTreeVisitor } // The LHS may be a LCL_VAR/LCL_FLD, these are not indirections so we need to handle them here. - // It can also be a GT_INDEX, this is an indirection but it never applies to lclvar addresses - // so it needs to be handled here as well. - switch (indir->OperGet()) { case GT_LCL_VAR: return m_compiler->lvaGetDesc(indir->AsLclVar())->lvExactSize; case GT_LCL_FLD: return genTypeSize(indir->TypeGet()); - case GT_INDEX: - return indir->AsIndex()->gtIndElemSize; default: break; } diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index c540c484cfff08..17ebbed2590662 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -260,7 +260,6 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) // These should have been morphed away to become GT_INDs: case GT_FIELD: - case GT_INDEX: unreached(); break; diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index 1713db4979f552..b78396e5564a97 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -58,8 +58,7 @@ void ArrIndex::PrintBoundsCheckNodes(unsigned dim /* = -1 */) // the "type" member // // Notes: -// This tree produces GT_INDEX node, the caller is supposed to morph it appropriately -// so it can be codegen'ed. +// This tree produces a GT_IND(GT_INDEX_ADDR) node, the caller is supposed to morph it. // GenTree* LC_Array::ToGenTree(Compiler* comp, BasicBlock* bb) { @@ -71,13 +70,15 @@ GenTree* LC_Array::ToGenTree(Compiler* comp, BasicBlock* bb) int rank = GetDimRank(); for (int i = 0; i < rank; ++i) { - arr = comp->gtNewIndexRef(TYP_REF, arr, comp->gtNewLclvNode(arrIndex->indLcls[i], - comp->lvaTable[arrIndex->indLcls[i]].lvType)); + GenTree* idx = comp->gtNewLclvNode(arrIndex->indLcls[i], comp->lvaTable[arrIndex->indLcls[i]].lvType); + GenTree* arrAddr = comp->gtNewArrayIndexAddr(arr, idx, TYP_REF, NO_CLASS_HANDLE); // Clear the range check flag and mark the index as non-faulting: we guarantee that all necessary range // checking has already been done by the time this array index expression is invoked. - arr->gtFlags &= ~(GTF_INX_RNGCHK | GTF_EXCEPT); - arr->gtFlags |= GTF_INX_NOFAULT; + arrAddr->gtFlags &= ~GTF_INX_RNGCHK; + arrAddr->gtFlags |= GTF_INX_ADDR_NONNULL; + + arr = comp->gtNewIndexIndir(arrAddr->AsIndexAddr()); } // If asked for arrlen invoke arr length operator. if (oper == ArrLen) @@ -2186,7 +2187,7 @@ bool Compiler::optIsStackLocalInvariant(unsigned loopNum, unsigned lclNum) // // Arguments: // tree the tree to be checked if it is the array [] operation. -// result the extracted GT_INDEX information is updated in result. +// result the extracted GT_INDEX_ADDR information is updated in result. // lhsNum for the root level (function is recursive) callers should pass BAD_VAR_NUM. // topLevelIsFinal OUT: set to `true` if see a non-TYP_REF element type array. // @@ -2196,8 +2197,8 @@ bool Compiler::optIsStackLocalInvariant(unsigned loopNum, unsigned lclNum) // dimension of [] encountered. // // Operation: -// Given a "tree" extract the GT_INDEX node in "result" as ArrIndex. In morph -// we have converted a GT_INDEX tree into a scaled index base offset expression. +// Given a "tree" extract the GT_INDEX_ADDR node in "result" as ArrIndex. In morph +// we have converted a GT_INDEX_ADDR tree into a scaled index base offset expression. // However, we don't actually bother to parse the morphed tree. All we care about is // the bounds check node: it contains the array base and element index. The other side // of the COMMA node can vary between array of primitive type and array of struct. There's @@ -2306,7 +2307,7 @@ bool Compiler::optExtractArrIndex(GenTree* tree, ArrIndex* result, unsigned lhsN // // Arguments: // tree the tree to be checked if it is an array [][][] operation. -// result OUT: the extracted GT_INDEX information. +// result OUT: the extracted GT_INDEX_ADDR information. // lhsNum var number of array object we're looking for. // topLevelIsFinal OUT: set to `true` if we reached a non-TYP_REF element type array. // @@ -2333,7 +2334,7 @@ bool Compiler::optReconstructArrIndexHelp(GenTree* tree, ArrIndex* result, unsig GenTree* lhs = before->gtGetOp1(); GenTree* rhs = before->gtGetOp2(); - // "rhs" should contain an GT_INDEX + // "rhs" should contain an index expression. if (!lhs->IsLocal() || !optReconstructArrIndexHelp(rhs, result, lhsNum, topLevelIsFinal)) { return false; diff --git a/src/coreclr/jit/loopcloning.h b/src/coreclr/jit/loopcloning.h index 13bd2fa5e090c5..8120e82a14c7d0 100644 --- a/src/coreclr/jit/loopcloning.h +++ b/src/coreclr/jit/loopcloning.h @@ -70,7 +70,7 @@ exception occurs. block, stmt, tree information) to do the optimization later. a) This involves checking if the loop is well-formed with respect to the optimization being performed. - b) In array bounds check case, reconstructing the morphed GT_INDEX + b) In array bounds check case, reconstructing the morphed GT_INDEX_ADDR nodes back to their array representation. i) The array index is stored in the "context" variable with additional block, tree, stmt info. @@ -195,7 +195,7 @@ class Compiler; * * Represents an array access and associated bounds checks. * Array access is required to have the array and indices in local variables. - * This struct is constructed using a GT_INDEX node that is broken into + * This struct is constructed using a GT_INDEX_ADDR node that is broken into * its sub trees. * */ diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ed84ba1401412c..e76fa0d054d689 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -4527,102 +4527,43 @@ BasicBlock* Compiler::fgSetRngChkTargetInner(SpecialCodeKind kind, bool delay) return nullptr; } -/***************************************************************************** - * - * Expand a GT_INDEX node and fully morph the child operands - * - * The orginal GT_INDEX node is bashed into the GT_IND node that accesses - * the array element. We expand the GT_INDEX node into a larger tree that - * evaluates the array base and index. The simplest expansion is a GT_COMMA - * with a GT_BOUNDS_CHECK and a GT_IND with a GTF_INX_RNGCHK flag. - * For complex array or index expressions one or more GT_COMMA assignments - * are inserted so that we only evaluate the array or index expressions once. - * - * The fully expanded tree is then morphed. This causes gtFoldExpr to - * perform local constant prop and reorder the constants in the tree and - * fold them. - * - * We then parse the resulting array element expression in order to locate - * and label the constants and variables that occur in the tree. - */ - -const int MAX_ARR_COMPLEXITY = 4; -const int MAX_INDEX_COMPLEXITY = 4; - -GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) +//------------------------------------------------------------------------ +// fgMorphIndexAddr: Expand a GT_INDEX_ADDR node and fully morph the child operands. +// +// We expand the GT_INDEX_ADDR node into a larger tree that evaluates the array +// base and index. The simplest expansion is a GT_COMMA with a GT_BOUNDS_CHECK. +// For complex array or index expressions one or more GT_COMMA assignments +// are inserted so that we only evaluate the array or index expressions once. +// +// The fully expanded tree is then morphed. This causes gtFoldExpr to +// perform local constant prop and reorder the constants in the tree and +// fold them. +// +// Arguments: +// indexAddr - The INDEX_ADRR tree to morph +// +// Return Value: +// The resulting tree. +// +GenTree* Compiler::fgMorphIndexAddr(GenTreeIndexAddr* indexAddr) { - noway_assert(tree->gtOper == GT_INDEX); - GenTreeIndex* asIndex = tree->AsIndex(); - var_types elemTyp = asIndex->TypeGet(); - unsigned elemSize = asIndex->gtIndElemSize; - CORINFO_CLASS_HANDLE elemStructType = asIndex->gtStructElemClass; - - 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 - if (opts.OptimizationEnabled() && asIndex->Arr()->OperIs(GT_CNS_STR) && - !asIndex->Arr()->AsStrCon()->IsStringEmptyField() && asIndex->Index()->IsIntCnsFitsInI32()) - { - const int cnsIndex = static_cast(asIndex->Index()->AsIntConCommon()->IconValue()); - if (cnsIndex >= 0) - { - const int maxStrSize = 1024; - char16_t str[maxStrSize]; - int length = info.compCompHnd->getStringLiteral(asIndex->Arr()->AsStrCon()->gtScpHnd, - asIndex->Arr()->AsStrCon()->gtSconCPX, str, maxStrSize); - if ((cnsIndex < length)) - { - GenTree* cnsCharNode = gtNewIconNode(str[cnsIndex], TYP_INT); - INDEBUG(cnsCharNode->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - return cnsCharNode; - } - } - } + const int MAX_ARR_COMPLEXITY = 4; + const int MAX_INDEX_COMPLEXITY = 4; -#ifdef FEATURE_SIMD - if (varTypeIsStruct(elemTyp) && structSizeMightRepresentSIMDType(elemSize)) - { - // If this is a SIMD type, this is the point at which we lose the type information, - // so we need to set the correct type on the GT_IND. - // (We don't care about the base type here, so we only check, but don't retain, the return value). - unsigned simdElemSize = 0; - if (getBaseJitTypeAndSizeOfSIMDType(elemStructType, &simdElemSize) != CORINFO_TYPE_UNDEF) - { - assert(simdElemSize == elemSize); - elemTyp = getSIMDTypeForSize(elemSize); - // 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 = NO_CLASS_HANDLE; - } - } -#endif // FEATURE_SIMD + var_types elemTyp = indexAddr->gtElemType; + unsigned elemSize = indexAddr->gtElemSize; + uint8_t elemOffs = static_cast(indexAddr->gtElemOffset); + CORINFO_CLASS_HANDLE elemStructType = indexAddr->gtStructElemClass; - // Set up the array length's offset into lenOffs - // And the first element's offset into elemOffs - ssize_t lenOffs; - uint8_t elemOffs; - if (tree->gtFlags & GTF_INX_STRING_LAYOUT) - { - lenOffs = OFFSETOF__CORINFO_String__stringLen; - elemOffs = OFFSETOF__CORINFO_String__chars; - tree->gtFlags &= ~GTF_INX_STRING_LAYOUT; // Clear this flag as it is used for GTF_IND_VOLATILE - } - else - { - // We have a standard array - lenOffs = OFFSETOF__CORINFO_Array__length; - elemOffs = OFFSETOF__CORINFO_Array__data; - } + noway_assert(!varTypeIsStruct(elemTyp) || (elemStructType != NO_CLASS_HANDLE)); - // In minopts, we expand GT_INDEX to GT_IND(GT_INDEX_ADDR) in order to minimize the size of the IR. As minopts + // In minopts, we will not be expanding GT_INDEX_ADDR in order to minimize the size of the IR. As minopts // compilation time is roughly proportional to the size of the IR, this helps keep compilation times down. // Furthermore, this representation typically saves on code size in minopts w.r.t. the complete expansion // performed when optimizing, as it does not require LclVar nodes (which are always stack loads/stores in // minopts). // - // When we *are* optimizing, we fully expand GT_INDEX to: + // When we *are* optimizing, we fully expand GT_INDEX_ADDR to: // 1. Evaluate the array address expression and store the result in a temp if the expression is complex or // side-effecting. // 2. Evaluate the array index expression and store the result in a temp if the expression is complex or @@ -4632,63 +4573,46 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) // 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. if (opts.MinOpts()) { - 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), elemOffs); - indexAddr->gtFlags |= (array->gtFlags | index->gtFlags) & GTF_ALL_EFFECT; + indexAddr->Arr() = fgMorphTree(indexAddr->Arr()); + indexAddr->Index() = fgMorphTree(indexAddr->Index()); + indexAddr->AddAllEffectsFlags(indexAddr->Arr(), indexAddr->Index()); // Mark the indirection node as needing a range check if necessary. // Note this will always be true unless JitSkipArrayBoundCheck() is used - if ((indexAddr->gtFlags & GTF_INX_RNGCHK) != 0) + if (indexAddr->IsBoundsChecked()) { fgSetRngChkTarget(indexAddr); } - if (!tree->TypeIs(TYP_STRUCT)) - { - tree->ChangeOper(GT_IND); - } - else - { - DEBUG_DESTROY_NODE(tree); - tree = gtNewObjNode(elemStructType, indexAddr); - INDEBUG(tree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - } - GenTreeIndir* const indir = tree->AsIndir(); - indir->Addr() = indexAddr; - bool canCSE = indir->CanCSE(); - indir->gtFlags = indexAddr->gtFlags & GTF_ALL_EFFECT; - if (!canCSE) - { - indir->SetDoNotCSE(); - } - - INDEBUG(indexAddr->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - - return indir; + return indexAddr; } - GenTree* arrRef = asIndex->Arr(); - GenTree* index = asIndex->Index(); +#ifdef FEATURE_SIMD + if (varTypeIsStruct(elemTyp) && structSizeMightRepresentSIMDType(elemSize)) + { + elemTyp = impNormStructType(elemStructType); + } +#endif // FEATURE_SIMD - bool chkd = ((tree->gtFlags & GTF_INX_RNGCHK) != 0); // if false, range checking will be disabled - bool indexNonFaulting = ((tree->gtFlags & GTF_INX_NOFAULT) != 0); // if true, mark GTF_IND_NONFAULTING - bool nCSE = ((tree->gtFlags & GTF_DONT_CSE) != 0); + // TODO-CQ: support precise equivalence classes for SIMD-typed arrays in VN. + if (elemTyp != TYP_STRUCT) + { + elemStructType = NO_CLASS_HANDLE; + } - GenTree* arrRefDefn = nullptr; // non-NULL if we need to allocate a temp for the arrRef expression - GenTree* indexDefn = nullptr; // non-NULL if we need to allocate a temp for the index expression - GenTree* bndsChk = nullptr; + GenTree* arrRef = indexAddr->Arr(); + GenTree* index = indexAddr->Index(); + GenTree* arrRefDefn = nullptr; // non-NULL if we need to allocate a temp for the arrRef expression + GenTree* indexDefn = nullptr; // non-NULL if we need to allocate a temp for the index expression + GenTreeBoundsChk* boundsCheck = nullptr; // If we're doing range checking, introduce a GT_BOUNDS_CHECK node for the address. - if (chkd) + if (indexAddr->IsBoundsChecked()) { GenTree* arrRef2 = nullptr; // The second copy will be used in array address expression GenTree* index2 = nullptr; @@ -4704,7 +4628,7 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) // were mostly ameliorated by adding this condition. // // Likewise, allocate a temporary if the expression is a GT_LCL_FLD node. These used to be created - // after fgMorphArrayIndex from GT_FIELD trees so this preserves the existing behavior. This is + // after fgMorphIndexAddr from GT_FIELD trees so this preserves the existing behavior. This is // perhaps a decision that should be left to CSE but FX diffs show that it is slightly better to // do this here. @@ -4749,27 +4673,23 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) } #endif // TARGET_64BIT - GenTree* arrLen = gtNewArrLen(TYP_INT, arrRef, (int)lenOffs, compCurBB); + GenTree* arrLen = gtNewArrLen(TYP_INT, arrRef, (int)indexAddr->gtLenOffset, compCurBB); if (bndsChkType != TYP_INT) { arrLen = gtNewCastNode(bndsChkType, arrLen, true, bndsChkType); } - GenTreeBoundsChk* arrBndsChk = new (this, GT_BOUNDS_CHECK) GenTreeBoundsChk(index, arrLen, SCK_RNGCHK_FAIL); - arrBndsChk->gtInxType = elemTyp; - - bndsChk = arrBndsChk; + boundsCheck = new (this, GT_BOUNDS_CHECK) GenTreeBoundsChk(index, arrLen, SCK_RNGCHK_FAIL); + boundsCheck->gtInxType = elemTyp; // Now we'll switch to using the second copies for arrRef and index // to compute the address expression - arrRef = arrRef2; index = index2; } // Create the "addr" which is "*(arrRef + ((index * elemSize) + elemOffs))" - GenTree* addr; #ifdef TARGET_64BIT @@ -4853,74 +4773,50 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) addr = gtNewOperNode(GT_ADD, TYP_BYREF, arrRef, addr); } + // TODO-Throughout: bash the INDEX_ADDR to ARR_ADDR here instead of creating a new node. 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); - - // If the index node is a floating-point type, notify the compiler - // we'll potentially use floating point registers at the time of codegen. - if (varTypeUsesFloatReg(tree->gtType)) + if (indexAddr->IsNotNull()) { - this->compFloatingPointUsed = true; - } - - // We've now consumed the GTF_INX_RNGCHK and GTF_INX_NOFAULT, and the node - // is no longer a GT_INDEX node. - tree->gtFlags &= ~(GTF_INX_RNGCHK | GTF_INX_NOFAULT); - - tree->AsOp()->gtOp1 = addr; - - // 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 - { - tree->gtFlags |= GTF_EXCEPT; - } - if (nCSE) + // Transfer the zero-offset annotation from INDEX_ADDR to ARR_ADDR... + FieldSeqNode* zeroOffsetFldSeq; + if (GetZeroOffsetFieldMap()->Lookup(indexAddr, &zeroOffsetFldSeq)) { - tree->gtFlags |= GTF_DONT_CSE; + fgAddFieldSeqForZeroOffset(addr, zeroOffsetFldSeq); } - // Did we create a bndsChk tree? - if (bndsChk) - { - // Use a GT_COMMA node to prepend the array bound check - // - tree = gtNewOperNode(GT_COMMA, elemTyp, bndsChk, tree); + GenTree* tree = addr; - /* Mark the indirection node as needing a range check */ - fgSetRngChkTarget(bndsChk); + // Prepend the bounds check and the assignment trees that were created (if any). + if (boundsCheck != nullptr) + { + tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), boundsCheck, tree); + fgSetRngChkTarget(boundsCheck); } if (indexDefn != nullptr) { - // Use a GT_COMMA node to prepend the index assignment - // tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), indexDefn, tree); } + if (arrRefDefn != nullptr) { - // Use a GT_COMMA node to prepend the arRef assignment - // tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), arrRefDefn, tree); } - JITDUMP("fgMorphArrayIndex (before remorph):\n") + JITDUMP("fgMorphIndexAddr (before remorph):\n") DISPTREE(tree) - GenTree* morphedTree = fgMorphTree(tree); - DBEXEC(morphedTree != tree, morphedTree->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); + tree = fgMorphTree(tree); + DBEXEC(tree == indexAddr, tree->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); JITDUMP("fgMorphArrayIndex (after remorph):\n") - DISPTREE(morphedTree) + DISPTREE(tree) - return morphedTree; + return tree; } //------------------------------------------------------------------------ @@ -8600,8 +8496,9 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call) fgWalkTreePost(&value, resetMorphedFlag); #endif // DEBUG - GenTree* const arrIndexNode = gtNewIndexRef(TYP_REF, arr, index); - GenTree* const arrStore = gtNewAssignNode(arrIndexNode, value); + GenTree* const arrIndexAddr = gtNewArrayIndexAddr(arr, index, TYP_REF, NO_CLASS_HANDLE); + GenTree* const arrIndex = gtNewIndexIndir(arrIndexAddr->AsIndexAddr()); + GenTree* const arrStore = gtNewAssignNode(arrIndex, value); GenTree* result = fgMorphTree(arrStore); if (argSetup != nullptr) @@ -9626,7 +9523,6 @@ GenTree* Compiler::fgMorphGetStructAddr(GenTree** pTree, CORINFO_CLASS_HANDLE cl { case GT_LCL_FLD: case GT_LCL_VAR: - case GT_INDEX: case GT_FIELD: case GT_ARR_ELEM: addr = gtNewOperNode(GT_ADDR, TYP_BYREF, tree); @@ -10392,8 +10288,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_FIELD: return fgMorphField(tree, mac); - case GT_INDEX: - return fgMorphArrayIndex(tree); + case GT_INDEX_ADDR: + return fgMorphIndexAddr(tree->AsIndexAddr()); case GT_CAST: { @@ -10478,6 +10374,19 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) } break; + case GT_IND: + if (opts.OptimizationEnabled() && !optValnumCSE_phase) + { + GenTree* constNode = gtFoldIndirConst(tree->AsIndir()); + if (constNode != nullptr) + { + assert(constNode->OperIsConst()); // No further morphing required. + INDEBUG(constNode->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + return constNode; + } + } + break; + case GT_DIV: // Replace "val / dcon" with "val * (1.0 / dcon)" if dcon is a power of two. // Powers of two within range are always exactly represented, diff --git a/src/coreclr/jit/simd.cpp b/src/coreclr/jit/simd.cpp index 7beee673d45b4a..92706856a0dce8 100644 --- a/src/coreclr/jit/simd.cpp +++ b/src/coreclr/jit/simd.cpp @@ -1639,30 +1639,31 @@ bool Compiler::areLocalFieldsContiguous(GenTreeLclFld* first, GenTreeLclFld* sec // TODO-CQ: // Right this can only check array element with const number as index. In future, // we should consider to allow this function to check the index using expression. - +// bool Compiler::areArrayElementsContiguous(GenTree* op1, GenTree* op2) { - noway_assert(op1->gtOper == GT_INDEX); - noway_assert(op2->gtOper == GT_INDEX); - GenTreeIndex* op1Index = op1->AsIndex(); - GenTreeIndex* op2Index = op2->AsIndex(); + assert(op1->OperIs(GT_IND) && op2->OperIs(GT_IND)); + assert(!op1->TypeIs(TYP_STRUCT) && (op1->TypeGet() == op2->TypeGet())); + + GenTreeIndexAddr* op1IndexAddr = op1->AsIndir()->Addr()->AsIndexAddr(); + GenTreeIndexAddr* op2IndexAddr = op2->AsIndir()->Addr()->AsIndexAddr(); - GenTree* op1ArrayRef = op1Index->Arr(); - GenTree* op2ArrayRef = op2Index->Arr(); + GenTree* op1ArrayRef = op1IndexAddr->Arr(); + GenTree* op2ArrayRef = op2IndexAddr->Arr(); assert(op1ArrayRef->TypeGet() == TYP_REF); assert(op2ArrayRef->TypeGet() == TYP_REF); - GenTree* op1IndexNode = op1Index->Index(); - GenTree* op2IndexNode = op2Index->Index(); + GenTree* op1IndexNode = op1IndexAddr->Index(); + GenTree* op2IndexNode = op2IndexAddr->Index(); if ((op1IndexNode->OperGet() == GT_CNS_INT && op2IndexNode->OperGet() == GT_CNS_INT) && op1IndexNode->AsIntCon()->gtIconVal + 1 == op2IndexNode->AsIntCon()->gtIconVal) { - if (op1ArrayRef->OperGet() == GT_FIELD && op2ArrayRef->OperGet() == GT_FIELD && + if (op1ArrayRef->OperIs(GT_FIELD) && op2ArrayRef->OperIs(GT_FIELD) && areFieldsParentsLocatedSame(op1ArrayRef, op2ArrayRef)) { return true; } - else if (op1ArrayRef->OperIsLocal() && op2ArrayRef->OperIsLocal() && + else if (op1ArrayRef->OperIs(GT_LCL_VAR) && op2ArrayRef->OperIs(GT_LCL_VAR) && op1ArrayRef->AsLclVarCommon()->GetLclNum() == op2ArrayRef->AsLclVarCommon()->GetLclNum()) { return true; @@ -1682,14 +1683,13 @@ bool Compiler::areArrayElementsContiguous(GenTree* op1, GenTree* op2) // TODO-CQ: // Right now this can only check field and array. In future we should add more cases. // - bool Compiler::areArgumentsContiguous(GenTree* op1, GenTree* op2) { - if (op1->OperGet() == GT_INDEX && op2->OperGet() == GT_INDEX) + if (op1->OperIs(GT_IND) && op2->OperIs(GT_IND)) { return areArrayElementsContiguous(op1, op2); } - else if (op1->OperGet() == GT_FIELD && op2->OperGet() == GT_FIELD) + else if (op1->OperIs(GT_FIELD) && op2->OperIs(GT_FIELD)) { return areFieldsContiguous(op1, op2); } @@ -1713,17 +1713,16 @@ bool Compiler::areArgumentsContiguous(GenTree* op1, GenTree* op2) // return the address node. // // TODO-CQ: -// 1. Currently just support for GT_FIELD and GT_INDEX, because we can only verify the GT_INDEX node or GT_Field -// are located contiguously or not. In future we should support more cases. +// Currently just supports GT_FIELD and GT_IND(GT_INDEX_ADDR), because we can only verify those nodes +// are located contiguously or not. In future we should support more cases. // GenTree* Compiler::createAddressNodeForSIMDInit(GenTree* tree, unsigned simdSize) { - assert(tree->OperGet() == GT_FIELD || tree->OperGet() == GT_INDEX); GenTree* byrefNode = nullptr; unsigned offset = 0; var_types baseType = tree->gtType; - if (tree->OperGet() == GT_FIELD) + if (tree->OperIs(GT_FIELD)) { GenTree* objRef = tree->AsField()->GetFldObj(); if (objRef != nullptr && objRef->gtOper == GT_ADDR) @@ -1751,23 +1750,25 @@ GenTree* Compiler::createAddressNodeForSIMDInit(GenTree* tree, unsigned simdSize assert(byrefNode != nullptr); offset = tree->AsField()->gtFldOffset; } - else if (tree->OperGet() == GT_INDEX) + else { + assert(tree->OperIs(GT_IND) && tree->AsIndir()->Addr()->OperIs(GT_INDEX_ADDR)); - GenTree* index = tree->AsIndex()->Index(); - assert(index->OperGet() == GT_CNS_INT); + GenTreeIndexAddr* indexAddr = tree->AsIndir()->Addr()->AsIndexAddr(); + GenTree* arrayRef = indexAddr->Arr(); + GenTree* index = indexAddr->Index(); + assert(index->IsCnsIntOrI()); GenTree* checkIndexExpr = nullptr; - unsigned indexVal = (unsigned)(index->AsIntCon()->gtIconVal); + unsigned indexVal = (unsigned)index->AsIntCon()->gtIconVal; offset = indexVal * genTypeSize(tree->TypeGet()); - GenTree* arrayRef = tree->AsIndex()->Arr(); // Generate the boundary check exception. // The length for boundary check should be the maximum index number which should be // (first argument's index number) + (how many array arguments we have) - 1 // = indexVal + arrayElementsCount - 1 unsigned arrayElementsCount = simdSize / genTypeSize(baseType); - checkIndexExpr = new (this, GT_CNS_INT) GenTreeIntCon(TYP_INT, indexVal + arrayElementsCount - 1); + checkIndexExpr = gtNewIconNode(indexVal + arrayElementsCount - 1); GenTreeArrLen* arrLen = gtNewArrLen(TYP_INT, arrayRef, (int)OFFSETOF__CORINFO_Array__length, compCurBB); GenTreeBoundsChk* arrBndsChk = new (this, GT_BOUNDS_CHECK) GenTreeBoundsChk(checkIndexExpr, arrLen, SCK_ARG_RNG_EXCPN); @@ -1775,10 +1776,6 @@ GenTree* Compiler::createAddressNodeForSIMDInit(GenTree* tree, unsigned simdSize offset += OFFSETOF__CORINFO_Array__data; byrefNode = gtNewOperNode(GT_COMMA, arrayRef->TypeGet(), arrBndsChk, gtCloneExpr(arrayRef)); } - else - { - unreached(); - } GenTree* address = gtNewOperNode(GT_ADD, TYP_BYREF, byrefNode, gtNewIconNode(offset, TYP_I_IMPL)); @@ -1792,7 +1789,7 @@ GenTree* Compiler::createAddressNodeForSIMDInit(GenTree* tree, unsigned simdSize // // Arguments: // stmt - GenTree*. Input statement node. - +// void Compiler::impMarkContiguousSIMDFieldAssignments(Statement* stmt) { if (opts.OptimizationDisabled()) From 6ec53a03f707a328dfb9b2cc220cf44f11b9e9bc Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sat, 23 Apr 2022 23:55:43 +0300 Subject: [PATCH 3/4] Add a zero-diff quirk --- src/coreclr/jit/gentree.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 143dadd23549fd..f0fef29667ef6c 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -15722,6 +15722,15 @@ void Compiler::gtExtractSideEffList(GenTree* expr, PushSideEffects(node); return Compiler::WALK_SKIP_SUBTREES; } + + // TODO-ADDR: remove this quirk added to avoid diffs. + if (node->OperIs(GT_IND) && node->AsIndir()->Addr()->OperIs(GT_INDEX_ADDR) && + !m_compiler->fgGlobalMorph) + { + JITDUMP("Keep the GT_INDEX_ADDR and GT_IND together:\n"); + PushSideEffects(node); + return Compiler::WALK_SKIP_SUBTREES; + } } // Generally all GT_CALL nodes are considered to have side-effects. From c7bc5402d40d6b5753f9b997db8a7ec83f4fbca6 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 30 May 2022 14:19:44 +0300 Subject: [PATCH 4/4] Update the first class structs document Remove references to things that no longer exist. --- .../design/coreclr/jit/first-class-structs.md | 35 ++++--------------- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/docs/design/coreclr/jit/first-class-structs.md b/docs/design/coreclr/jit/first-class-structs.md index 4571d311accba8..5981413bd8427a 100644 --- a/docs/design/coreclr/jit/first-class-structs.md +++ b/docs/design/coreclr/jit/first-class-structs.md @@ -70,11 +70,6 @@ Current Representation of Struct Values These struct-typed nodes are created by the importer, but transformed in morph, and so are not encountered by most phases of the JIT: -* `GT_INDEX`: This is transformed to a `GT_IND` - * Currently, the IND is marked with `GTF_IND_ARR_INDEX` and the node pointer of the `GT_IND` acts as a key - into the array info map. - * Proposed: This should be transformed into a `GT_OBJ` when it represents a struct type, and then the - class handle would no longer need to be obtained from the array info map. * `GT_FIELD`: This is transformed to a `GT_LCL_VAR` by the `Compiler::fgMarkAddressExposedLocals()` phase if it's a promoted struct field, or to a `GT_LCL_FLD` or GT_IND` by `fgMorphField()`. * Proposed: A non-promoted struct typed field should be transformed into a `GT_OBJ`, so that consistently all struct @@ -90,9 +85,7 @@ encountered by most phases of the JIT: ### Struct “objects” as lvalues * The lhs of a struct assignment is a block or local node: - * `GT_OBJ` nodes represent the “shape” info via a struct handle, along with the GC info - (location and type of GC references within the struct). - * These are currently used only to represent struct values that contain GC references (although see below). + * `GT_OBJ` nodes represent struct types with a handle, and store a pointer to the `ClassLayout` object. * `GT_BLK` nodes represent struct types with no GC references, or opaque blocks of fixed size. * These have no struct handle, resulting in some pessimization or even incorrect code when the appropriate struct handle can't be determined. @@ -101,12 +94,12 @@ encountered by most phases of the JIT: [#21705](https://github.com/dotnet/coreclr/pull/21705) they are no longer large nodes. * `GT_STORE_OBJ` and `GT_STORE_BLK` have the same structure as `GT_OBJ` and `GT_BLK`, respectively * `Data()` is op2 - * `GT_DYN_BLK` and `GT_STORE_DYN_BLK` (GenTreeDynBlk extends GenTreeBlk) + * `GT_STORE_DYN_BLK` (GenTreeStoreDynBlk extends GenTreeBlk) * Additional child `gtDynamicSize` - * Note that these aren't really struct types; they represent dynamically sized blocks + * Note that these aren't really struct stores; they represent dynamically sized blocks of arbitrary data. - * For `GT_LCL_FLD` nodes, we don't retain shape information, except indirectly via the `FieldSeqNode`. - * For `GT_LCL_VAR` nodes, the`ClassLayout` is obtained from the `LclVarDsc`. + * For `GT_LCL_FLD` nodes, we store a pointer to `ClassLayout` in the node. + * For `GT_LCL_VAR` nodes, the `ClassLayout` is obtained from the `LclVarDsc`. ### Struct “objects” as rvalues @@ -131,10 +124,6 @@ After morph, a struct-typed value on the RHS of assignment is one of: * `GT_CALL` * `GT_LCL_VAR` * `GT_LCL_FLD` - * Note: With `compDoOldStructRetyping()`, a GT_LCL_FLD` with a primitive type of the same size as the struct - is used to represent a reference to the full struct when it is passed in a register. - This forces the struct to live on the stack, and makes it more difficult to optimize these struct values, - which is why this mechanism is being phased out. * `GT_SIMD` * `GT_OBJ` nodes can also be used as rvalues when they are call arguments * Proposed: `GT_OBJ` nodes can be used in any context where a struct rvalue or lvalue might occur, @@ -173,8 +162,7 @@ There are three main phases in the JIT that make changes to the representation o registers. The necessary transformations for correct code generation would be made in `Lowering`. - * With `compDoOldStructRetyping()`, if it is passed in a single register, it is morphed into a - `GT_LCL_FLD` node of the appropriate primitive type. + * If it is passed in a single register, it is morphed into a `GT_LCL_FLD` node of the appropriate primitive type. * This may involve making a copy, if the size cannot be safely loaded. * Proposed: This would remain a `GT_OBJ` and would be appropriately transformed in `Lowering`, e.g. using `GT_BITCAST`. @@ -310,22 +298,13 @@ This would be enabled first by [Defer ABI-specific transformations to Lowering]( for block copies. See [\#21711 Improve init/copy block codegen](https://github.com/dotnet/coreclr/pull/21711). * This also includes cleanup of the block morphing methods such that block nodes needn't be visited multiple - times, such as `fgMorphBlkToInd` (may be simply unneeded), `fgMorphBlkNode` and `fgMorphBlkOperand`. + times, such as `fgMorphBlkNode` and `fgMorphBlkOperand`. These methods were introduced to preserve old behavior, but should be simplified. -* Somewhat related is the handling of struct-typed array elements. Currently, after the `GT_INDEX` is transformed - into a `GT_IND`, that node must be retained as the key into the `ArrayInfoMap`. For structs, this is then - wrapped in `OBJ(ADDR(...))`. We should be able to change the IND to OBJ and avoid wrapping, and should also be - able to remove the class handle from the array info map and instead used the one provided by the `GT_OBJ`. - ### Miscellaneous Cleanup These are all marked with `TODO-1stClassStructs` or `TODO-Cleanup` in the last case: -* The handling of `DYN_BLK` is unnecessarily complicated to duplicate previous behavior (i.e. to enable previous - refactorings to be zero-diff). These nodes are infrequent so the special handling should just be eliminated - (e.g. see `GenTree::GetChild()`). - * The checking at the end of `gtNewTempAssign()` should be simplified. * When we create a struct assignment, we use `impAssignStruct()`. This code will, in some cases, create