Merge "x86_64: Fix the rex prefix for movzxb, movsxb, movb"

3 files changed, 102 insertions, 25 deletions

diff --git a/compiler/utils/assembler_test.h b/compiler/utils/assembler_test.h
index b13edb68bf..3fe1a31d70 100644
--- a/compiler/utils/assembler_test.h
+++ b/compiler/utils/assembler_test.h
@@ -44,7 +44,9 @@ static std::string tmpnam_;
 
 enum class RegisterView {  // private
   kUsePrimaryName,
-  kUseSecondaryName
+  kUseSecondaryName,
+  kUseTertiaryName,
+  kUseQuaternaryName,
 };
 
 template<typename Ass, typename Reg, typename FPReg, typename Imm>
@@ -97,6 +99,15 @@ class AssemblerTest : public testing::Test {
         fmt);
   }
 
+  std::string Repeatrb(void (Ass::*f)(Reg, Reg), std::string fmt) {
+    return RepeatTemplatedRegisters<Reg, Reg>(f,
+        GetRegisters(),
+        GetRegisters(),
+        &AssemblerTest::GetRegName<RegisterView::kUseSecondaryName>,
+        &AssemblerTest::GetRegName<RegisterView::kUseQuaternaryName>,
+        fmt);
+  }
+
   std::string RepeatRr(void (Ass::*f)(Reg, Reg), std::string fmt) {
     return RepeatTemplatedRegisters<Reg, Reg>(f,
         GetRegisters(),
@@ -240,6 +251,18 @@ class AssemblerTest : public testing::Test {
     UNREACHABLE();
   }
 
+  // Tertiary register names are the tertiary view on registers, e.g., 16b on 64b systems.
+  virtual std::string GetTertiaryRegisterName(const Reg& reg ATTRIBUTE_UNUSED) {
+    UNIMPLEMENTED(FATAL) << "Architecture does not support tertiary registers";
+    UNREACHABLE();
+  }
+
+  // Quaternary register names are the quaternary view on registers, e.g., 8b on 64b systems.
+  virtual std::string GetQuaternaryRegisterName(const Reg& reg ATTRIBUTE_UNUSED) {
+    UNIMPLEMENTED(FATAL) << "Architecture does not support quaternary registers";
+    UNREACHABLE();
+  }
+
   std::string GetRegisterName(const Reg& reg) {
     return GetRegName<RegisterView::kUsePrimaryName>(reg);
   }
@@ -520,6 +543,14 @@ class AssemblerTest : public testing::Test {
       case RegisterView::kUseSecondaryName:
         sreg << GetSecondaryRegisterName(reg);
         break;
+
+      case RegisterView::kUseTertiaryName:
+        sreg << GetTertiaryRegisterName(reg);
+        break;
+
+      case RegisterView::kUseQuaternaryName:
+        sreg << GetQuaternaryRegisterName(reg);
+        break;
     }
     return sreg.str();
   }
diff --git a/compiler/utils/x86_64/assembler_x86_64.cc b/compiler/utils/x86_64/assembler_x86_64.cc
index 30e821860c..3ba926236f 100644
--- a/compiler/utils/x86_64/assembler_x86_64.cc
+++ b/compiler/utils/x86_64/assembler_x86_64.cc
@@ -209,7 +209,9 @@ void X86_64Assembler::movzxb(CpuRegister dst, CpuRegister src) {
 
 void X86_64Assembler::movzxb(CpuRegister dst, const Address& src) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
-  EmitOptionalByteRegNormalizingRex32(dst, src);
+  // Byte register is only in the source register form, so we don't use
+  // EmitOptionalByteRegNormalizingRex32(dst, src);
+  EmitOptionalRex32(dst, src);
   EmitUint8(0x0F);
   EmitUint8(0xB6);
   EmitOperand(dst.LowBits(), src);
@@ -227,7 +229,9 @@ void X86_64Assembler::movsxb(CpuRegister dst, CpuRegister src) {
 
 void X86_64Assembler::movsxb(CpuRegister dst, const Address& src) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
-  EmitOptionalByteRegNormalizingRex32(dst, src);
+  // Byte register is only in the source register form, so we don't use
+  // EmitOptionalByteRegNormalizingRex32(dst, src);
+  EmitOptionalRex32(dst, src);
   EmitUint8(0x0F);
   EmitUint8(0xBE);
   EmitOperand(dst.LowBits(), src);
@@ -2173,11 +2177,18 @@ void X86_64Assembler::EmitRex64(CpuRegister dst, const Operand& operand) {
 }
 
 void X86_64Assembler::EmitOptionalByteRegNormalizingRex32(CpuRegister dst, CpuRegister src) {
-  EmitOptionalRex(true, false, dst.NeedsRex(), false, src.NeedsRex());
+  // For src, SPL, BPL, SIL, DIL need the rex prefix.
+  bool force = src.AsRegister() > 3;
+  EmitOptionalRex(force, false, dst.NeedsRex(), false, src.NeedsRex());
 }
 
 void X86_64Assembler::EmitOptionalByteRegNormalizingRex32(CpuRegister dst, const Operand& operand) {
-  uint8_t rex = 0x40 | operand.rex();  // REX.0000
+  uint8_t rex = operand.rex();
+  // For dst, SPL, BPL, SIL, DIL need the rex prefix.
+  bool force = dst.AsRegister() > 3;
+  if (force) {
+    rex |= 0x40;  // REX.0000
+  }
   if (dst.NeedsRex()) {
     rex |= 0x44;  // REX.0R00
   }
diff --git a/compiler/utils/x86_64/assembler_x86_64_test.cc b/compiler/utils/x86_64/assembler_x86_64_test.cc
index 86bdf20d38..116190a832 100644
--- a/compiler/utils/x86_64/assembler_x86_64_test.cc
+++ b/compiler/utils/x86_64/assembler_x86_64_test.cc
@@ -174,6 +174,40 @@ class AssemblerX86_64Test : public AssemblerTest<x86_64::X86_64Assembler, x86_64
       secondary_register_names_.emplace(x86_64::CpuRegister(x86_64::R14), "r14d");
       secondary_register_names_.emplace(x86_64::CpuRegister(x86_64::R15), "r15d");
 
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::RAX), "ax");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::RBX), "bx");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::RCX), "cx");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::RDX), "dx");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::RBP), "bp");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::RSP), "sp");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::RSI), "si");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::RDI), "di");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::R8), "r8w");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::R9), "r9w");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::R10), "r10w");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::R11), "r11w");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::R12), "r12w");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::R13), "r13w");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::R14), "r14w");
+      tertiary_register_names_.emplace(x86_64::CpuRegister(x86_64::R15), "r15w");
+
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::RAX), "al");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::RBX), "bl");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::RCX), "cl");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::RDX), "dl");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::RBP), "bpl");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::RSP), "spl");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::RSI), "sil");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::RDI), "dil");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::R8), "r8b");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::R9), "r9b");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::R10), "r10b");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::R11), "r11b");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::R12), "r12b");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::R13), "r13b");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::R14), "r14b");
+      quaternary_register_names_.emplace(x86_64::CpuRegister(x86_64::R15), "r15b");
+
       fp_registers_.push_back(new x86_64::XmmRegister(x86_64::XMM0));
       fp_registers_.push_back(new x86_64::XmmRegister(x86_64::XMM1));
       fp_registers_.push_back(new x86_64::XmmRegister(x86_64::XMM2));
