kill rdx register for mulx instead of specifying as fixed register for use

Author: Daniel-Svensson

src/coreclr/jit/lsrabuild.cpp

@@ -787,8 +787,18 @@ regMaskTP LinearScan::getKillSetForMul(GenTreeOp* mulNode)
         // If we can use the mulx instruction, we don't need to kill RAX
         if (mulNode->IsUnsigned() && compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2))
         {
-            // For mulx we force one arg to RDX, but we do not modify it
-            // keep killMask set to RBM_NONE
+            // While we do not need to kill RDX, the register allocator
+            // seems to produce less spills if we kill it than if we specify it as fixed register
+            // for one of the operands.
+            // If second operand is used from memory, we define op1 as fixed rd register, so we don't need to kill it.
+            if (op2->isUsedFromMemory())
+            {
+                killMask = RBM_NONE;
+            }
+            else
+            {
+                killMask = RBM_RDX;
+            }
         }
         else
         {

src/coreclr/jit/lsraxarch.cpp

@@ -3233,7 +3233,7 @@ int LinearScan::BuildMul(GenTree* tree)
 
     bool isUnsignedMultiply    = tree->IsUnsigned();
     bool requiresOverflowCheck = tree->gtOverflowEx();
-    bool useMulx               = tree->OperGet() != GT_MUL && isUnsignedMultiply &&
+    bool useMulx               = !tree->OperIs(GT_MUL) && isUnsignedMultiply &&
                    compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2);
 
     // ToDo-APX : imul currently doesn't have rex2 support. So, cannot use R16-R31.
@@ -3250,15 +3250,18 @@ int LinearScan::BuildMul(GenTree* tree)
         // Lowering has ensured that op1 is never the memory operand
         assert(!op1->isUsedFromMemory());
 
-        // prefer to have the constant in RDX (op1) this is especially useful for MUL_HI usage
-        if (op2->IsCnsIntOrI())
+        SingleTypeRegSet srcCandidates1 = RBM_NONE;
+
+        // If one of the operands is a memory address, specify RDX for the other operand
+        if (op2->isUsedFromMemory())
         {
-            std::swap(op1, op2);
+            // If op2 is a memory operand, we place it in RDX
+            srcCandidates1 = SRBM_RDX;
         }
 
         // 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(op1, srcCandidates1);
         srcCount += BuildOperandUses(op2, RBM_NONE);
     }
     else