Merge pull request #3317 from Sonicadvance1/fix_imul_flags2

OpcodeDispatcher: Fixes flags generation in imul
FEX-Emu · Dec 19, 2023 · 3d2cbc5 · 3d2cbc5
2 parents 81c85d7 + fe3df00
commit 3d2cbc5
Show file tree

Hide file tree

Showing 8 changed files with 130 additions and 8 deletions.
diff --git a/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp b/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp
@@ -308,6 +308,16 @@ DEF_OP(UMul) {
   mul(EmitSize, GetReg(Node), GetReg(Op->Src1.ID()), GetReg(Op->Src2.ID()));
 }
 
+DEF_OP(UMull) {
+  auto Op = IROp->C<IR::IROp_UMull>();
+  umull(GetReg(Node).X(), GetReg(Op->Src1.ID()).W(), GetReg(Op->Src2.ID()).W());
+}
+
+DEF_OP(SMull) {
+  auto Op = IROp->C<IR::IROp_SMull>();
+  smull(GetReg(Node).X(), GetReg(Op->Src1.ID()).W(), GetReg(Op->Src2.ID()).W());
+}
+
 DEF_OP(Div) {
   auto Op = IROp->C<IR::IROp_Div>();
 

diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp
@@ -2897,7 +2897,7 @@ void OpDispatchBuilder::IMUL1SrcOp(OpcodeArgs) {
       break;
     }
     case 4: {
-      ResultHigh = _Mul(OpSize::i64Bit, Src1, Src2);
+      ResultHigh = _SMull(Src1, Src2);
       ResultHigh = _Sbfe(OpSize::i64Bit, Size * 8, Size * 8, ResultHigh);
       // Flipped order to save a move
       Dest = _Mul(OpSize::i32Bit, Src1, Src2);
@@ -2935,7 +2935,7 @@ void OpDispatchBuilder::IMUL2SrcOp(OpcodeArgs) {
       break;
     }
     case 4: {
-      ResultHigh = _Mul(OpSize::i64Bit, Src1, Src2);
+      ResultHigh = _SMull(Src1, Src2);
       ResultHigh = _Sbfe(OpSize::i64Bit, Size * 8, Size * 8, ResultHigh);
       // Flipped order to save a move
       Dest = _Mul(OpSize::i32Bit, Src1, Src2);

diff --git a/FEXCore/Source/Interface/IR/IR.json b/FEXCore/Source/Interface/IR/IR.json
@@ -1151,6 +1151,18 @@
           "Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit"
         ]
       },
+      "GPR = UMull GPR:$Src1, GPR:$Src2": {
+        "Desc": ["Integer unsigned multiplication long",
+                 "Multiplies two 32-bit numbers, returning a 64-bit destination register."
+                ],
+        "DestSize": "FEXCore::IR::OpSize::i64Bit"
+      },
+      "GPR = SMull GPR:$Src1, GPR:$Src2": {
+        "Desc": ["Integer signed multiplication long",
+                 "Multiplies two 32-bit numbers, returning a 64-bit destination register."
+                ],
+        "DestSize": "FEXCore::IR::OpSize::i64Bit"
+      },
       "GPR = Div OpSize:#Size, GPR:$Src1, GPR:$Src2": {
         "Desc": ["Integer signed division"
                 ],

diff --git a/unittests/ASM/FEX_bugs/IMUL_garbagedata_negative.asm b/unittests/ASM/FEX_bugs/IMUL_garbagedata_negative.asm
@@ -0,0 +1,100 @@
+%ifdef CONFIG
+{
+  "RegData": {
+    "R15": "0x0000000000003fff"
+  }
+}
+%endif
+
+%macro cfmerge 0
+
+; Get CF
+lahf
+shr rax, 8
+and rax, 1
+
+; Merge in to results
+shl r15, 1
+or r15, rax
+
+%endmacro
+
+; FEX had a bug where imul flag calculation was incorrect.
+; CF and OF are set due to overflow.
+
+mov r15, 0
+mov rax, 0
+
+; Multiply starting value
+mov ebx, 0x6D
+
+; imul 1-src
+mov ebx, 0xaaaaaaab
+mov ecx, 0x6D
+imul cx, bx
+cfmerge
+
+mov ebx, 0xaaaaaaab
+mov ecx, 0x6D
+imul ecx, ebx
+cfmerge
+
+mov rbx, 0xaaaaaaaa_aaaaaaab
+mov rcx, 0x6D
+imul rcx, rbx
+cfmerge
+
+; imul 2-src 8-bit check
+mov ebx, 0xaaaaaaab
+imul cx, bx, 0x6D
+cfmerge
+
+mov ebx, 0xaaaaaaab
+imul ecx, ebx, 0x6D
+cfmerge
+
+mov rbx, 0xaaaaaaaa_aaaaaaab
+imul ecx, ebx, 0x6D
+cfmerge
+
+; imul 2-src 16-bit, 32-bit, 64-bit check
+mov rbx, 0xaaaaaaaa_aaaaaaab
+imul cx, bx, 0x600D
+cfmerge
+
+mov rbx, 0xaaaaaaaa_aaaaaaab
+imul ecx, ebx, 0x600D0000
+cfmerge
+
+
+mov rbx, 0xaaaaaaaa_aaaaaaab
+imul rcx, rbx, 0x600D0000
+cfmerge
+
+mov rbx, 0x0aaaaaaa_aaaaaaab
+imul rcx, rbx, -0x600D0000
+cfmerge
+
+
+; IMUL implicit dest
+mov rax, 0x0aaaaaaa_aaaaaaab
+mov ecx, 0x6D
+imul cl
+cfmerge
+
+mov rax, 0x0aaaaaaa_aaaaaaab
+mov ecx, 0x600D
+imul cx
+cfmerge
+
+mov rax, 0x0aaaaaaa_aaaaaaab
+mov ecx, 0x600D0000
+imul ecx
+cfmerge
+
+mov rax, 0x0aaaaaaa_aaaaaaab
+mov rcx, 0x600D0000_00000000
+imul rcx
+cfmerge
+
+hlt
diff --git a/unittests/InstructionCountCI/FlagM/Primary.json b/unittests/InstructionCountCI/FlagM/Primary.json
@@ -1861,7 +1861,7 @@
       "Comment": "0x69",
       "ExpectedArm64ASM": [
         "mov w20, #0x101",
-        "mul x21, x7, x20",
+        "smull x21, w7, w20",
         "asr x21, x21, #32",
         "mul w4, w7, w20",
         "sbfx x20, x4, #31, #1",
@@ -1900,7 +1900,7 @@
       "Comment": "0x6b",
       "ExpectedArm64ASM": [
         "mov w20, #0x3",
-        "mul x21, x7, x20",
+        "smull x21, w7, w20",
         "asr x21, x21, #32",
         "mul w4, w7, w20",
         "sbfx x20, x4, #31, #1",

diff --git a/unittests/InstructionCountCI/FlagM/Secondary.json b/unittests/InstructionCountCI/FlagM/Secondary.json
@@ -1006,7 +1006,7 @@
       "ExpectedInstructionCount": 6,
       "Comment": "0x0f 0xaf",
       "ExpectedArm64ASM": [
-        "mul x20, x4, x7",
+        "smull x20, w4, w7",
         "asr x20, x20, #32",
         "mul w4, w4, w7",
         "sbfx x21, x4, #31, #1",

diff --git a/unittests/InstructionCountCI/Primary.json b/unittests/InstructionCountCI/Primary.json
@@ -2091,7 +2091,7 @@
       "Comment": "0x69",
       "ExpectedArm64ASM": [
         "mov w20, #0x101",
-        "mul x21, x7, x20",
+        "smull x21, w7, w20",
         "asr x21, x21, #32",
         "mul w4, w7, w20",
         "sbfx x20, x4, #31, #1",
@@ -2154,7 +2154,7 @@
       "Comment": "0x6b",
       "ExpectedArm64ASM": [
         "mov w20, #0x3",
-        "mul x21, x7, x20",
+        "smull x21, w7, w20",
         "asr x21, x21, #32",
         "mul w4, w7, w20",
         "sbfx x20, x4, #31, #1",

diff --git a/unittests/InstructionCountCI/Secondary.json b/unittests/InstructionCountCI/Secondary.json
@@ -1867,7 +1867,7 @@
       "ExpectedInstructionCount": 6,
       "Comment": "0x0f 0xaf",
       "ExpectedArm64ASM": [
-        "mul x20, x4, x7",
+        "smull x20, w4, w7",
         "asr x20, x20, #32",
         "mul w4, w4, w7",
         "sbfx x21, x4, #31, #1",