JIT: Emit mulx for GT_MULHI and GT_MUL_LONG if BMI2 is available

src/coreclr/jit/codegenxarch.cpp

@@ -822,38 +822,76 @@ void CodeGen::genCodeForMulHi(GenTreeOp* treeNode)
     // to get the high bits of the multiply, we are constrained to using the
     // 1-op form:  RDX:RAX = RAX * rm
     // The 3-op form (Rx=Ry*Rz) does not support it.
-
+    // When BMI2 is available, we can use the MULX instruction to get the high bits
     genConsumeOperands(treeNode->AsOp());
 
     GenTree* regOp = op1;
     GenTree* rmOp  = op2;
 
-    // Set rmOp to the memory operand (if any)
-    if (op1->isUsedFromMemory() || (op2->isUsedFromReg() && (op2->GetRegNum() == REG_RAX)))
+    // Lowering has ensured that op1 is never the memory operand to simplify checks here
+    assert(!op1->isUsedFromMemory());
+
+    if (treeNode->IsUnsigned() && compiler->compOpportunisticallyDependsOn(InstructionSet_BMI2))
     {
-        regOp = op2;
-        rmOp  = op1;
-    }
-    assert(regOp->isUsedFromReg());
+        if (op2->isUsedFromReg() && (op2->GetRegNum() == REG_RDX))
+        {
+            regOp = op2;
+            rmOp  = op1;
+        }
+        assert(regOp->isUsedFromReg());
 
-    // Setup targetReg when neither of the source operands was a matching register
-    inst_Mov(targetType, REG_RAX, regOp->GetRegNum(), /* canSkip */ true);
+        // Setup targetReg when neither of the source operands was a matching register
+        inst_Mov(targetType, REG_RDX, regOp->GetRegNum(), /* canSkip */ true);
 
-    instruction ins;
-    if ((treeNode->gtFlags & GTF_UNSIGNED) == 0)
-    {
-        ins = INS_imulEAX;
+        if (treeNode->OperGet() == GT_MULHI)
+        {
+            // emit MULX instruction, use targetReg twice to only store high result
+            inst_RV_RV_TT(INS_mulx, size, targetReg, targetReg, rmOp, /* isRMW */ false, INS_OPTS_NONE);
+        }
+        else
+        {
+#if TARGET_64BIT
+            assert(false);
+#else
+            assert(treeNode->OperGet() == GT_MUL_LONG);
+
+            // emit MULX instruction
+            regNumber hiReg  = targetReg;
+            regNumber lowReg = treeNode->AsMultiRegOp()->GetRegByIndex(1);
+            inst_RV_RV_TT(INS_mulx, size, hiReg, lowReg, rmOp, /* isRMW */ false, INS_OPTS_NONE);
+#endif
+        }
     }
-    else
+    else // Generate MUL or IMUL instruction
     {
-        ins = INS_mulEAX;
-    }
-    emit->emitInsBinary(ins, size, treeNode, rmOp);
+        // If op2 is already present in RAX use that as implicit operand
+        if (op2->isUsedFromReg() && (op2->GetRegNum() == REG_RAX))
+        {
+            regOp = op2;
+            rmOp  = op1;
+        }
+        assert(regOp->isUsedFromReg());
 
-    // Move the result to the desired register, if necessary
-    if (treeNode->OperGet() == GT_MULHI)
-    {
-        inst_Mov(targetType, targetReg, REG_RDX, /* canSkip */ true);
+        // Setup targetReg when neither of the source operands was a matching register
+        inst_Mov(targetType, REG_RAX, regOp->GetRegNum(), /* canSkip */ true);
+
+        instruction ins;
+        if (!treeNode->IsUnsigned())
+        {
+            ins = INS_imulEAX;
+        }
+        else
+        {
+            ins = INS_mulEAX;
+        }
+        emit->emitInsBinary(ins, size, treeNode, rmOp);
+
+        // Move the result to the desired register, if necessary
+        if (treeNode->OperGet() == GT_MULHI)
+        {
+            assert(targetReg == REG_RDX);
+            inst_Mov(targetType, targetReg, REG_RDX, /* canSkip */ true);
+        }
     }
 
     genProduceReg(treeNode);

src/coreclr/jit/lower.h

@@ -510,6 +510,13 @@ class Lowering final : public Phase
 
 #endif // TARGET_XARCH
 
+#if TARGET_X86
+            if (parentNode->OperIs(GT_MUL_LONG))
+            {
+                return genTypeSize(childNode->TypeGet()) == operatorSize / 2;
+            }
+#endif // TARGET_X86
+
             return genTypeSize(childNode->TypeGet()) == operatorSize;
         }
 

