From 03f18391aefe8ea7fcd5339f2f390f47ac06c833 Mon Sep 17 00:00:00 2001
From: Andy Ayers <andya@microsoft.com>
Date: Mon, 31 Jul 2023 14:52:17 -0700
Subject: [PATCH 1/2] JIT: don't dead store untracked OSR locals based on ref
 count

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.

So, we can't rely on ref counts to enable a dead store of an OSR local.

Fixes #89666.
---
 src/coreclr/jit/liveness.cpp               |  6 ++-
 src/tests/JIT/opt/OSR/Runtime_89666.cs     | 50 ++++++++++++++++++++++
 src/tests/JIT/opt/OSR/Runtime_89666.csproj | 15 +++++++
 3 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 src/tests/JIT/opt/OSR/Runtime_89666.cs
 create mode 100644 src/tests/JIT/opt/OSR/Runtime_89666.csproj

diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp
index 517dc848dc6b35..851b08f6c8ef1c 100644
--- a/src/coreclr/jit/liveness.cpp
+++ b/src/coreclr/jit/liveness.cpp
@@ -1575,7 +1575,11 @@ bool Compiler::fgComputeLifeUntrackedLocal(VARSET_TP&           life,
 
     // We have accurate ref counts when running late liveness so we can eliminate
     // some stores if the lhs local has a ref count of 1.
-    if (isDef && compRationalIRForm && (varDsc.lvRefCnt() == 1) && !varDsc.lvPinned)
+    //
+    // For OSR locals we can't rely on ref counts this way, since some of the appearances
+    // of the local may be in the now-unseen Tier0 portion.
+    //
+    if (isDef && compRationalIRForm && (varDsc.lvRefCnt() == 1) && !varDsc.lvPinned && !varDsc.lvIsOSRLocal)
     {
         if (varDsc.lvIsStructField)
         {
diff --git a/src/tests/JIT/opt/OSR/Runtime_89666.cs b/src/tests/JIT/opt/OSR/Runtime_89666.cs
new file mode 100644
index 00000000000000..5a21e92d341024
--- /dev/null
+++ b/src/tests/JIT/opt/OSR/Runtime_89666.cs
@@ -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
index 00000000000000..9fc3a6fe026a8a
--- /dev/null
+++ b/src/tests/JIT/opt/OSR/Runtime_89666.csproj
@@ -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>

From cabc3103706c49041f09e5b2e118bb1d3a0d7bb5 Mon Sep 17 00:00:00 2001
From: Andy Ayers <andya@microsoft.com>
Date: Tue, 1 Aug 2023 11:33:14 -0700
Subject: [PATCH 2/2] use implicit reference instead

---
 src/coreclr/jit/lclvars.cpp  | 6 ++++++
 src/coreclr/jit/liveness.cpp | 6 +-----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp
index 5f6352d8fc2e6e..04ddc4fcca391d 100644
--- a/src/coreclr/jit/lclvars.cpp
+++ b/src/coreclr/jit/lclvars.cpp
@@ -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/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp
index 851b08f6c8ef1c..517dc848dc6b35 100644
--- a/src/coreclr/jit/liveness.cpp
+++ b/src/coreclr/jit/liveness.cpp
@@ -1575,11 +1575,7 @@ bool Compiler::fgComputeLifeUntrackedLocal(VARSET_TP&           life,
 
     // We have accurate ref counts when running late liveness so we can eliminate
     // some stores if the lhs local has a ref count of 1.
-    //
-    // For OSR locals we can't rely on ref counts this way, since some of the appearances
-    // of the local may be in the now-unseen Tier0 portion.
-    //
-    if (isDef && compRationalIRForm && (varDsc.lvRefCnt() == 1) && !varDsc.lvPinned && !varDsc.lvIsOSRLocal)
+    if (isDef && compRationalIRForm && (varDsc.lvRefCnt() == 1) && !varDsc.lvPinned)
     {
         if (varDsc.lvIsStructField)
         {