Fix multi-reg copy (#37062)

Also, fix dump for multi-reg copy. Fix #37059
dotnet · May 27, 2020 · ef72b95 · ef72b95
1 parent 0c043d5
commit ef72b95
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 24 deletions.
diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp
@@ -11750,8 +11750,8 @@ void CodeGen::genRegCopy(GenTree* treeNode)
         // There should never be any circular dependencies, and we will check that here.
 
         GenTreeCopyOrReload* copyNode = treeNode->AsCopyOrReload();
-        unsigned             regCount = copyNode->GetRegCount();
-        // GenTreeCopyOrReload only reports the number of registers that are valid.
+        // GenTreeCopyOrReload only reports the highest index that has a valid register.
+        unsigned regCount = copyNode->GetRegCount();
         assert(regCount <= MAX_MULTIREG_COUNT);
 
         // First set the source registers as busy if they haven't been spilled.
@@ -11767,13 +11767,15 @@ void CodeGen::genRegCopy(GenTree* treeNode)
         // First do any copies - we'll do the reloads after all the copies are complete.
         for (unsigned i = 0; i < regCount; ++i)
         {
-            regNumber sourceReg     = op1->GetRegByIndex(i);
-            regNumber targetReg     = copyNode->GetRegNumByIdx(i);
-            regMaskTP targetRegMask = genRegMask(targetReg);
-            // GenTreeCopyOrReload only reports the number of registers that are valid.
+            regNumber sourceReg = op1->GetRegByIndex(i);
+            regNumber targetReg = copyNode->GetRegNumByIdx(i);
+            // GenTreeCopyOrReload only reports the highest index that has a valid register.
+            // However there may be lower indices that have no valid register (i.e. the register
+            // on the source is still valid at the consumer).
             if (targetReg != REG_NA)
             {
                 // We shouldn't specify a no-op move.
+                regMaskTP targetRegMask = genRegMask(targetReg);
                 assert(sourceReg != targetReg);
                 assert((busyRegs & targetRegMask) == 0);
                 // Clear sourceReg from the busyRegs, and add targetReg.

diff --git a/src/coreclr/src/jit/gentree.cpp b/src/coreclr/src/jit/gentree.cpp
@@ -10205,9 +10205,10 @@ void Compiler::gtDispRegVal(GenTree* tree)
             break;
     }
 
+#if FEATURE_MULTIREG_RET
     if (tree->IsMultiRegCall())
     {
-        // 0th reg is GettRegNum(), which is already printed above.
+        // 0th reg is GetRegNum(), which is already printed above.
         // Print the remaining regs of a multi-reg call node.
         const GenTreeCall* call     = tree->AsCall();
         const unsigned     regCount = call->GetReturnTypeDesc()->TryGetReturnRegCount();
@@ -10216,28 +10217,37 @@ void Compiler::gtDispRegVal(GenTree* tree)
             printf(",%s", compRegVarName(call->GetRegNumByIdx(i)));
         }
     }
-    else if (tree->IsCopyOrReloadOfMultiRegCall())
+    else if (tree->IsCopyOrReload())
     {
+        GenTree*                   op1          = tree->gtGetOp1();
         const GenTreeCopyOrReload* copyOrReload = tree->AsCopyOrReload();
-        const GenTreeCall*         call         = tree->gtGetOp1()->AsCall();
-        const unsigned             regCount     = call->GetReturnTypeDesc()->TryGetReturnRegCount();
-        for (unsigned i = 1; i < regCount; ++i)
+        unsigned                   regCount     = 0;
+        if (op1->OperIs(GT_CALL))
         {
-            printf(",%s", compRegVarName(copyOrReload->GetRegNumByIdx(i)));
-        }
-    }
-
-#if FEATURE_MULTIREG_RET
-    if (tree->IsCopyOrReload())
-    {
-        for (int i = 1; i < MAX_RET_REG_COUNT; i++)
-        {
-            regNumber reg = (regNumber)tree->AsCopyOrReload()->GetRegNumByIdx(i);
-            if (reg == REG_NA)
+            if (op1->IsMultiRegCall())
             {
-                break;
+                regCount = op1->AsCall()->GetReturnTypeDesc()->TryGetReturnRegCount();
+                // If it hasn't yet been initialized, we'd still like to see the registers printed.
+                if (regCount == 0)
+                {
+                    regCount = MAX_RET_REG_COUNT;
+                }
             }
-            printf(",%s", compRegVarName(reg));
+        }
+        else if (op1->IsMultiRegLclVar())
+        {
+            regCount = op1->AsLclVar()->GetFieldCount(this);
+        }
+        else if (op1->IsMultiRegNode())
+        {
+            regCount = op1->GetMultiRegCount();
+        }
+        // We will only have valid regs for positions that require copy or reload.
+        // But we'd like to keep track of where they are so we print all positions.
+        for (unsigned i = 1; i < regCount; i++)
+        {
+            regNumber reg = tree->AsCopyOrReload()->GetRegNumByIdx(i);
+            printf(",%s", (reg == REG_NA) ? ",NA" : compRegVarName(reg));
         }
     }
 #endif