src/coreclr/jit/lowerxarch.cpp

@@ -7950,14 +7950,15 @@ void Lowering::ContainCheckMul(GenTreeOp* node)
     bool isSafeToContainOp1 = true;
     bool isSafeToContainOp2 = true;
 
-    bool     isUnsignedMultiply    = ((node->gtFlags & GTF_UNSIGNED) != 0);
+    bool     isUnsignedMultiply    = node->IsUnsigned();
     bool     requiresOverflowCheck = node->gtOverflowEx();
     bool     useLeaEncoding        = false;
     GenTree* memOp                 = nullptr;
 
     bool                 hasImpliedFirstOperand = false;
     GenTreeIntConCommon* imm                    = nullptr;
     GenTree*             other                  = nullptr;
+    var_types            nodeType               = node->TypeGet();
 
     // Multiply should never be using small types
     assert(!varTypeIsSmall(node->TypeGet()));
@@ -7977,6 +7978,16 @@ void Lowering::ContainCheckMul(GenTreeOp* node)
     else if (node->OperGet() == GT_MUL_LONG)
     {
         hasImpliedFirstOperand = true;
+        // GT_MUL_LONG produce ULONG or LONG but work on UINT or INT
+        if (nodeType == TYP_LONG)
+        {
+            nodeType = TYP_INT;
+        }
+        else
+        {
+            assert(nodeType == TYP_ULONG);
+            nodeType = TYP_UINT;
+        }
     }
 #endif
     else if (IsContainableImmed(node, op2) || IsContainableImmed(node, op1))
@@ -8013,7 +8024,7 @@ void Lowering::ContainCheckMul(GenTreeOp* node)
     //
     if (memOp == nullptr)
     {
-        if ((op2->TypeGet() == node->TypeGet()) && IsContainableMemoryOp(op2))
+        if ((op2->TypeGet() == nodeType) && IsContainableMemoryOp(op2))
         {
             isSafeToContainOp2 = IsSafeToContainMem(node, op2);
             if (isSafeToContainOp2)
@@ -8022,7 +8033,7 @@ void Lowering::ContainCheckMul(GenTreeOp* node)
             }
         }
 
-        if ((memOp == nullptr) && (op1->TypeGet() == node->TypeGet()) && IsContainableMemoryOp(op1))
+        if ((memOp == nullptr) && (op1->TypeGet() == nodeType) && IsContainableMemoryOp(op1))
         {
             isSafeToContainOp1 = IsSafeToContainMem(node, op1);
             if (isSafeToContainOp1)
@@ -8033,7 +8044,7 @@ void Lowering::ContainCheckMul(GenTreeOp* node)
     }
     else
     {
-        if ((memOp->TypeGet() != node->TypeGet()))
+        if ((memOp->TypeGet() != nodeType))
         {
             memOp = nullptr;
         }
@@ -8058,6 +8069,11 @@ void Lowering::ContainCheckMul(GenTreeOp* node)
         if (memOp != nullptr)
         {
             MakeSrcContained(node, memOp);
+            // Swap the operands so that the contained memory operand is always op2.
+            if (memOp == op1)
+            {
+                std::swap(node->gtOp1, node->gtOp2);
+            }
         }
         else
         {

src/coreclr/jit/lsrabuild.cpp

@@ -782,7 +782,20 @@ regMaskTP LinearScan::getKillSetForMul(GenTreeOp* mulNode)
     regMaskTP killMask = RBM_NONE;
 #ifdef TARGET_XARCH
     assert(mulNode->OperIsMul());
-    if (!mulNode->OperIs(GT_MUL) || (((mulNode->gtFlags & GTF_UNSIGNED) != 0) && mulNode->gtOverflowEx()))
+    if (!mulNode->OperIs(GT_MUL))
+    {
+        // If we can use the mulx instruction, we don't need to kill RAX
+        if (mulNode->IsUnsigned() && compiler->compOpportunisticallyDependsOn(InstructionSet_BMI2))
+        {
+            // For mulx we force one arg to RDX, but we do not modify it
+            // keep killMask set to RBM_NONE
+        }
+        else
+        {
+            killMask = RBM_RAX | RBM_RDX;
+        }
+    }
+    else if (mulNode->IsUnsigned() && mulNode->gtOverflowEx())
     {
         killMask = RBM_RAX | RBM_RDX;
     }

src/coreclr/jit/lsraxarch.cpp

@@ -795,6 +795,14 @@ bool LinearScan::isRMWRegOper(GenTree* tree)
             }
             return (!tree->gtGetOp2()->isContainedIntOrIImmed() && !tree->gtGetOp1()->isContainedIntOrIImmed());
         }
