Handle multiReg COPY
authorCarol Eidt <carol.eidt@microsoft.com>
Mon, 27 Aug 2018 21:04:19 +0000 (14:04 -0700)
committerCarol Eidt <carol.eidt@microsoft.com>
Tue, 28 Aug 2018 20:32:22 +0000 (13:32 -0700)
On x86, `MUL_LONG` wasn't considered a multi-reg node, as it should be, so that when it gets spilled or copied, the additional register will be correctly handled.
Also, the ARM and X86 versions of genStoreLongLclVar should be identical and shared (neither version were handling the copy of a `MUL_LONG`).
Finally, fix the LSRA dumping of multi-reg nodes.

Fix dotnet/coreclr#19397

Commit migrated from https://github.com/dotnet/coreclr/commit/9e55f6cf2c20945077974311be03951ce9eb346b

14 files changed:
src/coreclr/src/jit/codegenarm.cpp
src/coreclr/src/jit/codegenlinear.cpp
src/coreclr/src/jit/codegenxarch.cpp
src/coreclr/src/jit/gentree.cpp
src/coreclr/src/jit/gentree.h
src/coreclr/src/jit/gtlist.h
src/coreclr/src/jit/gtstructs.h
src/coreclr/src/jit/lsra.cpp
src/coreclr/src/jit/lsrabuild.cpp
src/coreclr/src/jit/lsraxarch.cpp
src/coreclr/tests/arm/Tests.lst
src/coreclr/tests/arm64/Tests.lst
src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_19397/GitHub_19397.cs [new file with mode: 0644]
src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_19397/GitHub_19397.csproj [new file with mode: 0644]

index 7097fd7..63ef942 100644 (file)
@@ -1634,58 +1634,6 @@ void CodeGen::genEmitHelperCall(unsigned helper, int argSize, emitAttr retSize,
 }
 
 //------------------------------------------------------------------------
-// genStoreLongLclVar: Generate code to store a non-enregistered long lclVar
-//
-// Arguments:
-//    treeNode - A TYP_LONG lclVar node.
-//
-// Return Value:
-//    None.
-//
-// Assumptions:
-//    'treeNode' must be a TYP_LONG lclVar node for a lclVar that has NOT been promoted.
-//    Its operand must be a GT_LONG node.
-//
-void CodeGen::genStoreLongLclVar(GenTree* treeNode)
-{
-    emitter* emit = getEmitter();
-
-    GenTreeLclVarCommon* lclNode = treeNode->AsLclVarCommon();
-    unsigned             lclNum  = lclNode->gtLclNum;
-    LclVarDsc*           varDsc  = &(compiler->lvaTable[lclNum]);
-    assert(varDsc->TypeGet() == TYP_LONG);
-    assert(!varDsc->lvPromoted);
-    GenTree* op1 = treeNode->gtOp.gtOp1;
-    noway_assert(op1->OperGet() == GT_LONG || op1->OperGet() == GT_MUL_LONG);
-    genConsumeRegs(op1);
-
-    if (op1->OperGet() == GT_LONG)
-    {
-        // Definitions of register candidates will have been lowered to 2 int lclVars.
-        assert(!treeNode->gtHasReg());
-
-        GenTree* loVal = op1->gtGetOp1();
-        GenTree* hiVal = op1->gtGetOp2();
-
-        noway_assert((loVal->gtRegNum != REG_NA) && (hiVal->gtRegNum != REG_NA));
-
-        emit->emitIns_S_R(ins_Store(TYP_INT), EA_4BYTE, loVal->gtRegNum, lclNum, 0);
-        emit->emitIns_S_R(ins_Store(TYP_INT), EA_4BYTE, hiVal->gtRegNum, lclNum, genTypeSize(TYP_INT));
-    }
-    else if (op1->OperGet() == GT_MUL_LONG)
-    {
-        assert((op1->gtFlags & GTF_MUL_64RSLT) != 0);
-
-        GenTreeMultiRegOp* mul = op1->AsMultiRegOp();
-
-        // Stack store
-        getEmitter()->emitIns_S_R(ins_Store(TYP_INT), emitTypeSize(TYP_INT), mul->gtRegNum, lclNum, 0);
-        getEmitter()->emitIns_S_R(ins_Store(TYP_INT), emitTypeSize(TYP_INT), mul->gtOtherReg, lclNum,
-                                  genTypeSize(TYP_INT));
-    }
-}
-
-//------------------------------------------------------------------------
 // genCodeForMulLong: Generates code for int*int->long multiplication
 //
 // Arguments:
