Mark local struct as having overlapping fields after struct reinterpretation

Methods like System.Runtime.CompilerServices.Unsafe.As<TFrom, TTo> may
have struct reinterpretation when function signature specifies Struct1&
and the method returns Struct2& where Struct1 and Struct2 are different
structs. This may confuse jit optimizations (in particular, value
numbering) because fields of a struct of type Struct1 may be accessed
using handles of Struct2. This fix marks the source local involved in
such struct reinterpretation as having overlapping fields. That prevents
SSA builder from inserting the local into SSA.

Fixes #24159.

No diffs in framework assemblies and coreclr benchmarks.

4 files changed, 117 insertions, 5 deletions

diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
index 05eb086fdf..993889d0db 100644
--- a/src/jit/importer.cpp
+++ b/src/jit/importer.cpp
@@ -16428,7 +16428,6 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
                     }
 
                     op2 = tmpOp2;
-
 #ifdef DEBUG
                     if (impInlineInfo->retExpr)
                     {
@@ -16440,6 +16439,45 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
 #endif
                 }
 
+                // If we are inlining a method that returns a struct byref, check whether we are "reinterpreting" the
+                // struct.
+                GenTree* effectiveRetVal = op2->gtEffectiveVal();
+                if ((returnType == TYP_BYREF) && (info.compRetType == TYP_BYREF) &&
+                    (effectiveRetVal->OperGet() == GT_ADDR))
+                {
+                    GenTree* addrChild = effectiveRetVal->gtGetOp1();
+                    if (addrChild->OperGet() == GT_LCL_VAR)
+                    {
+                        LclVarDsc* varDsc = lvaGetDesc(addrChild->AsLclVarCommon());
+
+                        if (varTypeIsStruct(addrChild->TypeGet()) && !isOpaqueSIMDLclVar(varDsc))
+                        {
+                            CORINFO_CLASS_HANDLE referentClassHandle;
+                            CorInfoType          referentType =
+                                info.compCompHnd->getChildType(info.compMethodInfo->args.retTypeClass,
+                                                               &referentClassHandle);
+                            if (varTypeIsStruct(JITtype2varType(referentType)) &&
+                                (varDsc->lvVerTypeInfo.GetClassHandle() != referentClassHandle))
+                            {
+                                // We are returning a byref to struct1; the method signature specifies return type as
+                                // byref
+                                // to struct2. struct1 and struct2 are different so we are "reinterpreting" the struct.
+                                // This may happen in, for example, System.Runtime.CompilerServices.Unsafe.As<TFrom,
+                                // TTo>.
+                                // We need to mark the source struct variable as having overlapping fields because its
+                                // fields may be accessed using field handles of a different type, which may confuse
+                                // optimizations, in particular, value numbering.
+
+                                JITDUMP("\nSetting lvOverlappingFields to true on V%02u because of struct "
+                                        "reinterpretation\n",
+                                        addrChild->AsLclVarCommon()->gtLclNum);
+
+                                varDsc->lvOverlappingFields = true;
+                            }
+                        }
+                    }
+                }
+
 #ifdef DEBUG
                 if (verbose)
                 {
diff --git a/tests/scripts/run-corefx-tests-exclusions.txt b/tests/scripts/run-corefx-tests-exclusions.txt
index 7027caa833..0c852dcf68 100644
--- a/tests/scripts/run-corefx-tests-exclusions.txt
+++ b/tests/scripts/run-corefx-tests-exclusions.txt
@@ -20,10 +20,6 @@
 #
 # Please list a GitHub issue for every exclusion.
 
-# https://github.com/dotnet/coreclr/issues/24159
--nomethod "System.Buffers.Tests.ArrayBufferWriterTests_Byte.Advance"
--nomethod "System.Buffers.Tests.ArrayBufferWriterTests_Char.Advance"
-
 # https://github.com/dotnet/coreclr/issues/22442
 -nomethod "Microsoft.Win32.SystemEventsTests.CreateTimerTests.ConcurrentTimers"
 -nomethod "Microsoft.Win32.SystemEventsTests.SessionSwitchTests.SignalsSessionSwitch"
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_24159/GitHub_24159.cs b/tests/src/JIT/Regression/JitBlue/GitHub_24159/GitHub_24159.cs
new file mode 100644
index 0000000000..8017ca5e0c
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_24159/GitHub_24159.cs
@@ -0,0 +1,61 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Runtime.CompilerServices;
+
+namespace GitHub_24159
+{
+
+    public struct Str1
+    {
+        public int i1;
+        public int i2;
+        public int i3;
+        public int i4;
+        public int i5;
+    }
+
+    public struct Str2
+    {
+        public int j1;
+        public int j2;
+        public int j3;
+        public int j4;
+        public int j5;
+    }
+
+    class Test
+    {
+        static int i;
+
+        public static int Main()
+        {
+            i = 0;
+
+            Str1 str1 = new Str1();
+
+            if (i != 0)
+            {
+                str1 = new Str1();
+            }
+            else
+            {
+                str1.i2 = 7;
+            }
+
+            // This call reinterprets the struct.
+            Str2 str2 = Unsafe.As<Str1, Str2>(ref str1);
+
+            // The bug was that value numbering couldn't recognize
+            // that this field has been updated on str1.
+            int k = str2.j2;
+
+            Console.WriteLine(k);
+
+            return k + 93;
+
+        }
+    }
+}
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_24159/GitHub_24159.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_24159/GitHub_24159.csproj
new file mode 100644
index 0000000000..d86ed9f3d7
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_24159/GitHub_24159.csproj
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Release</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <OutputType>Exe</OutputType>
+    <DebugType></DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>
\ No newline at end of file