+#ifdef TARGET_X86
+        case GT_MUL_LONG:
+#endif
+        case GT_MULHI:
+        {
+            // MUL, IMUL are RMW but mulx is not (which is used for unsigned operands when BMI2 is availible)
+            return !(tree->IsUnsigned() && compiler->compOpportunisticallyDependsOn(InstructionSet_BMI2));
+        }
 
 #ifdef FEATURE_HW_INTRINSICS
         case GT_HWINTRINSIC:
@@ -3213,18 +3221,47 @@ int LinearScan::BuildMul(GenTree* tree)
         return BuildSimple(tree);
     }
 
-    // ToDo-APX : imul currently doesn't have rex2 support. So, cannot  use R16-R31.
-    int              srcCount      = BuildBinaryUses(tree->AsOp());
+    bool isUnsignedMultiply    = tree->IsUnsigned();
+    bool requiresOverflowCheck = tree->gtOverflowEx();
+    bool useMulx               = tree->OperGet() != GT_MUL && isUnsignedMultiply &&
+                   compiler->compOpportunisticallyDependsOn(InstructionSet_BMI2);
+
+    // ToDo-APX : imul currently doesn't have rex2 support. So, cannot use R16-R31.
+    int              srcCount      = 0;
     int              dstCount      = 1;
     SingleTypeRegSet dstCandidates = RBM_NONE;
 
-    bool isUnsignedMultiply    = ((tree->gtFlags & GTF_UNSIGNED) != 0);
-    bool requiresOverflowCheck = tree->gtOverflowEx();
+    // Lowering has ensured that op1 is never the memory operand
+    assert(!op1->isUsedFromMemory());
 
-    // There are three forms of x86 multiply:
+    // Start with building the uses, ensuring that one of the operands is in the implicit register (RAX or RDX)
+    // Place first operand in implicit register, unless:
+    //  * it is a memory address
+    // *  or the second operand is already in the register
+    if (useMulx)
+    {
+        // In lowering, we place any memory operand in op2 so we default to placing op1 in RDX
+        // By selecting RDX here we don't have to kill it
+        srcCount = BuildOperandUses(op1, SRBM_RDX);
+        srcCount += BuildOperandUses(op2, RBM_NONE);
+    }
+    else
+    {
+        SingleTypeRegSet srcCandidates1 = RBM_NONE;
+        // If op2 is memory then tell allocator to pass op1 in RAX
+        if (op2->isUsedFromMemory())
+        {
+            srcCandidates1 = SRBM_RAX;
+        }
+        srcCount = BuildRMWUses(tree, op1, op2, srcCandidates1, RBM_NONE);
+    }
+
+    // There are three forms of x86 multiply in base instruction set
     // one-op form:     RDX:RAX = RAX * r/m
     // two-op form:     reg *= r/m
     // three-op form:   reg = r/m * imm
+    // If the BMI2 instruction set is supported there is an additional unsigned multiply
+    // mulx             reg1:reg2 = RDX * reg3/m
 
     // This special widening 32x32->64 MUL is not used on x64
 #if defined(TARGET_X86)
@@ -3248,16 +3285,22 @@ int LinearScan::BuildMul(GenTree* tree)
     }
     else if (tree->OperGet() == GT_MULHI)
     {
-        // Have to use the encoding:RDX:RAX = RAX * rm. Since we only care about the
-        // upper 32 bits of the result set the destination candidate to REG_RDX.
-        dstCandidates = SRBM_RDX;
+        if (!useMulx)
+        {
+            // Have to use the encoding:RDX:RAX = RAX * rm. Since we only care about the
+            // upper 32 bits of the result set the destination candidate to REG_RDX.
+            dstCandidates = SRBM_RDX;
+        }
     }
 #if defined(TARGET_X86)
     else if (tree->OperGet() == GT_MUL_LONG)
     {
-        // have to use the encoding:RDX:RAX = RAX * rm
-        dstCandidates = SRBM_RAX | SRBM_RDX;
-        dstCount      = 2;
+        dstCount = 2;
+        if (!useMulx)
+        {
+            // We have to use the encoding:RDX:RAX = RAX * rm
+            dstCandidates = SRBM_RAX | SRBM_RDX;
+        }
     }
 #endif
     GenTree* containedMemOp = nullptr;