From 231f85f754110bea22930effcbd348babec32423 Mon Sep 17 00:00:00 2001
From: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Date: Sat, 29 Apr 2023 01:02:43 +0300
Subject: [PATCH] Fix volatility checks in VN and loop side effects code
 (#85467)

* 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 |  2 +-
 src/coreclr/jit/valuenum.cpp  | 17 +++++++----------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 1ee468aef9eec..eb84ce38a50a1 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -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;
diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp
index 8e4c218841e20..28d8e85abaa73 100644
--- a/src/coreclr/jit/valuenum.cpp
+++ b/src/coreclr/jit/valuenum.cpp
@@ -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)
         {