Fix volatility checks in VN and loop side effects code (#85467)
authorSingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Fri, 28 Apr 2023 22:02:43 +0000 (01:02 +0300)
committerGitHub <noreply@github.com>
Fri, 28 Apr 2023 22:02:43 +0000 (00:02 +0200)
* Fix volatile check in loop side effects code

* Restore the volatility logic in VN

Volatile stores should be processed at the point of ASG.

src/coreclr/jit/optimizer.cpp
src/coreclr/jit/valuenum.cpp

index 1ee468a..eb84ce3 100644 (file)
@@ -8640,7 +8640,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
                 {
                     GenTree* arg = lhs->AsIndir()->Addr()->gtEffectiveVal(/*commaOnly*/ true);
 
-                    if ((tree->gtFlags & GTF_IND_VOLATILE) != 0)
+                    if ((lhs->gtFlags & GTF_IND_VOLATILE) != 0)
                     {
                         memoryHavoc |= memoryKindSet(GcHeap, ByrefExposed);
                         continue;
index 8e4c218..28d8e85 100644 (file)
@@ -10721,8 +10721,13 @@ void Compiler::fgValueNumberTree(GenTree* tree)
             ValueNumPair addrXvnp;
             vnStore->VNPUnpackExc(addr->gtVNPair, &addrNvnp, &addrXvnp);
 
+            // To be able to propagate exception sets, we give location nodes the "Void" VN.
+            if ((tree->gtFlags & GTF_IND_ASG_LHS) != 0)
+            {
+                tree->gtVNPair = vnStore->VNPWithExc(vnStore->VNPForVoid(), addrXvnp);
+            }
             // Is the dereference immutable?  If so, model it as referencing the read-only heap.
-            if (tree->gtFlags & GTF_IND_INVARIANT)
+            else if (tree->gtFlags & GTF_IND_INVARIANT)
             {
                 assert(!isVolatile); // We don't expect both volatile and invariant
 
@@ -10822,9 +10827,7 @@ void Compiler::fgValueNumberTree(GenTree* tree)
                 ValueNum newUniq = vnStore->VNForExpr(compCurBB, tree->TypeGet());
                 tree->gtVNPair   = vnStore->VNPWithExc(ValueNumPair(newUniq, newUniq), 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)
+            else
             {
                 var_types loadType = tree->TypeGet();
                 ssize_t   offset   = 0;
@@ -10867,12 +10870,6 @@ void Compiler::fgValueNumberTree(GenTree* tree)
 
                 tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp);
             }
-
-            // To be able to propagate exception sets, we give location nodes the "Void" VN.
-            if ((tree->gtFlags & GTF_IND_ASG_LHS) != 0)
-            {
-                tree->gtVNPair = vnStore->VNPWithExc(vnStore->VNPForVoid(), addrXvnp);
-            }
         }
         else if (tree->OperGet() == GT_CAST)
         {