[WIP] CopyProp Improvements

- Within a block, don't remove last uses from the live set, enabling us to catch the `b = a (last use); c = b` case.
- We don't label defs VNs, so lookup the VN for DEF, not just USASG.

Fix #24912

src/jit/compiler.h

@@ -6226,13 +6226,18 @@ class Compiler
 public:
     // VN based copy propagation.
     typedef ArrayStack<GenTree*> GenTreePtrStack;
-    typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, GenTreePtrStack*> LclNumToGenTreePtrStack;
+    typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, GenTreePtrStack*>     LclNumToGenTreePtrStack;
+    typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, GenTreeLclVarCommon*> LclNumToLclNode;
 
     // Kill set to track variables with intervening definitions.
     VARSET_TP optCopyPropKillSet;
 
     // Copy propagation functions.
-    void optCopyProp(BasicBlock* block, GenTreeStmt* stmt, GenTree* tree, LclNumToGenTreePtrStack* curSsaName);
+    void optCopyProp(BasicBlock*              block,
+                     GenTreeStmt*             stmt,
+                     GenTree*                 tree,
+                     LclNumToGenTreePtrStack* curSsaName,
+                     LclNumToLclNode*         lastUses);
     void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
     void optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
     bool optIsSsaLocal(GenTree* tree);

src/jit/compiler.hpp

