Merge pull request #3108 from lioncash/mulx

OpcodeDispatcher: Improve output of MULX

FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp

@@ -2711,15 +2711,29 @@ void OpDispatchBuilder::RORX(OpcodeArgs) {
 void OpDispatchBuilder::MULX(OpcodeArgs) {
   // RDX is the implied source operand in the instruction
   const auto OperandSize = GetSrcSize(Op);
+  const auto OpSize = IR::SizeToOpSize(OperandSize);
 
-  OrderedNode* Src1 = LoadSource(GPRClass, Op, Op->Src[1], Op->Flags, -1);
-  OrderedNode* Src2 = LoadGPRRegister(X86State::REG_RDX, OperandSize);
+  // Src1 can be a memory operand, so ensure we constrain to the
+  // absolute width of the access in that scenario.
+  const auto GPRSize = CTX->GetGPRSize();
+  const auto Src1Size = Op->Src[1].IsGPR() ? GPRSize : OperandSize;
 
-  OrderedNode* ResultLo = _UMul(IR::SizeToOpSize(OperandSize), Src1, Src2);
-  OrderedNode* ResultHi = _UMulH(IR::SizeToOpSize(OperandSize), Src1, Src2);
+  OrderedNode* Src1 = LoadSource_WithOpSize(GPRClass, Op, Op->Src[1], Src1Size, Op->Flags, -1);
+  OrderedNode* Src2 = LoadGPRRegister(X86State::REG_RDX, GPRSize);
 
-  StoreResult(GPRClass, Op, Op->Src[0], ResultLo, -1);
-  StoreResult(GPRClass, Op, Op->Dest, ResultHi, -1);
+  // As per the Intel Software Development Manual, if the destination and
+  // first operand correspond to the same register, then the result
+  // will be the high half of the multiplication result.
+  if (Op->Dest.Data.GPR.GPR == Op->Src[0].Data.GPR.GPR) {
+    OrderedNode* ResultHi = _UMulH(OpSize, Src1, Src2);
+    StoreResult(GPRClass, Op, Op->Dest, ResultHi, -1);
+  } else {
+    OrderedNode* ResultLo = _UMul(OpSize, Src1, Src2);
+    OrderedNode* ResultHi = _UMulH(OpSize, Src1, Src2);
+
+    StoreResult(GPRClass, Op, Op->Src[0], ResultLo, -1);
+    StoreResult(GPRClass, Op, Op->Dest, ResultHi, -1);
+  }
 }
 
 void OpDispatchBuilder::PDEP(OpcodeArgs) {

unittests/InstructionCountCI/VEX_map2.json

@@ -3904,6 +3904,34 @@
       ]
     },
     "mulx eax, ebx, ecx": {
+      "ExpectedInstructionCount": 5,
+      "Optimal": "No",
+      "Comment": [
+        "Map 2 0b11 0xf6 32-bit"
+      ],
+      "ExpectedArm64ASM": [
+        "mul w7, w5, w6",
+        "ubfx x0, x5, #0, #32",
+        "ubfx x1, x6, #0, #32",
+        "mul x4, x0, x1",
+        "lsr x4, x4, #32"
+      ]
+    },
+    "mulx eax, eax, ebx": {
+      "ExpectedInstructionCount": 4,
+      "Optimal": "No",
+      "Comment": [
+        "Same two destinations should only compute high part",
+        "Map 2 0b11 0xf6 32-bit"
+      ],
+      "ExpectedArm64ASM": [
+        "ubfx x0, x7, #0, #32",
+        "ubfx x1, x6, #0, #32",
+        "mul x4, x0, x1",
+        "lsr x4, x4, #32"
+      ]
+    },
+    "mulx eax, ebx, [ecx]": {
       "ExpectedInstructionCount": 7,
       "Optimal": "No",
       "Comment": [
@@ -3911,10 +3939,10 @@
       ],
       "ExpectedArm64ASM": [
         "mov w20, w5",
-        "mov w21, w6",
-        "mul w7, w20, w21",
+        "ldr w20, [x20]",
+        "mul w7, w20, w6",
         "ubfx x0, x20, #0, #32",
-        "ubfx x1, x21, #0, #32",
+        "ubfx x1, x6, #0, #32",
         "mul x4, x0, x1",
         "lsr x4, x4, #32"
       ]
@@ -3930,6 +3958,29 @@
         "umulh x4, x5, x6"
       ]
     },
+    "mulx rax, rax, rbx": {
+      "ExpectedInstructionCount": 1,
+      "Optimal": "Yes",
+      "Comment": [
+        "Same two destinations should only compute high part",
+        "Map 2 0b11 0xf6 64-bit"
+      ],
+      "ExpectedArm64ASM": [
+        "umulh x4, x7, x6"
+      ]
+    },
+    "mulx rax, rbx, [rcx]": {
+      "ExpectedInstructionCount": 3,
+      "Optimal": "Yes",
+      "Comment": [
+        "Map 2 0b11 0xf6 64-bit"
+      ],
+      "ExpectedArm64ASM": [
+        "ldr x20, [x5]",
+        "mul x7, x20, x6",
+        "umulh x4, x20, x6"
+      ]
+    },
     "bextr eax, ebx, ecx": {
       "ExpectedInstructionCount": 19,
       "Optimal": "No",