@@ -216,9 +250,21 @@ class AssemblerX86_64Test : public AssemblerTest<x86_64::X86_64Assembler, x86_64
     return secondary_register_names_[reg];
   }
 
+  std::string GetTertiaryRegisterName(const x86_64::CpuRegister& reg) OVERRIDE {
+    CHECK(tertiary_register_names_.find(reg) != tertiary_register_names_.end());
+    return tertiary_register_names_[reg];
+  }
+
+  std::string GetQuaternaryRegisterName(const x86_64::CpuRegister& reg) OVERRIDE {
+    CHECK(quaternary_register_names_.find(reg) != quaternary_register_names_.end());
+    return quaternary_register_names_[reg];
+  }
+
  private:
   std::vector<x86_64::CpuRegister*> registers_;
   std::map<x86_64::CpuRegister, std::string, X86_64CpuRegisterCompare> secondary_register_names_;
+  std::map<x86_64::CpuRegister, std::string, X86_64CpuRegisterCompare> tertiary_register_names_;
+  std::map<x86_64::CpuRegister, std::string, X86_64CpuRegisterCompare> quaternary_register_names_;
 
   std::vector<x86_64::XmmRegister*> fp_registers_;
 };
@@ -878,31 +924,12 @@ std::string setcc_test_fn(AssemblerX86_64Test::Base* assembler_test,
                                "l", "ge", "le" };
 
   std::vector<x86_64::CpuRegister*> registers = assembler_test->GetRegisters();
-
-  std::string byte_regs[16];
-  byte_regs[x86_64::RAX] = "al";
-  byte_regs[x86_64::RBX] = "bl";
-  byte_regs[x86_64::RCX] = "cl";
-  byte_regs[x86_64::RDX] = "dl";
-  byte_regs[x86_64::RBP] = "bpl";
-  byte_regs[x86_64::RSP] = "spl";
-  byte_regs[x86_64::RSI] = "sil";
-  byte_regs[x86_64::RDI] = "dil";
-  byte_regs[x86_64::R8] = "r8b";
-  byte_regs[x86_64::R9] = "r9b";
-  byte_regs[x86_64::R10] = "r10b";
-  byte_regs[x86_64::R11] = "r11b";
-  byte_regs[x86_64::R12] = "r12b";
-  byte_regs[x86_64::R13] = "r13b";
-  byte_regs[x86_64::R14] = "r14b";
-  byte_regs[x86_64::R15] = "r15b";
-
   std::ostringstream str;
 
   for (auto reg : registers) {
     for (size_t i = 0; i < 15; ++i) {
       assembler->setcc(static_cast<x86_64::Condition>(i), *reg);
-      str << "set" << suffixes[i] << " %" << byte_regs[reg->AsRegister()] << "\n";
+      str << "set" << suffixes[i] << " %" << assembler_test->GetQuaternaryRegisterName(*reg) << "\n";
     }
   }
 
@@ -1033,4 +1060,12 @@ TEST_F(AssemblerX86_64Test, DecreaseFrame) {
   DriverFn(&decreaseframe_test_fn, "DecreaseFrame");
 }
 
+TEST_F(AssemblerX86_64Test, MovzxbRegs) {
+  DriverStr(Repeatrb(&x86_64::X86_64Assembler::movzxb, "movzbl %{reg2}, %{reg1}"), "movzxb");
+}
+
+TEST_F(AssemblerX86_64Test, MovsxbRegs) {
+  DriverStr(Repeatrb(&x86_64::X86_64Assembler::movsxb, "movsbl %{reg2}, %{reg1}"), "movsxb");
+}
+
 }  // namespace art