@@ -4155,7 +4155,8 @@ bool Compiler::fgStructTempNeedsExplicitZeroInit(LclVarDsc* varDsc, BasicBlock*
 /*****************************************************************************/
 ValueNum Compiler::GetUseAsgDefVNOrTreeVN(GenTree* op)
 {
-    if (op->gtFlags & GTF_VAR_USEASG)
+    // We don't assign value numbers to defs.
+    if ((op->gtFlags & (GTF_VAR_USEASG | GTF_VAR_DEF)) != 0)
     {
         unsigned lclNum = op->AsLclVarCommon()->GetLclNum();
         unsigned ssaNum = GetSsaNumForLocalVarDef(op);

src/jit/copyprop.cpp

@@ -128,7 +128,8 @@ int Compiler::optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDs
 //    tree        -  The tree to perform copy propagation on
 //    curSsaName  -  The map from lclNum to its recently live definitions as a stack
 
-void Compiler::optCopyProp(BasicBlock* block, GenTreeStmt* stmt, GenTree* tree, LclNumToGenTreePtrStack* curSsaName)
+void Compiler::optCopyProp(
+    BasicBlock* block, GenTreeStmt* stmt, GenTree* tree, LclNumToGenTreePtrStack* curSsaName, LclNumToLclNode* lastUses)
 {
     // TODO-Review: EH successor/predecessor iteration seems broken.
     if (block->bbCatchTyp == BBCT_FINALLY || block->bbCatchTyp == BBCT_FAULT)
@@ -269,6 +270,14 @@ void Compiler::optCopyProp(BasicBlock* block, GenTreeStmt* stmt, GenTree* tree,
         tree->gtLclVarCommon.SetLclNum(newLclNum);
         tree->AsLclVarCommon()->SetSsaNum(newSsaNum);
         gtUpdateSideEffects(stmt, tree);
+        GenTreeLclVarCommon** lastUsePtr = lastUses->LookupPointer(newLclNum);
+        if (lastUsePtr != nullptr)
+        {
+            GenTreeLclVarCommon* oldLastUse = *lastUsePtr;
+            oldLastUse->SetLastUse(false);
+            tree->AsLclVarCommon()->SetLastUse(true);
+            *lastUsePtr = tree->AsLclVarCommon();
+        }
 #ifdef DEBUG
         if (verbose)
         {
@@ -311,6 +320,7 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curS
 #endif
 
     TreeLifeUpdater<false> treeLifeUpdater(this);
+    LclNumToLclNode        lastUses(getAllocator(CMK_CopyProp));
 
     // There are no definitions at the start of the block. So clear it.
     compCurLifeTree = nullptr;
@@ -322,9 +332,26 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curS
         // Walk the tree to find if any local variable can be replaced with current live definitions.
         for (GenTree* tree = stmt->gtStmtList; tree != nullptr; tree = tree->gtNext)
         {
+            // We are going to avoid removing last uses from our liveness vector locally.
+            // This allows us to propagate copies we would otherwise miss, e.g.
+            //    V1 = V0; (V0 is a last use)
+            //    use V1
+            // We re-mark it after calling UpdateLife, but we remember it so that we can clear
+            // it if we add a later use.
+            //
+            bool isLastUse = (tree->OperIs(GT_LCL_VAR) && tree->AsLclVarCommon()->IsLastUse());
+            if (isLastUse)
+            {
+                tree->AsLclVarCommon()->SetLastUse(false);
+                lastUses.Set(tree->AsLclVarCommon()->gtLclNum, tree->AsLclVarCommon(), LclNumToLclNode::Overwrite);
+            }
             treeLifeUpdater.UpdateLife(tree);
+            if (isLastUse)
+            {
+                tree->AsLclVarCommon()->SetLastUse(true);
+            }
 
-            optCopyProp(block, stmt, tree, curSsaName);
+            optCopyProp(block, stmt, tree, curSsaName, &lastUses);
 
             // TODO-Review: Merge this loop with the following loop to correctly update the
             // live SSA num while also propagating copies.

src/jit/gentree.h

@@ -1948,17 +1948,6 @@ struct GenTree
         ClearRegOptional();
     }
 
-    bool IsRegVarDeath() const
-    {
-        unreached();
-        return (gtFlags & GTF_VAR_DEATH) ? true : false;
-    }
-    bool IsRegVarBirth() const
-    {
-        unreached();
-        return (gtFlags & GTF_REG_BIRTH) ? true : false;
-    }
-
     bool IsReverseOp() const
     {
         return (gtFlags & GTF_REVERSE_OPS) ? true : false;
@@ -2716,6 +2705,16 @@ struct GenTreeLclVarCommon : public GenTreeUnOp
         return (gtSsaNum != SsaConfig::RESERVED_SSA_NUM);
     }
 
+    bool IsLastUse() const
+    {
+        return ((gtFlags & GTF_VAR_DEATH) != 0) ? true : false;
+    }
+
+    void SetLastUse(bool value)
+    {
+        gtFlags = (value) ? (gtFlags | GTF_VAR_DEATH) : (gtFlags & ~GTF_VAR_DEATH);
+    }
+
 #if DEBUGGABLE_GENTREE
     GenTreeLclVarCommon() : GenTreeUnOp()
     {

tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_17435/GitHub_17435.cs

@@ -1,3 +1,7 @@
+// 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;
 using System.Runtime.InteropServices;

tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_17957/GitHub_17957.cs

@@ -1,3 +1,7 @@
+// 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.Intrinsics.X86;
 using System.Runtime.Intrinsics;

tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666.cs

@@ -1,3 +1,7 @@
+// 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.Intrinsics.X86;
 using System.Runtime.Intrinsics;

tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21855/GitHub_21855.cs

@@ -1,3 +1,7 @@
+// 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.Intrinsics.X86;
 using System.Runtime.Intrinsics;

tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21899/GitHub_21899.cs

@@ -1,3 +1,7 @@
+// 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.Intrinsics.X86;
 using System.Runtime.Intrinsics;

tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_22815/GitHub_22815.cs

@@ -1,3 +1,7 @@
+// 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.Intrinsics.X86;
 using System.Runtime.Intrinsics;

tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_23438/GitHub_23438.cs

@@ -1,3 +1,7 @@
+// 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;
 using System.Runtime.Intrinsics;

tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_24912/GitHub_24912.cs

@@ -0,0 +1,57 @@
+// 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;
+using System.Runtime.InteropServices;
+using System.Runtime.Intrinsics.X86;
+using System.Runtime.Intrinsics;
+
+
+public class GitHub_17435
+{
+    const int Pass = 100;
+    const int Fail = 0;
+    static int Main()
+    {
+        Ok(new[] { Vector128.Create(1.0f) });
+        NotOk(new[] { Vector128.Create(1.0f) });
+        return Pass;
+    }
+
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining | MethodImplOptions.AggressiveOptimization)]
+    public static Vector128<float> Add(Vector128<float> left, Vector128<float> right)
+    {
+        if (Sse.IsSupported)
+        {
+            return Sse.Add(left, right);
+        }
+
+        return default;
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining | MethodImplOptions.AggressiveOptimization)]
+    public static Vector128<float> AddNoCheck(Vector128<float> left, Vector128<float> right)
+    {
+        return Sse.Add(left, right);
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void Ok(Vector128<float>[] src)
+    {
+        var vector = src[0];
+        AddNoCheck(vector, vector);
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void NotOk(Vector128<float>[] src)
+    {
+        // In this version, when the Add is inlined, the local copy propagation doesn't preserve the
+        // assertion that both 'left' and 'right' are equal to 'vector', so we wind up with an extra
+        // copy.
+        var vector = src[0];
+        Add(vector, vector);
+    }
+}

tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_24912/GitHub_24912.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>