JIT: Avoid removing multi-use boxes (#72842)

There was an assumption that the local operand to a GT_BOX node is
single-use which was being violated when GDV clones these as arguments.
We now allow multi-uses of these locals by setting a flag when cloning
and then handle it in gtTryRemoveBoxUpstreamEffects.

There is also GTF_VAR_CLONED that would be set on the local itself, but
given that transformations can affect the operand local node arbitrarily
I went with another of these type of flags on GT_BOX instead.

Fix #72775

5 files changed, 99 insertions, 0 deletions

diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp
index 968d9eb9716..d0f775b9efc 100644
--- a/src/coreclr/jit/compiler.cpp
+++ b/src/coreclr/jit/compiler.cpp
@@ -9539,6 +9539,11 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
                 {
                     chars += printf("[BOX_VALUE]");
                 }
+
+                if (tree->gtFlags & GTF_BOX_CLONED)
+                {
+                    chars += printf("[BOX_CLONED]");
+                }
                 break;
 
             case GT_ARR_ADDR:
diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index 8381c11b026..65809969a12 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -8508,6 +8508,8 @@ GenTree* Compiler::gtCloneExpr(
                 copy = new (this, GT_BOX)
                     GenTreeBox(tree->TypeGet(), tree->AsOp()->gtOp1, tree->AsBox()->gtAsgStmtWhenInlinedBoxValue,
                                tree->AsBox()->gtCopyStmtWhenInlinedBoxValue);
+                tree->AsBox()->SetCloned();
+                copy->AsBox()->SetCloned();
                 break;
 
             case GT_INTRINSIC:
@@ -13612,6 +13614,13 @@ GenTree* Compiler::gtTryRemoveBoxUpstreamEffects(GenTree* op, BoxRemovalOptions
         return nullptr;
     }
 
+    // If this box is no longer single-use, bail.
+    if (box->WasCloned())
+    {
+        JITDUMP(" bailing; unsafe to remove box that has been cloned\n");
+        return nullptr;
+    }
+
     // If we're eventually going to return the type handle, remember it now.
     GenTree* boxTypeHandle = nullptr;
     if ((options == BR_REMOVE_AND_NARROW_WANT_TYPE_HANDLE) || (options == BR_DONT_REMOVE_WANT_TYPE_HANDLE))
diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h
index a088f4c2241..e9d512e16c6 100644
--- a/src/coreclr/jit/gentree.h
+++ b/src/coreclr/jit/gentree.h
@@ -543,6 +543,7 @@ enum GenTreeFlags : unsigned int
     GTF_QMARK_CAST_INSTOF       = 0x80000000, // GT_QMARK -- Is this a top (not nested) level qmark created for
                                               //             castclass or instanceof?
 
+    GTF_BOX_CLONED              = 0x40000000, // GT_BOX -- this box and its operand has been cloned, cannot assume it to be single-use anymore
     GTF_BOX_VALUE               = 0x80000000, // GT_BOX -- "box" is on a value type
 
     GTF_ARR_ADDR_NONNULL        = 0x80000000, // GT_ARR_ADDR -- this array's address is not null
@@ -3874,6 +3875,16 @@ struct GenTreeBox : public GenTreeUnOp
     {
     }
 #endif
+
+    bool WasCloned()
+    {
+        return (gtFlags & GTF_BOX_CLONED) != 0;
+    }
+
+    void SetCloned()
+    {
+        gtFlags |= GTF_BOX_CLONED;
+    }
 };
 
 // GenTreeField -- data member ref (GT_FIELD)
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_72775/Runtime_72775.cs b/src/tests/JIT/Regression/JitBlue/Runtime_72775/Runtime_72775.cs
new file mode 100644
index 00000000000..a53cb3bb0d4
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_72775/Runtime_72775.cs
@@ -0,0 +1,53 @@
+// 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.Threading;
+
+public class Runtime_72775
+{
+    public static int Main()
+    {
+        for (int i = 0; i < 100; i++)
+        {
+            Call(new Impl1());
+            if (i > 30 && i < 40)
+                Thread.Sleep(10);
+        }
+
+        // With GDV, JIT would optimize Call by fully removing the box since Impl1.Foo does not use it.
+        // This would cause null to be passed to Impl2.Foo.
+        return Call(new Impl2());
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static int Call(I i) => i.Foo(5);
+}
+
+public interface I
+{
+    int Foo(object o);
+}
+
+class Impl1 : I
+{
+    public int Foo(object o) => 0;
+}
+
+class Impl2 : I
+{
+    public int Foo(object o)
+    {
+        if (o is not int i || i != 5)
+        {
+            Console.WriteLine("FAIL: Got {0}", o?.ToString() ?? "(null)");
+            return -1;
+        }
+        else
+        {
+            Console.WriteLine("PASS: Got 5");
+            return 100;
+        }
+    }
+}
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_72775/Runtime_72775.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_72775/Runtime_72775.csproj
new file mode 100644
index 00000000000..db201bf0c84
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_72775/Runtime_72775.csproj
@@ -0,0 +1,21 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+  <PropertyGroup>
+    <CLRTestBatchPreCommands><![CDATA[
+$(CLRTestBatchPreCommands)
+set COMPlus_TieredCompilation=1
+set COMPlus_TieredPGO=1
+]]></CLRTestBatchPreCommands>
+    <BashCLRTestPreCommands><![CDATA[
+$(BashCLRTestPreCommands)
+export COMPlus_TieredCompilation=1
+export COMPlus_TieredPGO=1
+]]></BashCLRTestPreCommands>
+  </PropertyGroup>
+</Project>
\ No newline at end of file