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

diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h
index abcc769..7698731 100644 (file)
--- 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 (file)
--- 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;