JIT: Add a check for forward sub store interference (#87615)

Forward sub was not taking into account that the source of the candidate
def can contain locals that may be changed by next statement that is
being forward substituted into. For example, it is not legal to forward
substitute V06 in the following case:

```
[000104] DA--G------ ▌  STORE_LCL_VAR short  V06 tmp5
[000109] ----------- └──▌  LCL_VAR   short  V07 tmp6          (last use)

[000139] -A--------- ▌  COMMA     void
[000121] DA--------- ├──▌  STORE_LCL_VAR int    V07 tmp6
[000145] ----------- │  └──▌  CAST      int <- short <- int
[000074] ----------- │     └──▌  CNS_INT   int    1
[000129] UA--------- └──▌  STORE_LCL_FLD short  V00 arg0         [+16]
[000143] -----------    └──▌  CAST      int <- short <- int
[000090] -----------       └──▌  LCL_VAR   int    V06 tmp5          (last use)
```

Fix #87614

src/coreclr/jit/compiler.h

@@ -6137,6 +6137,7 @@ private:
     PhaseStatus fgForwardSub();
     bool fgForwardSubBlock(BasicBlock* block);
     bool fgForwardSubStatement(Statement* statement);
+    bool fgForwardSubHasStoreInterference(Statement* defStmt, Statement* nextStmt, GenTree* nextStmtUse);
     void fgForwardSubUpdateLiveness(GenTree* newSubListFirst, GenTree* newSubListLast);
 
     // The given local variable, required to be a struct variable, is being assigned via

src/coreclr/jit/forwardsub.cpp

@@ -652,6 +652,14 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)
     //
     // if the next tree can't change the value of fwdSubNode or be impacted by fwdSubNode effects
     //
+    if (((fsv.GetFlags() & GTF_ASG) != 0) && fgForwardSubHasStoreInterference(stmt, nextStmt, fsv.GetNode()))
+    {
+        // We execute a store before the substitution local; that
+        // store could interfere with some of the locals in the source of
+        // the candidate def.
+        JITDUMP(" cannot reorder with potential interfering store\n");
+        return false;
+    }
     if (((fwdSubNode->gtFlags & GTF_CALL) != 0) && ((fsv.GetFlags() & GTF_ALL_EFFECT) != 0))
     {
         JITDUMP(" cannot reorder call with any side effect\n");
@@ -871,6 +879,69 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)
     return true;
 }
 
+//------------------------------------------------------------------------
+// fgForwardSubHasStoreInterference: Check if a forward sub candidate
+// interferes with stores in the statement it may be substituted into.
+//
+// Arguments:
+//    defStmt     - The statement with the def
+//    nextStmt    - The statement that is being substituted into
+//    nextStmtUse - Use of the local being substituted in the next statement
+//
+// Returns:
+//   True if there is interference.
+//
+// Remarks:
+//   We expect the caller to have checked for GTF_ASG before doing the precise
+//   check here.
+//
+bool Compiler::fgForwardSubHasStoreInterference(Statement* defStmt, Statement* nextStmt, GenTree* nextStmtUse)
+{
+    assert(defStmt->GetRootNode()->OperIsLocalStore());
+    assert(nextStmtUse->OperIsLocalRead());
+
+    GenTreeLclVarCommon* defNode = defStmt->GetRootNode()->AsLclVarCommon();
+
+    for (GenTreeLclVarCommon* defStmtLcl : defStmt->LocalsTreeList())
+    {
+        if (defStmtLcl == defNode)
+        {
+            break;
+        }
+
+        unsigned   defStmtLclNum       = defStmtLcl->GetLclNum();
+        LclVarDsc* defStmtLclDsc       = lvaGetDesc(defStmtLclNum);
+        unsigned   defStmtParentLclNum = BAD_VAR_NUM;
+        if (defStmtLclDsc->lvIsStructField)
+        {
+            defStmtParentLclNum = defStmtLclDsc->lvParentLcl;
+        }
+
+        for (GenTreeLclVarCommon* useStmtLcl : nextStmt->LocalsTreeList())
+        {
+            if (useStmtLcl == nextStmtUse)
+            {
+                break;
+            }
+
+            if (!useStmtLcl->OperIsLocalStore())
+            {
+                continue;
+            }
+
+            // If the next statement has a store earlier than the use and that
+            // store affects a local on the RHS of the forward sub candidate,
+            // then we have interference.
+            if ((useStmtLcl->GetLclNum() == defStmtLclNum) || (useStmtLcl->GetLclNum() == defStmtParentLclNum))
+            {
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
+
 //------------------------------------------------------------------------
 // fgForwardSubUpdateLiveness: correct liveness after performing a forward
 // substitution that added a new sub list of locals in a statement.

src/tests/JIT/Directed/physicalpromotion/fuzzlyn2.cs

@@ -0,0 +1,71 @@
+// 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 Xunit;
+
+// Generated by Fuzzlyn v1.5 on 2023-06-15 11:50:35
+// Run on X64 Windows
+// Seed: 1595496264005900603
+// Reduced from 20.9 KiB to 0.9 KiB in 00:00:18
+// Debug: Outputs 0
+// Release: Outputs 1
+public struct S0
+{
+    public bool F0;
+    public short F1;
+    public byte F2;
+    public uint F3;
+    public uint F4;
+    public short F5;
+    public sbyte F6;
+    public bool F7;
+    public S0(bool f0, short f1, byte f2, uint f3, uint f4, short f5, sbyte f6, bool f7)
+    {
+        F0 = f0;
+        F1 = f1;
+        F2 = f2;
+        F3 = f3;
+        F4 = f4;
+        F5 = f5;
+        F6 = f6;
+        F7 = f7;
+    }
+}
+
+public struct S1
+{
+    public S0 F1;
+    public S0 F2;
+}
+
+public class Runtime_87614
+{
+    [Fact]
+    public static int TestEntryPoint()
+    {
+        var vr1 = new S1();
+        int result = M3(vr1);
+        if (result != 0)
+        {
+            Console.WriteLine("FAIL: result is {0}", result);
+        }
+        return result == 0 ? 100 : 101;
+    }
+
+    public static int M3(S1 arg0)
+    {
+        arg0.F1 = new S0(arg0.F2.F0, 1, 0, arg0.F1.F4, arg0.F1.F4, arg0.F1.F1, 0, true);
+        Consume(arg0.F1.F0);
+        Consume(arg0.F1.F1);
+        Consume(arg0.F1.F4);
+        return arg0.F1.F5;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void Consume<T>(T value)
+    {
+    }
+}

src/tests/JIT/Directed/physicalpromotion/fuzzlyn2.csproj

@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+    <CLRTestEnvironmentVariable Include="DOTNET_JitEnablePhysicalPromotion" Value="1" />
+  </ItemGroup>
+</Project>