Merge pull request #8556 from Sintendo/bestrest

x64Emitter: Avoid 8-bit displacement when possible

Source/Core/Common/x64Emitter.cpp

@@ -13,7 +13,6 @@
 
 namespace Gen
 {
-// TODO(ector): Add EAX special casing, for ever so slightly smaller code.
 struct NormalOpDef
 {
   u8 toRm8, toRm32, fromRm8, fromRm32, imm8, imm32, simm8, eaximm8, eaximm32, ext;
@@ -270,67 +269,45 @@ void OpArg::WriteRest(XEmitter* emit, int extraBytes, X64Reg _operandReg,
     return;
   }
 
-  if (scale == 0)
+  if (scale == SCALE_NONE)
   {
     // Oh, no memory, Just a reg.
     mod = 3;  // 11
   }
+  else if (scale >= SCALE_NOBASE_2 && scale <= SCALE_NOBASE_8)
+  {
+    SIB = true;
+    mod = 0;
+    _offsetOrBaseReg = 5;
+    // Always has 32-bit displacement
+  }
   else
   {
-    // Ah good, no scaling.
+    if (scale != SCALE_ATREG)
-    if (scale == SCALE_ATREG && !((_offsetOrBaseReg & 7) == 4 || (_offsetOrBaseReg & 7) == 5))
-    {
-      // Okay, we're good. No SIB necessary.
-      int ioff = (int)offset;
-      if (ioff == 0)
-      {
-        mod = 0;
-      }
-      else if (ioff < -128 || ioff > 127)
-      {
-        mod = 2;  // 32-bit displacement
-      }
-      else
-      {
-        mod = 1;  // 8-bit displacement
-      }
-    }
-    else if (scale >= SCALE_NOBASE_2 && scale <= SCALE_NOBASE_8)
     {
       SIB = true;
-      mod = 0;
+    }
-      _offsetOrBaseReg = 5;
+    else if ((_offsetOrBaseReg & 7) == 4)
+    {
+      // Special case for which SCALE_ATREG needs SIB
+      SIB = true;
+      ireg = _offsetOrBaseReg;
+    }
+
+    // Okay, we're fine. Just disp encoding.
+    // We need displacement. Which size?
+    int ioff = (int)(s64)offset;
+    if (ioff == 0 && (_offsetOrBaseReg & 7) != 5)
+    {
+      mod = 0;  // No displacement
+    }
+    else if (ioff >= -128 && ioff <= 127)
+    {
+      mod = 1;  // 8-bit displacement
     }
     else
     {
-      if ((_offsetOrBaseReg & 7) == 4)  // this would occupy the SIB encoding :(
+      mod = 2;  // 32-bit displacement
-      {
-        // So we have to fake it with SIB encoding :(
-        SIB = true;
-      }
-
-      if (scale >= SCALE_1 && scale < SCALE_ATREG)
-      {
-        SIB = true;
-      }
-
-      if (scale == SCALE_ATREG && ((_offsetOrBaseReg & 7) == 4))
-      {
-        SIB = true;
-        ireg = _offsetOrBaseReg;
-      }
-
-      // Okay, we're fine. Just disp encoding.
-      // We need displacement. Which size?
-      int ioff = (int)(s64)offset;
-      if (ioff < -128 || ioff > 127)
-      {
-        mod = 2;  // 32-bit displacement
-      }
-      else
-      {
-        mod = 1;  // 8-bit displacement
-      }
     }
   }
 

Source/Core/Common/x64Emitter.h

@@ -313,11 +313,6 @@ inline u32 PtrOffset(const void* ptr, const void* base = nullptr)
   return (u32)distance;
 }
 
-// usage: int a[]; ARRAY_OFFSET(a,10)
-#define ARRAY_OFFSET(array, index) ((u32)((u64) & (array)[index] - (u64) & (array)[0]))
-// usage: struct {int e;} s; STRUCT_OFFSET(s,e)
-#define STRUCT_OFFSET(str, elem) ((u32)((u64) & (str).elem - (u64) & (str)))
-
 struct FixupBranch
 {
   enum class Type

Source/UnitTests/Common/x64EmitterTest.cpp

@@ -576,6 +576,97 @@ TEST_F(x64EmitterTest, MOV64)
   }
 }
 
