From bdaf20693ddf7ef6f7f6c3db1a4c4284adb35f13 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 7 Jun 2022 23:37:46 +0300 Subject: [PATCH 1/2] Delete a zero-diff quirk --- src/coreclr/jit/gentree.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index a42a72934033c6..f6a167a50e1390 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -15714,15 +15714,6 @@ 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 eb8528d0b12e30292c47e7c397dd0bd36809c18c Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 7 Jun 2022 23:58:06 +0300 Subject: [PATCH 2/2] Also update some comments Removing GT_INDEX/ADDR(IND) references. --- src/coreclr/jit/gentree.h | 8 +- src/coreclr/jit/loopcloning.cpp | 126 ++++++++++++++++---------------- src/coreclr/jit/morph.cpp | 4 +- 3 files changed, 71 insertions(+), 67 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 9aff6df4bed4c9..b9ba82532a5541 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -6414,10 +6414,10 @@ struct GenTreeBoundsChk : public GenTreeOp BasicBlock* gtIndRngFailBB; // Basic block to jump to for index-out-of-range 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 ARR_ADDR node of the morphed tree, but that can - // be hard to find. - var_types gtInxType; // Save the GT_INDEX type + // Store some information about the array element type that was in the GT_INDEX_ADDR node before morphing. + // 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; // The array element type. GenTreeBoundsChk(GenTree* index, GenTree* length, SpecialCodeKind kind) : GenTreeOp(GT_BOUNDS_CHECK, TYP_VOID, index, length) diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index b78396e5564a97..eb5d7af112aaaa 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -2294,7 +2294,7 @@ bool Compiler::optExtractArrIndex(GenTree* tree, ArrIndex* result, unsigned lhsN result->useBlock = compCurBB; result->rank++; - // If the array element type (saved from the GT_INDEX node during morphing) is anything but + // If the array element type (saved from the GT_INDEX_ADDR node during morphing) is anything but // TYP_REF, then it must the final level of jagged array. assert(arrBndsChk->gtInxType != TYP_VOID); *topLevelIsFinal = (arrBndsChk->gtInxType != TYP_REF); @@ -2360,7 +2360,7 @@ bool Compiler::optReconstructArrIndexHelp(GenTree* tree, ArrIndex* result, unsig // // 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. // // Return Value: // Returns true if array index can be extracted, else, return false. "rank" field in @@ -2375,99 +2375,103 @@ bool Compiler::optReconstructArrIndexHelp(GenTree* tree, ArrIndex* result, unsig // Note that the array expression is implied by the array bounds check under the COMMA, and the array bounds // checks is what is parsed from the morphed tree; the array addressing expression is not parsed. // However, the array bounds checks are not quite sufficient because of the way "morph" alters the trees. -// Specifically, we normally see a COMMA node with a LHS of the morphed array INDEX expression and RHS +// Specifically, we normally see a COMMA node with a LHS of the morphed array INDEX_ADDR expression and RHS // of the bounds check. E.g., for int[][], a[i][j] we have a pre-morph tree: // -// \--* INDEX int -// +--* INDEX ref -// | +--* LCL_VAR ref V00 loc0 -// | \--* LCL_VAR int V02 loc2 -// \--* LCL_VAR int V03 loc3 +// \--* IND int +// \--* INDEX_ADDR byref int[] +// +--* IND ref +// | \--* INDEX_ADDR byref ref[] +// | +--* LCL_VAR ref V00 arg0 +// | \--* LCL_VAR int V01 arg1 +// \--* LCL_VAR int V02 arg2 // // and post-morph tree: // // \--* COMMA int // +--* ASG ref -// | +--* LCL_VAR ref V19 tmp12 +// | +--* LCL_VAR ref V04 tmp1 // | \--* COMMA ref // | +--* BOUNDS_CHECK_Rng void -// | | +--* LCL_VAR int V02 loc2 +// | | +--* LCL_VAR int V01 arg1 // | | \--* ARR_LENGTH int -// | | \--* LCL_VAR ref V00 loc0 +// | | \--* LCL_VAR ref V00 arg0 // | \--* IND ref -// | \--* ADD byref -// | +--* LCL_VAR ref V00 loc0 -// | \--* ADD long -// | +--* LSH long -// | | +--* CAST long <- uint -// | | | \--* LCL_VAR int V02 loc2 -// | | \--* CNS_INT long 3 -// | \--* CNS_INT long 16 Fseq[#FirstElem] +// | \--* ARR_ADDR byref ref[] +// | \--* ADD byref +// | +--* LCL_VAR ref V00 arg0 +// | \--* ADD long +// | +--* LSH long +// | | +--* CAST long <- uint +// | | | \--* LCL_VAR int V01 arg1 +// | | \--* CNS_INT long 3 +// | \--* CNS_INT long 16 // \--* COMMA int // +--* BOUNDS_CHECK_Rng void -// | +--* LCL_VAR int V03 loc3 +// | +--* LCL_VAR int V02 arg2 // | \--* ARR_LENGTH int -// | \--* LCL_VAR ref V19 tmp12 +// | \--* LCL_VAR ref V04 tmp1 // \--* IND int -// \--* ADD byref -// +--* LCL_VAR ref V19 tmp12 -// \--* ADD long -// +--* LSH long -// | +--* CAST long <- uint -// | | \--* LCL_VAR int V03 loc3 -// | \--* CNS_INT long 2 -// \--* CNS_INT long 16 Fseq[#FirstElem] +// \--* ARR_ADDR byref int[] +// \--* ADD byref +// +--* LCL_VAR ref V04 tmp1 +// \--* ADD long +// +--* LSH long +// | +--* CAST long <- uint +// | | \--* LCL_VAR int V02 arg2 +// | \--* CNS_INT long 2 +// \--* CNS_INT long 16 // // However, for an array of structs that contains an array field, e.g. ValueTuple[], expression // a[i].Item1[j], // -// \--* INDEX int -// +--* FIELD ref Item1 -// | \--* ADDR byref -// | \--* INDEX struct -// | +--* LCL_VAR ref V01 loc1 -// | \--* LCL_VAR int V04 loc4 -// \--* LCL_VAR int V06 loc6 +// \--* IND int +// \--* INDEX_ADDR byref int[] +// +--* FIELD ref Item1 +// | \--* INDEX_ADDR byref System.ValueTuple`2[System.Int32[],System.Int32][] +// | +--* LCL_VAR ref V00 arg0 +// | \--* LCL_VAR int V01 arg1 +// \--* LCL_VAR int V02 arg2 // // Morph "hoists" the bounds check above the struct field access: // // \--* COMMA int // +--* ASG ref -// | +--* LCL_VAR ref V23 tmp16 +// | +--* LCL_VAR ref V04 tmp1 // | \--* COMMA ref // | +--* BOUNDS_CHECK_Rng void -// | | +--* LCL_VAR int V04 loc4 +// | | +--* LCL_VAR int V01 arg1 // | | \--* ARR_LENGTH int -// | | \--* LCL_VAR ref V01 loc1 +// | | \--* LCL_VAR ref V00 arg0 // | \--* IND ref -// | \--* ADDR byref Zero Fseq[Item1] -// | \--* IND struct -// | \--* ADD byref -// | +--* LCL_VAR ref V01 loc1 -// | \--* ADD long -// | +--* LSH long -// | | +--* CAST long <- uint -// | | | \--* LCL_VAR int V04 loc4 -// | | \--* CNS_INT long 4 -// | \--* CNS_INT long 16 Fseq[#FirstElem] +// | \--* ARR_ADDR byref System.ValueTuple`2[System.Int32[],System.Int32][] Zero Fseq[Item1] +// | \--* ADD byref +// | +--* LCL_VAR ref V00 arg0 +// | \--* ADD long +// | +--* LSH long +// | | +--* CAST long <- uint +// | | | \--* LCL_VAR int V01 arg1 +// | | \--* CNS_INT long 4 +// | \--* CNS_INT long 16 // \--* COMMA int // +--* BOUNDS_CHECK_Rng void -// | +--* LCL_VAR int V06 loc6 +// | +--* LCL_VAR int V02 arg2 // | \--* ARR_LENGTH int -// | \--* LCL_VAR ref V23 tmp16 +// | \--* LCL_VAR ref V04 tmp1 // \--* IND int -// \--* ADD byref -// +--* LCL_VAR ref V23 tmp16 -// \--* ADD long -// +--* LSH long -// | +--* CAST long <- uint -// | | \--* LCL_VAR int V06 loc6 -// | \--* CNS_INT long 2 -// \--* CNS_INT long 16 Fseq[#FirstElem] +// \--* ARR_ADDR byref int[] +// \--* ADD byref +// +--* LCL_VAR ref V04 tmp1 +// \--* ADD long +// +--* LSH long +// | +--* CAST long <- uint +// | | \--* LCL_VAR int V02 arg2 +// | \--* CNS_INT long 2 +// \--* CNS_INT long 16 // // This should not be parsed as a jagged array (e.g., a[i][j]). To ensure that it is not, the type of the -// GT_INDEX node is stashed in the GT_BOUNDS_CHECK node during morph. If we see a bounds check node where -// the GT_INDEX was not TYP_REF, then it must be the outermost jagged array level. E.g., if it is +// GT_INDEX_ADDR node is stashed in the GT_BOUNDS_CHECK node during morph. If we see a bounds check node +// where the GT_INDEX_ADDR was not TYP_REF, then it must be the outermost jagged array level. E.g., if it is // TYP_STRUCT, then we have an array of structs, and any further bounds checks must be of one of its fields. // // It would be much better if we didn't need to parse these trees at all, and did all this work pre-morph. diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 6898650f523cca..d6193f9f1983b7 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -4785,7 +4785,7 @@ GenTree* Compiler::fgMorphIndexAddr(GenTreeIndexAddr* indexAddr) addr = gtNewOperNode(GT_ADD, TYP_BYREF, arrRef, addr); } - // TODO-Throughout: bash the INDEX_ADDR to ARR_ADDR here instead of creating a new node. + // TODO-Throughput: 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); if (indexAddr->IsNotNull()) @@ -4825,7 +4825,7 @@ GenTree* Compiler::fgMorphIndexAddr(GenTreeIndexAddr* indexAddr) tree = fgMorphTree(tree); DBEXEC(tree == indexAddr, tree->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); - JITDUMP("fgMorphArrayIndex (after remorph):\n") + JITDUMP("fgMorphIndexAddr (after remorph):\n") DISPTREE(tree) return tree;