JIT: don't dead store OSR exposed locals (#89743)
authorAndy Ayers <andya@microsoft.com>
Tue, 1 Aug 2023 22:15:25 +0000 (15:15 -0700)
committerGitHub <noreply@github.com>
Tue, 1 Aug 2023 22:15:25 +0000 (15:15 -0700)
The local var ref count for OSR locals can be misleading, since some
of the appearances may be in the "tier0" parts of the methods and
won't be visible once we trim the OSR method down to just the part
we're going to compile.

Fix by giving OSR-exposed locals an implicit ref count.

Fixes #89666.

src/coreclr/jit/lclvars.cpp
src/tests/JIT/opt/OSR/Runtime_89666.cs [new file with mode: 0644]
src/tests/JIT/opt/OSR/Runtime_89666.csproj [new file with mode: 0644]

index 5f6352d..04ddc4f 100644 (file)
@@ -339,6 +339,12 @@ void Compiler::lvaInitTypeRef()
             {
                 JITDUMP("-- V%02u is OSR exposed\n", lclNum);
                 varDsc->lvIsOSRExposedLocal = true;
+
+                // Ensure that ref counts for exposed OSR locals take into account
+                // that some of the refs might be in the Tier0 parts of the method
+                // that get trimmed away.
+                //
+                varDsc->lvImplicitlyReferenced = 1;
             }
         }
     }
diff --git a/src/tests/JIT/opt/OSR/Runtime_89666.cs b/src/tests/JIT/opt/OSR/Runtime_89666.cs
new file mode 100644 (file)
index 0000000..5a21e92
--- /dev/null
@@ -0,0 +1,50 @@
+// 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.InteropServices;
+using Xunit;
+
+public class Runtime_89666
+{
+    [Fact]
+    public static int Test()
+    {
+        byte[] a1 = new byte[100_000];
+        byte[] a2 = new byte[100_000];
+
+        Problem(a1.Length, a1);
+        Problem(a2.Length, a2);
+
+        for (int i = 0; i < a1.Length; i++)
+        {
+            if (a1[i] != a2[i])
+            {
+                Console.WriteLine($"Found diff at {i}: {a1[i]} != {a2[i]}");
+                return -1;
+            }
+        }
+        return 100;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static void Problem(int n, byte[] a)
+    {
+        Random random = new Random(1);
+        int value = 0;
+        Span<byte> span = MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref value, 1));
+        for (int i = 0; i < n; i++)
+        {
+            // This write must be kept in the OSR method
+            value = random.Next();
+            Write(span, i, a);
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static void Write(Span<byte> s, int i, byte[] a)
+    {
+        a[i] = s[0];
+    }
+}
diff --git a/src/tests/JIT/opt/OSR/Runtime_89666.csproj b/src/tests/JIT/opt/OSR/Runtime_89666.csproj
new file mode 100644 (file)
index 0000000..9fc3a6f
--- /dev/null
@@ -0,0 +1,15 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <!-- Needed for CLRTestEnvironmentVariable -->
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
+    <DebugType />
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+    <CLRTestEnvironmentVariable Include="DOTNET_TieredCompilation" Value="1" />
+    <CLRTestEnvironmentVariable Include="DOTNET_TieredPGO" Value="0" />
+    <CLRTestEnvironmentVariable Include="DOTNET_TC_QuickJitForLoops" Value="1" />
+    <CLRTestEnvironmentVariable Include="DOTNET_TC_OnStackReplacement" Value="1" />
+  </ItemGroup>
+</Project>