From d4f87c7db1cbd34896ce59783389c15bf9848af2 Mon Sep 17 00:00:00 2001
From: Lioncache <mai.iam2048@gmail.com>
Date: Fri, 15 Sep 2023 20:26:36 -0400
Subject: [PATCH] OpcodeDispatcher: Improve output of MULX

We can cut down on a few of the generated moves. For
the case where both destinations alias one another,
we can just calculate the high part instead of both of them.
---
 .../Interface/Core/OpcodeDispatcher.cpp       | 26 +++++++--
 unittests/InstructionCountCI/VEX_map2.json    | 57 ++++++++++++++++++-
 2 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp
index 11db106a4..79d3728c6 100644
--- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp
+++ b/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) {
diff --git a/unittests/InstructionCountCI/VEX_map2.json b/unittests/InstructionCountCI/VEX_map2.json
index c34827f9f..4e96e48cd 100644
--- a/unittests/InstructionCountCI/VEX_map2.json
+++ b/unittests/InstructionCountCI/VEX_map2.json
@@ -3916,6 +3916,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": [
@@ -3923,10 +3951,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"
       ]
@@ -3942,6 +3970,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",