index 134a909..9962a62 100644 (file)
@@ -1947,3 +1947,57 @@ void CodeGen::genCodeForCast(GenTreeOp* tree)
     }
     // The per-case functions call genProduceReg()
 }
+
+#if !defined(_TARGET_64BIT_)
+//------------------------------------------------------------------------
+// genStoreLongLclVar: Generate code to store a non-enregistered long lclVar
+//
+// Arguments:
+//    treeNode - A TYP_LONG lclVar node.
+//
+// Return Value:
+//    None.
+//
+// Assumptions:
+//    'treeNode' must be a TYP_LONG lclVar node for a lclVar that has NOT been promoted.
+//    Its operand must be a GT_LONG node.
+//
+void CodeGen::genStoreLongLclVar(GenTree* treeNode)
+{
+    emitter* emit = getEmitter();
+
+    GenTreeLclVarCommon* lclNode = treeNode->AsLclVarCommon();
+    unsigned             lclNum  = lclNode->gtLclNum;
+    LclVarDsc*           varDsc  = &(compiler->lvaTable[lclNum]);
+    assert(varDsc->TypeGet() == TYP_LONG);
+    assert(!varDsc->lvPromoted);
+    GenTree* op1 = treeNode->gtOp.gtOp1;
+
+    // A GT_LONG is always contained, so it cannot have RELOAD or COPY inserted between it and its consumer,
+    // but a MUL_LONG may.
+    noway_assert(op1->OperIs(GT_LONG) || op1->gtSkipReloadOrCopy()->OperIs(GT_MUL_LONG));
+    genConsumeRegs(op1);
+
+    if (op1->OperGet() == GT_LONG)
+    {
+        GenTree* loVal = op1->gtGetOp1();
+        GenTree* hiVal = op1->gtGetOp2();
+
+        noway_assert((loVal->gtRegNum != REG_NA) && (hiVal->gtRegNum != REG_NA));
+
+        emit->emitIns_S_R(ins_Store(TYP_INT), EA_4BYTE, loVal->gtRegNum, lclNum, 0);
+        emit->emitIns_S_R(ins_Store(TYP_INT), EA_4BYTE, hiVal->gtRegNum, lclNum, genTypeSize(TYP_INT));
+    }
+    else
+    {
+        assert((op1->gtSkipReloadOrCopy()->gtFlags & GTF_MUL_64RSLT) != 0);
+        // This is either a multi-reg MUL_LONG, or a multi-reg reload or copy.
+        assert(op1->IsMultiRegNode() && (op1->GetMultiRegCount() == 2));
+
+        // Stack store
+        emit->emitIns_S_R(ins_Store(TYP_INT), emitTypeSize(TYP_INT), op1->GetRegByIndex(0), lclNum, 0);
+        emit->emitIns_S_R(ins_Store(TYP_INT), emitTypeSize(TYP_INT), op1->GetRegByIndex(1), lclNum,
+                          genTypeSize(TYP_INT));
+    }
+}
+#endif // !defined(_TARGET_64BIT_)
index 30e4d12..caccf47 100644 (file)
@@ -4530,29 +4530,41 @@ void CodeGen::genCodeForIndir(GenTreeIndir* tree)
     genProduceReg(tree);
 }
 
