Account for global stores in hoisting properly (#67213)

* Take HWI stores into account in hoisting

* Also fix ordinary stores while we're at it

The contract for whether the LHS of a global ASG(IND, ...)
should be marked with GLOB_REF is not clearly specified.

* Add tests

src/coreclr/jit/optimizer.cpp

@@ -6900,23 +6900,27 @@ void Compiler::optHoistLoopBlocks(unsigned loopNum, ArrayStack<BasicBlock*>* blo
                         }
                     }
                 }
-                else if (tree->OperIs(GT_ASG))
+                else if (tree->OperRequiresAsgFlag())
                 {
-                    // If the LHS of the assignment has a global reference, then assume it's a global side effect.
-                    GenTree* lhs = tree->AsOp()->gtOp1;
-                    if (lhs->gtFlags & GTF_GLOB_REF)
+                    // Assume all stores except "ASG(non-addr-exposed LCL, ...)" are globally visible.
+                    GenTreeLclVarCommon* lclNode;
+                    bool                 isGloballyVisibleStore;
+                    if (tree->OperIs(GT_ASG) && tree->DefinesLocal(m_compiler, &lclNode))
                     {
+                        isGloballyVisibleStore = m_compiler->lvaGetDesc(lclNode)->IsAddressExposed();
+                    }
+                    else
+                    {
+                        isGloballyVisibleStore = true;
+                    }
+
+                    if (isGloballyVisibleStore)
+                    {
+                        INDEBUG(failReason = "store to globally visible memory");
+                        treeIsHoistable    = false;
                         m_beforeSideEffect = false;
                     }
                 }
-                else if (tree->OperIs(GT_XADD, GT_XORR, GT_XAND, GT_XCHG, GT_LOCKADD, GT_CMPXCHG, GT_MEMORYBARRIER))
-                {
-                    // If this node is a MEMORYBARRIER or an Atomic operation
-                    // then don't hoist and stop any further hoisting after this node
-                    INDEBUG(failReason = "atomic op or memory barrier";)
-                    treeIsHoistable    = false;
-                    m_beforeSideEffect = false;
-                }
             }
 
             // If this 'tree' is hoistable then we return and the caller will

src/tests/JIT/opt/Hoisting/Hoisting.cs

+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.CompilerServices;
+using System.Runtime.Intrinsics;
+using System.Runtime.Intrinsics.X86;
+using System.Runtime.Intrinsics.Arm;
+
+unsafe class Hoisting
+{
+    public static int Main()
+    {
+        var p = stackalloc int[4];
+        p[0] = 1;
+        try
+        {
+            ProblemWithHwiStore(p, -1);
+            return 101;
+        }
+        catch (OverflowException)
+        {
+            if (p[0] != 0)
+            {
+                return 102;
+            }
+        }
+
+        try
+        {
+            ProblemWithNormalStore(p, -1);
+            return 103;
+        }
+        catch (OverflowException)
+        {
+            if (p[0] != -1)
+            {
+                return 104;
+            }
+        }
+
+        return 100;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void ProblemWithHwiStore(int* p, int b)
+    {
+        // Make sure we don't hoist the checked cast.
+        for (int i = 0; i < 10; i++)
+        {
+            Vector128.Store(Vector128<int>.Zero, p);
+            *p = (int)checked((uint)b);
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void ProblemWithNormalStore(int* p, int b)
+    {
+        // Make sure we don't hoist the checked cast.
+        for (int i = 0; i < 10; i++)
+        {
+            *p = b;
+            *p = (int)checked((uint)b);
+        }
+    }
+}

src/tests/JIT/opt/Hoisting/Hoisting.csproj

+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>PdbOnly</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>