From 33bd22b3e82b3e207a2e078c51c03a8f215ba8b1 Mon Sep 17 00:00:00 2001 From: Joseph Tremoulet Date: Fri, 13 Jan 2017 12:29:37 -0500 Subject: [PATCH] Refactor fgCurHeapVN and HeapSsaMap updates Consolodate the code for updating the current heap VN and HeapSsaMap entries into new helper method `recordHeapStore`, to make it easier to evolve that code. Change `fgValueNumberArrIndexAssign` to return the new heap VN to allow callers to pass it down into `recordHeapStore` accordingly. This is just a refactoring; no change to compiler behavior. Commit migrated from https://github.com/dotnet/coreclr/commit/5501ecb0855f295c0c60c3743c440badd5d67d61 --- src/coreclr/src/jit/compiler.h | 17 +++++++----- src/coreclr/src/jit/valuenum.cpp | 57 ++++++++++++++++++++-------------------- 2 files changed, 39 insertions(+), 35 deletions(-) diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index abcc769..7698731 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -3804,18 +3804,18 @@ public: // tree node). void fgValueNumber(); - // Updates "fgCurHeap" via the assignment H[elemTypeEq][arrVN][inx][fldSeq] = rhsVN. + // Computes new heap VN via the assignment H[elemTypeEq][arrVN][inx][fldSeq] = rhsVN. // Assumes that "elemTypeEq" is the (equivalence class rep) of the array element type. // The 'indType' is the indirection type of the lhs of the assignment and will typically // match the element type of the array or fldSeq. When this type doesn't match // or if the fldSeq is 'NotAField' we invalidate the array contents H[elemTypeEq][arrVN] // - void fgValueNumberArrIndexAssign(CORINFO_CLASS_HANDLE elemTypeEq, - ValueNum arrVN, - ValueNum inxVN, - FieldSeqNode* fldSeq, - ValueNum rhsVN, - var_types indType); + ValueNum fgValueNumberArrIndexAssign(CORINFO_CLASS_HANDLE elemTypeEq, + ValueNum arrVN, + ValueNum inxVN, + FieldSeqNode* fldSeq, + ValueNum rhsVN, + var_types indType); // Requires that "tree" is a GT_IND marked as an array index, and that its address argument // has been parsed to yield the other input arguments. If evaluation of the address @@ -3854,6 +3854,9 @@ public: // Called when an operation (performed by "tree", described by "msg") may cause the global Heap to be mutated. void fgMutateHeap(GenTreePtr tree DEBUGARG(const char* msg)); + // For a store at curTree, ecord the new heapVN in curHeapVN and curTree's HeapSsaMap entry. + void recordHeapStore(GenTreePtr curTree, ValueNum heapVN DEBUGARG(const char* msg)); + // Tree caused an update in the current heap VN. If "tree" has an associated heap SSA #, record that // value in that SSA #. void fgValueNumberRecordHeapSsa(GenTreePtr tree); diff --git a/src/coreclr/src/jit/valuenum.cpp b/src/coreclr/src/jit/valuenum.cpp index 603610e..651b45b 100644 --- a/src/coreclr/src/jit/valuenum.cpp +++ b/src/coreclr/src/jit/valuenum.cpp @@ -2712,12 +2712,12 @@ ValueNum ValueNumStore::ExtendPtrVN(GenTreePtr opA, FieldSeqNode* fldSeq) return res; } -void Compiler::fgValueNumberArrIndexAssign(CORINFO_CLASS_HANDLE elemTypeEq, - ValueNum arrVN, - ValueNum inxVN, - FieldSeqNode* fldSeq, - ValueNum rhsVN, - var_types indType) +ValueNum Compiler::fgValueNumberArrIndexAssign(CORINFO_CLASS_HANDLE elemTypeEq, + ValueNum arrVN, + ValueNum inxVN, + FieldSeqNode* fldSeq, + ValueNum rhsVN, + var_types indType) { bool invalidateArray = false; ValueNum elemTypeEqVN = vnStore->VNForHandle(ssize_t(elemTypeEq), GTF_ICON_CLASS_HDL); @@ -2813,10 +2813,7 @@ void Compiler::fgValueNumberArrIndexAssign(CORINFO_CLASS_HANDLE elemTypeEq, } #endif // DEBUG - // bbHeapDef must be set to true for any block that Mutates the global Heap - assert(compCurBB->bbHeapDef); - - fgCurHeapVN = vnStore->VNForMapStore(TYP_REF, fgCurHeapVN, elemTypeEqVN, newValAtArrType); + return vnStore->VNForMapStore(TYP_REF, fgCurHeapVN, elemTypeEqVN, newValAtArrType); } ValueNum Compiler::fgValueNumberArrIndexVal(GenTreePtr tree, VNFuncApp* pFuncApp, ValueNum addrXvn) @@ -4756,22 +4753,26 @@ ValueNum Compiler::fgHeapVNForLoopSideEffects(BasicBlock* entryBlock, unsigned i void Compiler::fgMutateHeap(GenTreePtr tree DEBUGARG(const char* msg)) { + // Update the current heap VN, and if we're tracking the heap SSA # caused by this node, record it. + recordHeapStore(tree, vnStore->VNForExpr(compCurBB, TYP_REF) DEBUGARG(msg)); +} + +void Compiler::recordHeapStore(GenTreePtr curTree, ValueNum heapVN DEBUGARG(const char* msg)) +{ // bbHeapDef must be set to true for any block that Mutates the global Heap assert(compCurBB->bbHeapDef); - - fgCurHeapVN = vnStore->VNForExpr(compCurBB, TYP_REF); - - // If we're tracking the heap SSA # caused by this node, record it. - fgValueNumberRecordHeapSsa(tree); + fgCurHeapVN = heapVN; #ifdef DEBUG if (verbose) { printf(" fgCurHeapVN assigned by %s at ", msg); - Compiler::printTreeID(tree); - printf(" to new unique VN: " STR_VN "%x.\n", fgCurHeapVN); + Compiler::printTreeID(curTree); + printf(" to VN: " STR_VN "%x.\n", heapVN); } #endif // DEBUG + + fgValueNumberRecordHeapSsa(curTree); } void Compiler::fgValueNumberRecordHeapSsa(GenTreePtr tree) @@ -5832,9 +5833,9 @@ void Compiler::fgValueNumberTree(GenTreePtr tree, bool evalAsgLhsInd) } #endif // DEBUG - fgValueNumberArrIndexAssign(elemTypeEq, arrVN, inxVN, fldSeq, rhsVNPair.GetLiberal(), - lhs->TypeGet()); - fgValueNumberRecordHeapSsa(tree); + ValueNum heapVN = fgValueNumberArrIndexAssign(elemTypeEq, arrVN, inxVN, fldSeq, + rhsVNPair.GetLiberal(), lhs->TypeGet()); + recordHeapStore(tree, heapVN DEBUGARG("Array element assignment")); } // It may be that we haven't parsed it yet. Try. else if (lhs->gtFlags & GTF_IND_ARR_INDEX) @@ -5869,9 +5870,9 @@ void Compiler::fgValueNumberTree(GenTreePtr tree, bool evalAsgLhsInd) fldSeq = GetFieldSeqStore()->Append(fldSeq, zeroOffsetFldSeq); } - fgValueNumberArrIndexAssign(elemTypeEq, arrVN, inxVN, fldSeq, rhsVNPair.GetLiberal(), - lhs->TypeGet()); - fgValueNumberRecordHeapSsa(tree); + ValueNum heapVN = fgValueNumberArrIndexAssign(elemTypeEq, arrVN, inxVN, fldSeq, + rhsVNPair.GetLiberal(), lhs->TypeGet()); + recordHeapStore(tree, heapVN DEBUGARG("assignment to unparseable array expression")); } else if (arg->IsFieldAddr(this, &obj, &staticOffset, &fldSeq)) { @@ -5980,10 +5981,11 @@ void Compiler::fgValueNumberTree(GenTreePtr tree, bool evalAsgLhsInd) assert(compCurBB->bbHeapDef); // Update the field map for firstField in Heap to this new value. - fgCurHeapVN = vnStore->VNApplySelectorsAssign(VNK_Liberal, fgCurHeapVN, firstFieldOnly, - newFldMapVN, indType, compCurBB); + ValueNum heapVN = + vnStore->VNApplySelectorsAssign(VNK_Liberal, fgCurHeapVN, firstFieldOnly, + newFldMapVN, indType, compCurBB); - fgValueNumberRecordHeapSsa(tree); + recordHeapStore(tree, heapVN DEBUGARG("StoreField")); } } else @@ -6033,8 +6035,7 @@ void Compiler::fgValueNumberTree(GenTreePtr tree, bool evalAsgLhsInd) assert(compCurBB->bbHeapDef); // Update the field map for the fgCurHeapVN - fgCurHeapVN = storeVal; - fgValueNumberRecordHeapSsa(tree); + recordHeapStore(tree, storeVal DEBUGARG("Static Field store")); } break; -- 2.7.4