[release/8.0] JIT: Fix physical promotion creating overlapping local uses

src/coreclr/jit/promotion.cpp

@@ -2340,8 +2340,36 @@ void ReplaceVisitor::ReadBackAfterCall(GenTreeCall* call, GenTree* user)
 //
 //   If the remainder of the struct local is dying, then we expect that this
 //   entire struct local is now dying, since all field accesses are going to be
-//   replaced with other locals. The exception is if there is a queued read
-//   back for any of the fields.
+//   replaced with other locals.
+//
+//   There are two exceptions to the above:
+//
+//     1) If there is a queued readback for any of the fields, then there is
+//     live state in the struct local, so it is not dying.
+//
+//     2) If there are further uses of the local in the same statement then we cannot
+//     actually act on the last-use information we would provide here. That's because
+//     uses of locals occur at the user and we do not model that here. In the real model
+//     there are cases where we do not have any place to insert any IR between the two uses.
+//     For example, consider:
+//
+//       ▌  CALL      void   Program:Foo(Program+S,Program+S)
+//       ├──▌  LCL_VAR   struct<Program+S, 4> V01 loc0
+//       └──▌  LCL_VAR   struct<Program+S, 4> V01 loc0
+//
+//     If V01 is promoted fully then both uses of V01 are last uses here; but
+//     replacing the IR with
+//
+//       ▌  CALL      void   Program:Foo(Program+S,Program+S)
+//       ├──▌  LCL_VAR   struct<Program+S, 4> V01 loc0          (last use)
+//       └──▌  COMMA     struct
+//          ├──▌  STORE_LCL_FLD int    V01 loc0         [+0]
+//          │  └──▌  LCL_VAR   int    V02 tmp0
+//          └──▌  LCL_VAR   struct<Program+S, 4> V01 loc0          (last use)
+//
+//     would be illegal since the created store overlaps with the first local,
+//     and does not take into account that both uses occur simultaneously at
+//     the position of the CALL node.
 //
 bool ReplaceVisitor::IsPromotedStructLocalDying(GenTreeLclVarCommon* lcl)
 {
@@ -2362,6 +2390,15 @@ bool ReplaceVisitor::IsPromotedStructLocalDying(GenTreeLclVarCommon* lcl)
         }
     }
 
+    for (GenTree* cur = lcl->gtNext; cur != nullptr; cur = cur->gtNext)
+    {
+        assert(cur->OperIsAnyLocal());
+        if (cur->TypeIs(TYP_STRUCT) && (cur->AsLclVarCommon()->GetLclNum() == lcl->GetLclNum()))
+        {
+            return false;
+        }
+    }
+
     return true;
 }
 
@@ -2577,7 +2614,7 @@ void ReplaceVisitor::WriteBackBeforeCurrentStatement(unsigned lcl, unsigned offs
 
         GenTree*   readBack = Promotion::CreateWriteBack(m_compiler, lcl, rep);
         Statement* stmt     = m_compiler->fgNewStmtFromTree(readBack);
-        JITDUMP("Writing back %s before " FMT_STMT "\n", rep.Description, stmt->GetID());
+        JITDUMP("Writing back %s before " FMT_STMT "\n", rep.Description, m_currentStmt->GetID());
         DISPSTMT(stmt);
         m_compiler->fgInsertStmtBefore(m_currentBlock, m_currentStmt, stmt);
         ClearNeedsWriteBack(rep);

src/tests/JIT/Regression/JitBlue/Runtime_81725/Runtime_81725.csproj

@@ -7,8 +7,6 @@
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="$(MSBuildProjectName).cs" />
-  </ItemGroup>
-  <ItemGroup>
     <CLRTestEnvironmentVariable Include="DOTNET_JitStressModeNames" Value="STRESS_FOLD" />
   </ItemGroup>
 </Project>

src/tests/JIT/Regression/JitBlue/Runtime_85088/Runtime_85088.csproj

@@ -1,5 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
+    <!-- Needed for CLRTestEnvironmentVariable -->
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
     <Optimize>True</Optimize>
   </PropertyGroup>
   <ItemGroup>

src/tests/JIT/Regression/JitBlue/Runtime_91056/Runtime_91056.cs

@@ -0,0 +1,33 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Xunit;
+using System.Runtime.CompilerServices;
+
+public class Runtime_91056
+{
+    [Fact]
+    public static void TestEntryPoint()
+    {
+        S s = default;
+        if (False())
+        {
+            s.A = 1234;
+        }
+
+        Foo(0, 0, s, s);
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static bool False() => false;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void Foo(int a, int b, S s1, S s2)
+    {
+    }
+
+    public struct S
+    {
+        public int A;
+    }
+}

src/tests/JIT/Regression/JitBlue/Runtime_91056/Runtime_91056.csproj

@@ -0,0 +1,12 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <!-- Needed for CLRTestEnvironmentVariable -->
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+    <CLRTestEnvironmentVariable Include="DOTNET_JitStressModeNames" Value="STRESS_PHYSICAL_PROMOTION STRESS_PHYSICAL_PROMOTION_COST STRESS_NO_OLD_PROMOTION" />
+    <CLRTestEnvironmentVariable Include="DOTNET_JitNoCSE" Value="1" />
+  </ItemGroup>
+</Project>