Fix wrong copy prop after ASG(promoted LCL_VAR with 1 field, call). (#41197)

* Add Andy's repro test.

* Andy's fix.

* fix other places

* Improve dump.

Author: Sergey Andreenko

src/coreclr/src/jit/compiler.h

@@ -6512,7 +6512,7 @@ class Compiler
     void optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, LclNumToGenTreePtrStack* curSsaName);
     void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
     void optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
-    bool optIsSsaLocal(GenTree* tree);
+    unsigned optIsSsaLocal(GenTree* tree);
     int optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDsc, bool preferOp2);
     void optVnCopyProp();
     INDEBUG(void optDumpCopyPropStack(LclNumToGenTreePtrStack* curSsaName));

src/coreclr/src/jit/copyprop.cpp

@@ -36,8 +36,8 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrSt
             {
                 continue;
             }
-            unsigned lclNum = tree->AsLclVarCommon()->GetLclNum();
-            if (!lvaInSsa(lclNum))
+            const unsigned lclNum = optIsSsaLocal(tree);
+            if (lclNum == BAD_VAR_NUM)
             {
                 continue;
             }
@@ -61,8 +61,19 @@ void Compiler::optDumpCopyPropStack(LclNumToGenTreePtrStack* curSsaName)
     JITDUMP("{ ");
     for (LclNumToGenTreePtrStack::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter)
     {
-        GenTree* node = iter.GetValue()->Top();
-        JITDUMP("%d-[%06d]:V%02u ", iter.Get(), dspTreeID(node), node->AsLclVarCommon()->GetLclNum());
+        GenTreeLclVarCommon* lclVar    = iter.GetValue()->Top()->AsLclVarCommon();
+        unsigned             ssaLclNum = optIsSsaLocal(lclVar);
+        assert(ssaLclNum != BAD_VAR_NUM);
+
+        if (ssaLclNum == lclVar->GetLclNum())
+        {
+            JITDUMP("%d-[%06d]:V%02u ", iter.Get(), dspTreeID(lclVar), ssaLclNum);
+        }
+        else
+        {
+            // A promoted field was asigned using the parent struct, print `ssa field lclNum(parent lclNum)`.
+            JITDUMP("%d-[%06d]:V%02u(V%02u) ", iter.Get(), dspTreeID(lclVar), ssaLclNum, lclVar->GetLclNum());
+        }
     }
     JITDUMP("}\n\n");
 }
@@ -150,10 +161,10 @@ void Compiler::optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, Lc
     {
         return;
     }
-    unsigned lclNum = tree->AsLclVarCommon()->GetLclNum();
+    const unsigned lclNum = optIsSsaLocal(tree);
 
     // Skip non-SSA variables.
-    if (!lvaInSsa(lclNum))
+    if (lclNum == BAD_VAR_NUM)
     {
         return;
     }
@@ -291,13 +302,39 @@ void Compiler::optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, Lc
     return;
 }
 
-/**************************************************************************************
- *
- * Helper to check if tree is a local that participates in SSA numbering.
- */
-bool Compiler::optIsSsaLocal(GenTree* tree)
+//------------------------------------------------------------------------------
+// optIsSsaLocal : helper to check if the tree is a local that participates in SSA numbering.
+//
+// Arguments:
+//    tree        -  The tree to perform the check on;
+//
+// Returns:
+//    - lclNum if the local is participating in SSA;
+//    - fieldLclNum if the parent local can be replaced by its only field;
+//    - BAD_VAR_NUM otherwise.
+//
+unsigned Compiler::optIsSsaLocal(GenTree* tree)
 {
-    return tree->IsLocal() && lvaInSsa(tree->AsLclVarCommon()->GetLclNum());
+    if (!tree->IsLocal())
+    {
+        return BAD_VAR_NUM;
+    }
+
+    GenTreeLclVarCommon* lclNode = tree->AsLclVarCommon();
+    unsigned             lclNum  = lclNode->GetLclNum();
+    LclVarDsc*           varDsc  = lvaGetDesc(lclNum);
+
+    if (!lvaInSsa(lclNum) && varDsc->CanBeReplacedWithItsField(this))
+    {
+        lclNum = varDsc->lvFieldLclStart;
+    }
+
+    if (!lvaInSsa(lclNum))
+    {
+        return BAD_VAR_NUM;
+    }
+
+    return lclNum;
 }
 
 //------------------------------------------------------------------------------
@@ -351,22 +388,22 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curS
             // embedded update. Killing the variable is a simplification to produce 0 ASM diffs
             // for an update release.
             //
-            if (optIsSsaLocal(tree) && (tree->gtFlags & GTF_VAR_DEF))
+            const unsigned lclNum = optIsSsaLocal(tree);
+            if ((lclNum != BAD_VAR_NUM) && (tree->gtFlags & GTF_VAR_DEF))
             {
-                VarSetOps::AddElemD(this, optCopyPropKillSet, lvaTable[tree->AsLclVarCommon()->GetLclNum()].lvVarIndex);
+                VarSetOps::AddElemD(this, optCopyPropKillSet, lvaTable[lclNum].lvVarIndex);
             }
         }
 
         // This logic must be in sync with SSA renaming process.
         for (GenTree* tree = stmt->GetTreeList(); tree != nullptr; tree = tree->gtNext)
         {
-            if (!optIsSsaLocal(tree))
+            const unsigned lclNum = optIsSsaLocal(tree);
+            if (lclNum == BAD_VAR_NUM)
             {
                 continue;
             }
 
-            unsigned lclNum = tree->AsLclVarCommon()->GetLclNum();
-
             // As we encounter a definition add it to the stack as a live definition.
             if (tree->gtFlags & GTF_VAR_DEF)
             {

src/coreclr/src/jit/valuenum.cpp

@@ -6789,6 +6789,10 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
             {
                 fgMutateAddressExposedLocal(tree DEBUGARG("COPYBLK - address-exposed local"));
             }
+            else
+            {
+                JITDUMP("LHS V%02u not in ssa at [%06u], so no VN assigned\n", lhsLclNum, dspTreeID(lclVarTree));
+            }
         }
         else
         {

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

@@ -0,0 +1,67 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// The test was showing a wrong copy propogation when a struct field was rewritten by 
+// a call assignment to the parent struct but that assignment was not supported by copyprop.
+
+using System;
+using System.Collections.Immutable;
+using System.Runtime.CompilerServices;
+
+class X
+{
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void E(ImmutableArray<string> a) {}
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static ImmutableArray<string> G() => ImmutableArray<string>.Empty;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static ImmutableArray<string> H()
+    {
+        string[] a = new string[100];
+        
+        for (int i = 0; i < a.Length; i++)
+        {
+            a[i] = "hello";
+        }
+
+        return ImmutableArray.Create<string>(a);
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]    
+    public static int F()
+    {
+        var a = H();
+        int r = 0;
+
+        foreach (var s in a)
+        {
+            if (s.Equals("hello")) r++;
+        }
+
+        var aa = a;
+
+        if (r > 0)
+
+        {
+            foreach (var s in a)
+            {
+                if (s.Equals("hello")) r--;
+            }
+        
+            aa = G();
+
+            foreach (var s in a)
+            {
+                if (s.Equals("hello")) r++;
+            }
+        }
+
+        E(aa);
+
+        return r;
+    }
+
+    public static int Main() => F();
+}

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

@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+  <PropertyGroup>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>