From 71bc8d937a04ed074c8e5d2da562fea7c1485c31 Mon Sep 17 00:00:00 2001
From: Kunal Pathak <Kunal.Pathak@microsoft.com>
Date: Thu, 20 May 2021 11:53:32 -0700
Subject: [PATCH] GT_COPY source should be marked as consume during resolution

---
 docs/design/coreclr/jit/lsra-detail.md |  6 ++--
 src/coreclr/jit/clrjit.natvis          | 38 ++++++++++++++++++++++++++
 src/coreclr/jit/lsra.cpp               | 14 ++++++++--
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/docs/design/coreclr/jit/lsra-detail.md b/docs/design/coreclr/jit/lsra-detail.md
index e3cb0c0c01f11f..12c8969b374635 100644
--- a/docs/design/coreclr/jit/lsra-detail.md
+++ b/docs/design/coreclr/jit/lsra-detail.md
@@ -170,11 +170,11 @@ There are four main phases to LSRA:
 
         -   For fork edges (the source block has multiple targets, but
             each target has only that one source), any required
-            resolution is placed at the target.
+            resolution is placed at the individual target(s).
 
         -   For join edges (a single target block has multiple sources,
             but each source has only that one target), any required
-            resolution is placed at the source.
+            resolution is placed at the individual source(s).
 
         -   Critical edges require more complicated handling, and may
             require splitting of the edge for placement of resolution.
@@ -707,7 +707,7 @@ LinearScanAllocation(List<RefPosition> refPositions)
            - Next, for the remaining variables, classify them as either:
              - In different registers at one or more targets. These require that the edge
                be split so that we can insert the move on the edge (this is the `diffResolutionSet`).
-             - In the same register at each target (this is the `sameResolutionSet`).
+             - In the same register at each target (this is the `sameResolutionSet`), but different from the end of this block.
                For these, we can insert a move at the end of this block, as long as they
                don't write to any of the registers read by the `diffResolutionSet` as those
                must remain live into the split block.
diff --git a/src/coreclr/jit/clrjit.natvis b/src/coreclr/jit/clrjit.natvis
index c242727719bc8f..07cfc192b5ae04 100644
--- a/src/coreclr/jit/clrjit.natvis
+++ b/src/coreclr/jit/clrjit.natvis
@@ -57,6 +57,44 @@ The .NET Foundation licenses this file to you under the MIT license.
   <Type Name="LinearScan">
     <DisplayString>LinearScan</DisplayString>
     <Expand>
+        <Item Name="inVarToRegMaps">"InVarToRegMaps"</Item>
+        <CustomListItems>
+          <Variable Name="block" InitialValue="this-&gt;compiler-&gt;fgFirstBB" />
+          <Variable Name="bbLiveInMap" InitialValue="block ? block->bbLiveIn[0] : 0" />
+          <Variable Name="inVarMap" InitialValue="block ? this->inVarToRegMaps[block->bbNum] : 0" />
+          <Variable Name="varIndex" InitialValue="0" />
+          <Loop Condition="block">
+              <Item Name="---BB{block->bbNum,2u}---">block->bbNum,2u</Item>
+              <Exec>varIndex = 0</Exec>
+              <Exec>inVarMap = this->inVarToRegMaps[block->bbNum]</Exec>
+              <Exec>bbLiveInMap = block->bbLiveIn[0]</Exec>
+              <Loop Condition="bbLiveInMap != 0">
+                <Item Name="V{this->localVarIntervals[varIndex]->varNum,2u}" Condition="(bbLiveInMap &amp; 1) != 0">((regNumber)inVarMap[varIndex]),en</Item>
+                <Exec>varIndex++</Exec>
+                <Exec>bbLiveInMap = bbLiveInMap >> 1</Exec>
+              </Loop>
+            <Exec>block = block->bbNext</Exec>
+          </Loop>
+        </CustomListItems>
+        <Item Name="outVarToRegMaps">"OutVarToRegMaps"</Item>
+        <CustomListItems>
+          <Variable Name="block" InitialValue="this-&gt;compiler-&gt;fgFirstBB" />
+          <Variable Name="bbLiveInMap" InitialValue="block ? block->bbLiveIn[0] : 0" />
+          <Variable Name="outVarMap" InitialValue="block ? this->outVarToRegMaps[block->bbNum] : 0" />
+          <Variable Name="varIndex" InitialValue="0" />
+          <Loop Condition="block">
+              <Item Name="---BB{block->bbNum,2u}---">block->bbNum,2u</Item>
+              <Exec>varIndex = 0</Exec>
+              <Exec>outVarMap = this->outVarToRegMaps[block->bbNum]</Exec>
+              <Exec>bbLiveInMap = block->bbLiveIn[0]</Exec>
+              <Loop Condition="bbLiveInMap != 0">
+                <Item Name="V{this->localVarIntervals[varIndex]->varNum,2u}" Condition="(bbLiveInMap &amp; 1) != 0">((regNumber)outVarMap[varIndex]),en</Item>
+                <Exec>varIndex++</Exec>
+                <Exec>bbLiveInMap = bbLiveInMap >> 1</Exec>
+              </Loop>
+            <Exec>block = block->bbNext</Exec>
+          </Loop>
+        </CustomListItems>
         <Item Name="AvailableRegs mask">this-&gt;m_AvailableRegs</Item>
         <CustomListItems>
           <Variable Name="reg" InitialValue="this->m_AvailableRegs" />
diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp
index 8417b1d63e9a77..d5c4e952067631 100644
--- a/src/coreclr/jit/lsra.cpp
+++ b/src/coreclr/jit/lsra.cpp
@@ -7589,6 +7589,14 @@ void LinearScan::handleOutgoingCriticalEdges(BasicBlock* block)
         assert(varTypeIsIntegralOrI(op1) && varTypeIsIntegralOrI(op2));
         consumedRegs |= genRegMask(op1->GetRegNum());
         consumedRegs |= genRegMask(op2->GetRegNum());
+
+        // Special handling for GT_COPY to not resolve into the source
+        // of switch's operand.
+        if (op1->OperIs(GT_COPY))
+        {
+            GenTree* srcOp1 = op1->gtGetOp1();
+            consumedRegs |= genRegMask(srcOp1->GetRegNum());
+        }
     }
 
 #ifdef TARGET_ARM64
@@ -7717,7 +7725,7 @@ void LinearScan::handleOutgoingCriticalEdges(BasicBlock* block)
             //
             // Another way to achieve similar resolution for vars live only at split edges is by removing them
             // from consideration up-front but it requires that we traverse those edges anyway to account for
-            // the registers that must note be overwritten.
+            // the registers that must not be overwritten.
             if (liveOnlyAtSplitEdge && maybeSameLivePaths)
             {
                 sameToReg = REG_NA;
@@ -9578,7 +9586,7 @@ void LinearScan::dumpLsraAllocationEvent(
         // Conflicting def/use
         case LSRA_EVENT_DEFUSE_CONFLICT:
             dumpRefPositionShort(activeRefPosition, currentBlock);
-            printf("DUconflict ");
+            printf("DUconflict    ");
             dumpRegRecords();
             break;
         case LSRA_EVENT_DEFUSE_CASE1:
@@ -10056,7 +10064,7 @@ void LinearScan::dumpNewBlock(BasicBlock* currentBlock, LsraLocation location)
     if (activeRefPosition->refType == RefTypeDummyDef)
     {
         dumpEmptyRefPosition();
-        printf("DDefs ");
+        printf("DDefs   ");
         printf(regNameFormat, "");
         return;
     }