Merge pull request #4655 from alyssarosenzweig/bug/divisor-mask

Fix divisor masking

FEXCore/Source/Interface/Core/JIT/ALUOps.cpp

@@ -969,11 +969,15 @@ DEF_OP(UDiv) {
     break;
   }
   case IR::OpSize::i32Bit: {
+    // We need to mask divisor if we have Upper bits, since the frontend does
+    // not on the hope that we can optimize to use the path above.
+    mov(ARMEmitter::Size::i32Bit, TMP2, Divisor);
+
     // TODO: 32-bit operation should be guaranteed not to leave garbage in the upper bits.
     mov(EmitSize, TMP1, Lower);
     bfi(EmitSize, TMP1, Upper, 32, 32);
-    udiv(EmitSize, Quotient, TMP1, Divisor);
-    msub(EmitSize, Remainder, Quotient, Divisor, TMP1);
+    udiv(EmitSize, Quotient, TMP1, TMP2);
+    msub(EmitSize, Remainder, Quotient, TMP2, TMP1);
     break;
   }
   case IR::OpSize::i64Bit: {

unittests/ASM/FEX_bugs/Divide32.asm

@@ -0,0 +1,20 @@
+%ifdef CONFIG
+{
+  "RegData": {
+    "RAX": "0x80000001",
+    "RDX": "0x1"
+  }
+}
+%endif
+
+; FEX had a bug where we failed to ignore garbage upper bits of a 32-bit divisor
+; with div. This test does a division with garbage in the upper bits where the
+; result would differ if they were not ignored.
+
+; 0x100000003 / 0x2 = 0x80000001 remainder 1
+mov edx, 1
+mov eax, 3
+mov rcx, 0xdeadbeef00000002
+div ecx
+
+hlt

unittests/InstructionCountCI/PrimaryGroup.json

@@ -2766,13 +2766,14 @@
       ]
     },
     "div ebx": {
-      "ExpectedInstructionCount": 6,
+      "ExpectedInstructionCount": 7,
       "Comment": "GROUP2 0xf7 /6",
       "ExpectedArm64ASM": [
+        "mov w1, w6",
         "mov x0, x4",
         "bfi x0, x5, #32, #32",
-        "udiv x20, x0, x6",
-        "msub x22, x20, x6, x0",
+        "udiv x20, x0, x1",
+        "msub x22, x20, x1, x0",
         "mov w4, w20",
         "mov w5, w22"
       ]