+//------------------------------------------------------------------------
+// genRegCopy: Produce code for a GT_COPY node.
+//
+// Arguments:
+//    tree - the GT_COPY node
+//
+// Notes:
+//    This will copy the register(s) produced by this nodes source, to
+//    the register(s) allocated to this GT_COPY node.
+//    It has some special handling for these casess:
+//    - when the source and target registers are in different register files
+//      (note that this is *not* a conversion).
+//    - when the source is a lclVar whose home location is being moved to a new
+//      register (rather than just being copied for temporary use).
+//
 void CodeGen::genRegCopy(GenTree* treeNode)
 {
     assert(treeNode->OperGet() == GT_COPY);
     GenTree* op1 = treeNode->gtOp.gtOp1;
 
-    if (op1->IsMultiRegCall())
+    if (op1->IsMultiRegNode())
     {
         genConsumeReg(op1);
 
-        GenTreeCopyOrReload* copyTree    = treeNode->AsCopyOrReload();
-        GenTreeCall*         call        = op1->AsCall();
-        ReturnTypeDesc*      retTypeDesc = call->GetReturnTypeDesc();
-        unsigned             regCount    = retTypeDesc->GetReturnRegCount();
+        GenTreeCopyOrReload* copyTree = treeNode->AsCopyOrReload();
+        unsigned             regCount = treeNode->GetMultiRegCount();
 
         for (unsigned i = 0; i < regCount; ++i)
         {
-            var_types type    = retTypeDesc->GetReturnRegType(i);
-            regNumber fromReg = call->GetRegNumByIdx(i);
+            var_types type    = op1->GetRegTypeByIndex(i);
+            regNumber fromReg = op1->GetRegByIndex(i);
             regNumber toReg   = copyTree->GetRegNumByIdx(i);
 
-            // A Multi-reg GT_COPY node will have valid reg only for those
-            // positions that corresponding result reg of call node needs
-            // to be copied.
+            // A Multi-reg GT_COPY node will have a valid reg only for those positions for which a corresponding
+            // result reg of the multi-reg node needs to be copied.
             if (toReg != REG_NA)
             {
                 assert(toReg != fromReg);
@@ -8620,61 +8632,6 @@ void CodeGen::genEmitHelperCall(unsigned helper, int argSize, emitAttr retSize,
     regSet.verifyRegistersUsed(killMask);
 }
 
-#if !defined(_TARGET_64BIT_)
-//-----------------------------------------------------------------------------
-//
-// Code Generation for Long integers
-//
-//-----------------------------------------------------------------------------
-
-//------------------------------------------------------------------------
-// genStoreLongLclVar: Generate code to store a non-enregistered long lclVar
-//
-// Arguments:
-//    treeNode - A TYP_LONG lclVar node.
-//
-// Return Value:
-//    None.
-//
-// Assumptions:
-//    'treeNode' must be a TYP_LONG lclVar node for a lclVar that has NOT been promoted.
-//    Its operand must be a GT_LONG node.
-//
-void CodeGen::genStoreLongLclVar(GenTree* treeNode)
-{
-    emitter* emit = getEmitter();
-
-    GenTreeLclVarCommon* lclNode = treeNode->AsLclVarCommon();
-    unsigned             lclNum  = lclNode->gtLclNum;
-    LclVarDsc*           varDsc  = &(compiler->lvaTable[lclNum]);
-    assert(varDsc->TypeGet() == TYP_LONG);
-    assert(!varDsc->lvPromoted);
-    GenTree* op1 = treeNode->gtOp.gtOp1;
-    noway_assert(op1->OperGet() == GT_LONG || op1->OperGet() == GT_MUL_LONG);
-    genConsumeRegs(op1);
-
-    if (op1->OperGet() == GT_LONG)
-    {
-        GenTree* loVal = op1->gtGetOp1();
-        GenTree* hiVal = op1->gtGetOp2();
-
-        noway_assert((loVal->gtRegNum != REG_NA) && (hiVal->gtRegNum != REG_NA));
-
-        emit->emitIns_S_R(ins_Store(TYP_INT), EA_4BYTE, loVal->gtRegNum, lclNum, 0);
-        emit->emitIns_S_R(ins_Store(TYP_INT), EA_4BYTE, hiVal->gtRegNum, lclNum, genTypeSize(TYP_INT));
-    }
-    else if (op1->OperGet() == GT_MUL_LONG)
-    {
-        assert((op1->gtFlags & GTF_MUL_64RSLT) != 0);
-
-        // Stack store
-        getEmitter()->emitIns_S_R(ins_Store(TYP_INT), emitTypeSize(TYP_INT), REG_LNGRET_LO, lclNum, 0);
-        getEmitter()->emitIns_S_R(ins_Store(TYP_INT), emitTypeSize(TYP_INT), REG_LNGRET_HI, lclNum,
-                                  genTypeSize(TYP_INT));
-    }
-}
-#endif // !defined(_TARGET_64BIT_)
-
 /*****************************************************************************
 * Unit testing of the XArch emitter: generate a bunch of instructions into the prolog
 * (it's as good a place as any), then use COMPlus_JitLateDisasm=* to see if the late
index c0cdc0a..466d4f7 100644 (file)
@@ -747,18 +747,22 @@ int GenTree::GetRegisterDstCount() const
     {
         return (const_cast<GenTree*>(this))->AsPutArgSplit()->gtNumRegs;
     }
-    // A PUTARG_REG could be a MultiRegOp on ARM since we could move a double register to two int registers,
-    // either for all double parameters w/SoftFP or for varargs).
-    else
+#endif
+#if !defined(_TARGET_64BIT_)
+    else if (OperIsMultiRegOp())
     {
+        // A MultiRegOp is a GT_MUL_LONG, GT_PUTARG_REG, or GT_BITCAST.
+        // For the latter two (ARM-only), they only have multiple registers if they produce a long value
+        // (GT_MUL_LONG always produces a long value).
+        CLANG_FORMAT_COMMENT_ANCHOR;
 #ifdef _TARGET_ARM_
-        assert(OperIsMultiRegOp());
         return (TypeGet() == TYP_LONG) ? 2 : 1;
 #else
-        unreached();
-#endif // _TARGET_ARM_
+        assert(OperIs(GT_MUL_LONG));
+        return 2;
+#endif
     }
-#endif // FEATURE_ARG_SPLIT
+#endif
     assert(!"Unexpected multi-reg node");
     return 0;
 }
index 724bae6..5841e9e 100644 (file)
@@ -1213,13 +1213,18 @@ public:
 
     bool OperIsMultiRegOp() const
     {
+#if !defined(_TARGET_64BIT_)
+        if (OperIs(GT_MUL_LONG))
+        {
+            return true;
+        }
 #if defined(_TARGET_ARM_)
-        if ((gtOper == GT_MUL_LONG) || (gtOper == GT_PUTARG_REG) || (gtOper == GT_BITCAST))
+        if (OperIs(GT_PUTARG_REG, GT_BITCAST))
         {
             return true;
         }
-#endif
-
+#endif // _TARGET_ARM_
+#endif // _TARGET_64BIT_
         return false;
     }
 
@@ -3839,7 +3844,7 @@ struct GenTreeCmpXchg : public GenTree
 #endif
 };
 
-#if defined(_TARGET_ARM_)
+#if !defined(_TARGET_64BIT_)
 struct GenTreeMultiRegOp : public GenTreeOp
 {
     regNumber gtOtherReg;
@@ -3994,7 +3999,7 @@ struct GenTreeMultiRegOp : public GenTreeOp
     }
 #endif
 };
-#endif // defined(_TARGET_ARM_)
+#endif // !defined(_TARGET_64BIT_)
 
 struct GenTreeFptrVal : public GenTree
 {
@@ -5425,6 +5430,11 @@ struct GenTreePutArgSplit : public GenTreePutArgStk
 #endif // FEATURE_ARG_SPLIT
 
 // Represents GT_COPY or GT_RELOAD node
+//
+// As it turns out, these are only needed on targets that happen to have multi-reg returns.
+// However, they are actually needed on any target that has any multi-reg ops. It is just
+// coincidence that those are the same (and there isn't a FEATURE_MULTIREG_OPS).
+//
 struct GenTreeCopyOrReload : public GenTreeUnOp
 {
 #if FEATURE_MULTIREG_RET
@@ -6031,7 +6041,7 @@ inline bool GenTree::IsMultiRegNode() const
         return true;
     }
 
-#if defined(_TARGET_ARM_)
+#if !defined(_TARGET_64BIT_)
     if (OperIsMultiRegOp() || OperIsPutArgSplit() || (gtOper == GT_COPY))
     {
         return true;
@@ -6061,7 +6071,7 @@ inline unsigned GenTree::GetMultiRegCount()
         return AsPutArgSplit()->gtNumRegs;
     }
 #endif
-#if defined(_TARGET_ARM_)
+#if !defined(_TARGET_64BIT_)
     if (OperIsMultiRegOp())
     {
         return AsMultiRegOp()->GetRegCount();
@@ -6102,7 +6112,7 @@ inline regNumber GenTree::GetRegByIndex(int regIndex)
         return AsPutArgSplit()->GetRegNumByIdx(regIndex);
     }
 #endif
-#if defined(_TARGET_ARM_)
+#if !defined(_TARGET_64BIT_)
     if (OperIsMultiRegOp())
     {
         return AsMultiRegOp()->GetRegNumByIdx(regIndex);
@@ -6143,7 +6153,7 @@ inline var_types GenTree::GetRegTypeByIndex(int regIndex)
         return AsPutArgSplit()->GetRegType(regIndex);
     }
 #endif
-#if defined(_TARGET_ARM_)
+#if !defined(_TARGET_64BIT_)
     if (OperIsMultiRegOp())
     {
         return AsMultiRegOp()->GetRegType(regIndex);
index dd972b6..9fd1275 100644 (file)
@@ -181,9 +181,7 @@ GTNODE(SUB_HI           , GenTreeOp          ,0,GTK_BINOP)
 // with long results are morphed into helper calls. It is similar to GT_MULHI,
 // the difference being that GT_MULHI drops the lo part of the result, whereas
 // GT_MUL_LONG keeps both parts of the result.
-#if defined(_TARGET_X86_)
-GTNODE(MUL_LONG         , GenTreeOp          ,1,GTK_BINOP)
-#elif defined (_TARGET_ARM_)
+#if !defined(_TARGET_64BIT_)
 GTNODE(MUL_LONG         , GenTreeMultiRegOp  ,1,GTK_BINOP)
 #endif
 
index bc64a34..2eac435 100644 (file)
@@ -121,7 +121,9 @@ GTSTRUCT_1(HWIntrinsic , GT_HWIntrinsic)
 GTSTRUCT_1(AllocObj    , GT_ALLOCOBJ)
 GTSTRUCT_1(RuntimeLookup, GT_RUNTIMELOOKUP)
 GTSTRUCT_2(CC          , GT_JCC, GT_SETCC)
-#if defined(_TARGET_ARM_)
+#if defined(_TARGET_X86_)
+GTSTRUCT_1(MultiRegOp  , GT_MUL_LONG)
+#elif defined (_TARGET_ARM_)
 GTSTRUCT_3(MultiRegOp  , GT_MUL_LONG, GT_PUTARG_REG, GT_BITCAST)
 #endif
 /*****************************************************************************/
index 7a86f33..02bfc2a 100644 (file)
@@ -130,21 +130,23 @@ void lsraAssignRegToTree(GenTree* tree, regNumber reg, unsigned regIdx)
     {
         tree->gtRegNum = reg;
     }
-#if FEATURE_ARG_SPLIT
-#ifdef _TARGET_ARM_
+#if !defined(_TARGET_64BIT_)
     else if (tree->OperIsMultiRegOp())
     {
         assert(regIdx == 1);
         GenTreeMultiRegOp* mul = tree->AsMultiRegOp();
         mul->gtOtherReg        = reg;
     }
-#endif // _TARGET_ARM_
+#endif // _TARGET_64BIT_
+#if FEATURE_MULTIREG_RET
     else if (tree->OperGet() == GT_COPY)
     {
         assert(regIdx == 1);
         GenTreeCopyOrReload* copy = tree->AsCopyOrReload();
         copy->gtOtherRegs[0]      = (regNumberSmall)reg;
     }
+#endif // FEATURE_MULTIREG_RET
+#if FEATURE_ARG_SPLIT
     else if (tree->OperIsPutArgSplit())
     {
         GenTreePutArgSplit* putArg = tree->AsPutArgSplit();
@@ -6176,11 +6178,10 @@ void LinearScan::insertCopyOrReload(BasicBlock* block, GenTree* tree, unsigned m
 #endif
     }
 
-    // If the parent is a reload/copy node, then tree must be a multi-reg call node
-    // that has already had one of its registers spilled. This is because multi-reg
-    // call node is the only node whose RefTypeDef positions get independently
-    // spilled or reloaded.  It is possible that one of its RefTypeDef position got
-    // spilled and the next use of it requires it to be in a different register.
+    // If the parent is a reload/copy node, then tree must be a multi-reg node
+    // that has already had one of its registers spilled.
+    // It is possible that one of its RefTypeDef positions got spilled and the next
+    // use of it requires it to be in a different register.
     //
     // In this case set the i'th position reg of reload/copy node to the reg allocated
     // for copy/reload refPosition.  Essentially a copy/reload node will have a reg
@@ -6190,8 +6191,7 @@ void LinearScan::insertCopyOrReload(BasicBlock* block, GenTree* tree, unsigned m
     if (parent->IsCopyOrReload())
     {
         noway_assert(parent->OperGet() == oper);
-        noway_assert(tree->IsMultiRegCall());
-        GenTreeCall*         call         = tree->AsCall();
+        noway_assert(tree->IsMultiRegNode());
         GenTreeCopyOrReload* copyOrReload = parent->AsCopyOrReload();
         noway_assert(copyOrReload->GetRegNumByIdx(multiRegIdx) == REG_NA);
         copyOrReload->SetRegNumByIdx(refPosition->assignedReg(), multiRegIdx);
@@ -8762,8 +8762,24 @@ void LinearScan::lsraGetOperandString(GenTree*          tree,
             }
             else
             {
-                _snprintf_s(operandString, operandStringLength, operandStringLength, "%s%s",
-                            getRegName(tree->gtRegNum, useFloatReg(tree->TypeGet())), lastUseChar);
+                regNumber reg       = tree->gtRegNum;
+                int       charCount = _snprintf_s(operandString, operandStringLength, operandStringLength, "%s%s",
+                                            getRegName(reg, genIsValidFloatReg(reg)), lastUseChar);
+                operandString += charCount;
+                operandStringLength -= charCount;
+
+                if (tree->IsMultiRegNode())
+                {
+                    unsigned regCount = tree->GetMultiRegCount();
+                    for (unsigned regIndex = 1; regIndex < regCount; regIndex++)
+                    {
+                        regNumber reg = tree->GetRegByIndex(regIndex);
+                        charCount     = _snprintf_s(operandString, operandStringLength, operandStringLength, ",%s%s",
+                                                getRegName(reg, genIsValidFloatReg(reg)), lastUseChar);
+                        operandString += charCount;
+                        operandStringLength -= charCount;
+                    }
+                }
             }
         }
         break;
@@ -8893,18 +8909,13 @@ void LinearScan::DumpOperandDefs(
     if (dstCount != 0)
     {
         // This operand directly produces registers; print it.
-        for (int i = 0; i < dstCount; i++)
+        if (!first)
         {
-            if (!first)
-            {
-                printf(",");
-            }
-
-            lsraGetOperandString(operand, mode, operandString, operandStringLength);
-            printf("%s", operandString);
-
-            first = false;
+            printf(",");
         }
+        lsraGetOperandString(operand, mode, operandString, operandStringLength);
+        printf("%s", operandString);
+        first = false;
     }
     else if (operand->isContained())
     {
index 706bc0c..8e97492 100644 (file)
@@ -2826,18 +2826,9 @@ int LinearScan::BuildStoreLoc(GenTreeLclVarCommon* storeLoc)
     {
         if (op1->OperIs(GT_MUL_LONG))
         {
-#ifdef _TARGET_X86_
-            // This is actually a bug. A GT_MUL_LONG produces two registers, but is modeled as only producing
-            // eax (and killing edx). This only works because it always occurs as var = GT_MUL_LONG (ensured by
-            // DecomposeMul), and therefore edx won't be reused before the store.
-            // TODO-X86-Cleanup: GT_MUL_LONG should be a multireg node on x86, just as on ARM.
-            srcCount     = 1;
-            singleUseRef = BuildUse(op1);
-#else
             srcCount = 2;
             BuildUse(op1, allRegs(TYP_INT), 0);
             BuildUse(op1, allRegs(TYP_INT), 1);
-#endif
         }
         else
         {
index aeb86e5..56e3cc7 100644 (file)
@@ -327,11 +327,13 @@ int LinearScan::BuildNode(GenTree* tree)
             srcCount = BuildModDiv(tree->AsOp());
             break;
 
-        case GT_MUL:
-        case GT_MULHI:
 #if defined(_TARGET_X86_)
         case GT_MUL_LONG:
+            dstCount = 2;
+            __fallthrough;
 #endif
+        case GT_MUL:
+        case GT_MULHI:
             srcCount = BuildMul(tree->AsOp());
             break;
 
@@ -682,8 +684,9 @@ int LinearScan::BuildNode(GenTree* tree)
 
     } // end switch (tree->OperGet())
 
-    // We need to be sure that we've set srcCount and dstCount appropriately
-    assert((dstCount < 2) || (tree->IsMultiRegCall() && dstCount == MAX_RET_REG_COUNT));
+    // We need to be sure that we've set srcCount and dstCount appropriately.
+    // Not that for XARCH, the maximum number of registers defined is 2.
+    assert((dstCount < 2) || ((dstCount == 2) && tree->IsMultiRegNode()));
     assert(isLocalDefUse == (tree->IsValue() && tree->IsUnusedValue()));
     assert(!tree->IsUnusedValue() || (dstCount != 0));
     assert(dstCount == tree->GetRegisterDstCount());
@@ -2821,6 +2824,7 @@ int LinearScan::BuildMul(GenTree* tree)
     }
 
     int       srcCount      = BuildBinaryUses(tree->AsOp());
+    int       dstCount      = 1;
     regMaskTP dstCandidates = RBM_NONE;
 
     bool isUnsignedMultiply    = ((tree->gtFlags & GTF_UNSIGNED) != 0);
@@ -2862,7 +2866,8 @@ int LinearScan::BuildMul(GenTree* tree)
     else if (tree->OperGet() == GT_MUL_LONG)
     {
         // have to use the encoding:RDX:RAX = RAX * rm
-        dstCandidates = RBM_RAX;
+        dstCandidates = RBM_RAX | RBM_RDX;
+        dstCount      = 2;
     }
 #endif
     GenTree* containedMemOp = nullptr;
@@ -2876,7 +2881,7 @@ int LinearScan::BuildMul(GenTree* tree)
         containedMemOp = op2;
     }
     regMaskTP killMask = getKillSetForMul(tree->AsOp());
-    BuildDefsWithKills(tree, 1, dstCandidates, killMask);
+    BuildDefsWithKills(tree, dstCount, dstCandidates, killMask);
     return srcCount;
 }
 
index a1b00ba..4a60035 100644 (file)
@@ -94619,3 +94619,11 @@ Expected=0
 MaxAllowedDurationSeconds=600
 Categories=EXPECTED_PASS
 HostStyle=0
+
+[GitHub_19397.cmd_11916]
+RelativePath=JIT\Regression\JitBlue\GitHub_19397\GitHub_19397\GitHub_19397.cmd
+WorkingDir=JIT\Regression\JitBlue\GitHub_19397\GitHub_19397
+Expected=0
+MaxAllowedDurationSeconds=600
+Categories=EXPECTED_PASS
+HostStyle=0
index ffbdf2c..76012ae 100644 (file)
@@ -94635,3 +94635,11 @@ Expected=0
 MaxAllowedDurationSeconds=600
 Categories=EXPECTED_PASS
 HostStyle=0
+
+[GitHub_19397.cmd_12235]
+RelativePath=JIT\Regression\JitBlue\GitHub_19397\GitHub_19397\GitHub_19397.cmd
+WorkingDir=JIT\Regression\JitBlue\GitHub_19397\GitHub_19397
+Expected=0
+MaxAllowedDurationSeconds=600
+Categories=EXPECTED_PASS
+HostStyle=0
diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_19397/GitHub_19397.cs b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_19397/GitHub_19397.cs
new file mode 100644 (file)
index 0000000..c8ef401
--- /dev/null
@@ -0,0 +1,37 @@
+// 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;
+
+// This is a regression test for a bug that was exposed with jitStressRegs=8 in
+// System.Text.ConsoleEncoding::GetMaxByteCount, due to the lack of handling
+// of a register-allocator-added COPY of a multi-reg multiply.
+
+namespace GitHub_19397
+{
+    public class Program
+    {
+        [MethodImplAttribute(MethodImplOptions.NoInlining)]
+        public static long getValue()
+        {
+            return(0x0101010101010101L);
+        }
+        static int Main()
+        {
+            long value = getValue();
+            Console.WriteLine($"Result is {value}");
+            if (value == 0x0101010101010101L)
+            {
+                Console.WriteLine("PASS");
+                return 100;
+            }
+            else
+            {
+                Console.WriteLine("FAIL");
+                return -1;
+            }
+        }
+
+    }
+}
diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_19397/GitHub_19397.csproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_19397/GitHub_19397.csproj
new file mode 100644 (file)
index 0000000..fe27f53
--- /dev/null
@@ -0,0 +1,38 @@
+<?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)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{76E69AA0-8C5A-4F76-8561-B8089FFA8D79}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+  </PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <PropertyGroup>
+    <DebugType></DebugType>
+    <Optimize>True</Optimize>
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>