+TEST_F(x64EmitterTest, MOV_AtReg)
+{
+  for (const auto& src : reg64names)
+  {
+    std::string segment = src.reg == RSP || src.reg == RBP ? "ss" : "ds";
+
+    emitter->MOV(64, R(RAX), MatR(src.reg));
+    EXPECT_EQ(emitter->GetCodePtr(),
+              code_buffer + 3 + ((src.reg & 7) == RBP || (src.reg & 7) == RSP));
+    ExpectDisassembly("mov rax, qword ptr " + segment + ":[" + src.name + "]");
+  }
+}
+
+TEST_F(x64EmitterTest, MOV_RegSum)
+{
+  for (const auto& src2 : reg64names)
+  {
+    for (const auto& src1 : reg64names)
+    {
+      if (src2.reg == RSP)
+        continue;
+      std::string segment = src1.reg == RSP || src1.reg == RBP ? "ss" : "ds";
+
+      emitter->MOV(64, R(RAX), MRegSum(src1.reg, src2.reg));
+      EXPECT_EQ(emitter->GetCodePtr(), code_buffer + 4 + ((src1.reg & 7) == RBP));
+      ExpectDisassembly("mov rax, qword ptr " + segment + ":[" + src1.name + "+" + src2.name + "]");
+    }
+  }
+}
+
+TEST_F(x64EmitterTest, MOV_Disp)
+{
+  for (const auto& dest : reg64names)
+  {
+    for (const auto& src : reg64names)
+    {
+      std::string segment = src.reg == RSP || src.reg == RBP ? "ss" : "ds";
+
+      emitter->MOV(64, R(dest.reg), MDisp(src.reg, 42));
+      EXPECT_EQ(emitter->GetCodePtr(), code_buffer + 4 + ((src.reg & 7) == RSP));
+      ExpectDisassembly("mov " + dest.name + ", qword ptr " + segment + ":[" + src.name + "+42]");
+
+      emitter->MOV(64, R(dest.reg), MDisp(src.reg, 1000));
+      EXPECT_EQ(emitter->GetCodePtr(), code_buffer + 7 + ((src.reg & 7) == RSP));
+      ExpectDisassembly("mov " + dest.name + ", qword ptr " + segment + ":[" + src.name + "+1000]");
+    }
+  }
+}
+
+TEST_F(x64EmitterTest, MOV_Scaled)
+{
+  for (const auto& src : reg64names)
+  {
+    if (src.reg == RSP)
+      continue;
+
+    emitter->MOV(64, R(RAX), MScaled(src.reg, 2, 42));
+    EXPECT_EQ(emitter->GetCodePtr(), code_buffer + 8);
+    ExpectDisassembly("mov rax, qword ptr ds:[" + src.name + "*2+42]");
+  }
+}
+
+TEST_F(x64EmitterTest, MOV_Complex)
+{
+  for (const auto& src1 : reg64names)
+  {
+    std::string segment = src1.reg == RSP || src1.reg == RBP ? "ss" : "ds";
+
+    for (const auto& src2 : reg64names)
+    {
+      if (src2.reg == RSP)
+        continue;
+
+      emitter->MOV(64, R(RAX), MComplex(src1.reg, src2.reg, 4, 0));
+      EXPECT_EQ(emitter->GetCodePtr(), code_buffer + 4 + ((src1.reg & 7) == RBP));
+      ExpectDisassembly("mov rax, qword ptr " + segment + ":[" + src1.name + "+" + src2.name +
+                        "*4]");
+
+      emitter->MOV(64, R(RAX), MComplex(src1.reg, src2.reg, 4, 42));
+      EXPECT_EQ(emitter->GetCodePtr(), code_buffer + 5);
+      ExpectDisassembly("mov rax, qword ptr " + segment + ":[" + src1.name + "+" + src2.name +
+                        "*4+42]");
+
+      emitter->MOV(64, R(RAX), MComplex(src1.reg, src2.reg, 4, 1000));
+      EXPECT_EQ(emitter->GetCodePtr(), code_buffer + 8);
+      ExpectDisassembly("mov rax, qword ptr " + segment + ":[" + src1.name + "+" + src2.name +
+                        "*4+1000]");
+    }
+  }
+}
+
 // TODO: Disassembler inverts operands here.
 // TWO_OP_ARITH_TEST(XCHG)
 // TWO_OP_ARITH_TEST(TEST)