Fix `optComputeLoopSideEffects` to account for HWI stores (#61911)
authorSingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Mon, 29 Nov 2021 13:40:11 +0000 (16:40 +0300)
committerGitHub <noreply@github.com>
Mon, 29 Nov 2021 13:40:11 +0000 (14:40 +0100)
Otherwise we can end up not seeing the loop has memory havoc.

Also added an assert that will prevent this issue from arising in the future.

src/coreclr/jit/optimizer.cpp
src/tests/JIT/opt/Loops/LoopSideEffectsForHwiStores.cs [new file with mode: 0644]
src/tests/JIT/opt/Loops/LoopSideEffectsForHwiStores.csproj [new file with mode: 0644]

index 10aabca..39d2328 100644 (file)
@@ -7539,6 +7539,15 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
                         }
                         break;
 
+#ifdef FEATURE_HW_INTRINSICS
+                    case GT_HWINTRINSIC:
+                        if (tree->AsHWIntrinsic()->OperIsMemoryStore())
+                        {
+                            memoryHavoc |= memoryKindSet(GcHeap, ByrefExposed);
+                        }
+                        break;
+#endif // FEATURE_HW_INTRINSICS
+
                     case GT_LOCKADD:
                     case GT_XORR:
                     case GT_XAND:
@@ -7547,7 +7556,6 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
                     case GT_CMPXCHG:
                     case GT_MEMORYBARRIER:
                     {
-                        assert(!tree->OperIs(GT_LOCKADD) && "LOCKADD should not appear before lowering");
                         memoryHavoc |= memoryKindSet(GcHeap, ByrefExposed);
                     }
                     break;
@@ -7587,6 +7595,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
 
                     default:
                         // All other gtOper node kinds, leave 'memoryHavoc' unchanged (i.e. false)
+                        assert(!tree->OperRequiresAsgFlag());
                         break;
                 }
             }
diff --git a/src/tests/JIT/opt/Loops/LoopSideEffectsForHwiStores.cs b/src/tests/JIT/opt/Loops/LoopSideEffectsForHwiStores.cs
new file mode 100644 (file)
index 0000000..b07f347
--- /dev/null
@@ -0,0 +1,73 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Runtime.CompilerServices;
+using System.Runtime.Intrinsics;
+using System.Runtime.Intrinsics.X86;
+using System.Runtime.Intrinsics.Arm;
+
+unsafe class LoopSideEffectsForHwiStores
+{
+    public static int Main()
+    {
+        static bool VerifyExpectedVtor(Vector128<int> a) => a.Equals(Vector128.Create(4));
+
+        var a = new ClassWithVtor();
+
+        fixed (Vector128<int>* p = &a.VtorField)
+        {
+            if (Sse2.IsSupported && !VerifyExpectedVtor(ProblemWithSse2(a, (byte*)p)))
+            {
+                System.Console.WriteLine("ProblemWithSse2 failed!");
+                return 101;
+            }
+
+            if (AdvSimd.IsSupported && !VerifyExpectedVtor(ProblemWithAdvSimd(a, (byte*)p)))
+            {
+                System.Console.WriteLine("ProblemWithAdvSimd failed!");
+                return 101;
+            }
+        }
+
+        return 100;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static unsafe Vector128<int> ProblemWithSse2(ClassWithVtor a, byte* p)
+    {
+        Vector128<int> vtor = Vector128<int>.Zero;
+
+        a.VtorField = Vector128.Create(1);
+        a.VtorField = Sse2.Add(a.VtorField, a.VtorField);
+
+        for (int i = 0; i < 10; i++)
+        {
+            vtor = Sse2.Add(vtor, Sse2.Add(a.VtorField, a.VtorField));
+            Sse2.Store(p, Vector128<byte>.Zero);
+        }
+
+        return vtor;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static unsafe Vector128<int> ProblemWithAdvSimd(ClassWithVtor a, byte* p)
+    {
+        Vector128<int> vtor = Vector128<int>.Zero;
+
+        a.VtorField = Vector128.Create(1);
+        a.VtorField = AdvSimd.Add(a.VtorField, a.VtorField);
+
+        for (int i = 0; i < 10; i++)
+        {
+            vtor = AdvSimd.Add(vtor, AdvSimd.Add(a.VtorField, a.VtorField));
+            AdvSimd.Store(p, Vector128<byte>.Zero);
+        }
+
+        return vtor;
+    }
+
+    class ClassWithVtor
+    {
+        public Vector128<int> VtorField;
+    }
+}
diff --git a/src/tests/JIT/opt/Loops/LoopSideEffectsForHwiStores.csproj b/src/tests/JIT/opt/Loops/LoopSideEffectsForHwiStores.csproj
new file mode 100644 (file)
index 0000000..7e34380
--- /dev/null
@@ -0,0 +1,14 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <CLRTestPriority>1</CLRTestPriority>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>PdbOnly</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>