commit b50ceebb814f28a6ade94974d3e8614c8585760e
author Ulya Trafimovich <skvadrik@google.com> Mon Jul 12 14:20:16 2021 +0100
committer Ulyana Trafimovich <skvadrik@google.com> Mon Jul 12 16:24:37 2021 +0000
tree 8297534745f75989ddf6b6c124ccc623a8eee71f
parent 38837d44f47a14ccd07e9de3c26d3e40b9768d0d

x86: Add missing variants for XCHG and tests for this instruction.

Add XCHG variants when both operands are registers. Handle the special
case when at least one of the register operands is EAX/AX. Add tests for
the new variants and for other variants that previously weren't tested.

Bug: 65872996
Test: m test-art-host-gtest
Change-Id: I1e80d1b86f3d2e804b90c2a181e4eabded30d8ae

compiler/utils/x86/assembler_x86.cc

diff --git a/compiler/utils/x86/assembler_x86.cc b/compiler/utils/x86/assembler_x86.cc
index adc7bcc..b6708de 100644
--- a/compiler/utils/x86/assembler_x86.cc
+++ b/compiler/utils/x86/assembler_x86.cc
@@ -2887,9 +2887,22 @@
 }
 
 
-void X86Assembler::xchgb(Register reg, const Address& address) {
-  // For testing purpose
-  xchgb(static_cast<ByteRegister>(reg), address);
+bool X86Assembler::try_xchg_eax(Register dst, Register src) {
+  if (src != EAX && dst != EAX) {
+    return false;
+  }
+  if (dst == EAX) {
+    std::swap(src, dst);
+  }
+  EmitUint8(0x90 + dst);
+  return true;
+}
+
+
+void X86Assembler::xchgb(ByteRegister dst, ByteRegister src) {
+  AssemblerBuffer::EnsureCapacity ensured(&buffer_);
+  EmitUint8(0x86);
+  EmitRegisterOperand(dst, src);
 }
 
 
@@ -2900,6 +2913,29 @@
 }
 
 
+void X86Assembler::xchgb(Register dst, Register src) {
+  xchgb(static_cast<ByteRegister>(dst), static_cast<ByteRegister>(src));
+}
+
+
+void X86Assembler::xchgb(Register reg, const Address& address) {
+  xchgb(static_cast<ByteRegister>(reg), address);
+}
+
+
+void X86Assembler::xchgw(Register dst, Register src) {
+  AssemblerBuffer::EnsureCapacity ensured(&buffer_);
+  EmitOperandSizeOverride();
+  if (try_xchg_eax(dst, src)) {
+    // A short version for AX.
+    return;
+  }
+  // General case.
+  EmitUint8(0x87);
+  EmitRegisterOperand(dst, src);
+}
+
+
 void X86Assembler::xchgw(Register reg, const Address& address) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   EmitOperandSizeOverride();
@@ -2910,6 +2946,11 @@
 
 void X86Assembler::xchgl(Register dst, Register src) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
+  if (try_xchg_eax(dst, src)) {
+    // A short version for EAX.
+    return;
+  }
+  // General case.
   EmitUint8(0x87);
   EmitRegisterOperand(dst, src);
 }

compiler/utils/x86/assembler_x86.h

diff --git a/compiler/utils/x86/assembler_x86.h b/compiler/utils/x86/assembler_x86.h
index db92a80..f6e7fbc 100644
--- a/compiler/utils/x86/assembler_x86.h
+++ b/compiler/utils/x86/assembler_x86.h
@@ -736,8 +736,14 @@
   void fptan();
   void fprem();
 
+  void xchgb(ByteRegister dst, ByteRegister src);
   void xchgb(ByteRegister reg, const Address& address);
+
+  // Wrappers for `xchgb` that accept `Register` instead of `ByteRegister` (used for testing).
+  void xchgb(Register dst, Register src);
   void xchgb(Register reg, const Address& address);
+
+  void xchgw(Register dst, Register src);
   void xchgw(Register reg, const Address& address);
 
   void xchgl(Register dst, Register src);
@@ -1044,6 +1050,10 @@
   uint8_t EmitVexPrefixByteTwo(bool W,
                                int SET_VEX_L,
                                int SET_VEX_PP);
+
+  // Helper function to emit a shorter variant of XCHG for when at least one operand is EAX/AX.
+  bool try_xchg_eax(Register dst, Register src);
+
   ConstantArea constant_area_;
   bool has_AVX_;     // x86 256bit SIMD AVX.
   bool has_AVX2_;    // x86 256bit SIMD AVX 2.0.

compiler/utils/x86/assembler_x86_test.cc

diff --git a/compiler/utils/x86/assembler_x86_test.cc b/compiler/utils/x86/assembler_x86_test.cc
index b7b1d33..92ce788 100644
--- a/compiler/utils/x86/assembler_x86_test.cc
+++ b/compiler/utils/x86/assembler_x86_test.cc
@@ -331,19 +331,28 @@
   DriverStr(expected, "LoadLongConstant");
 }
 
-TEST_F(AssemblerX86Test, Xchgb) {
-  DriverStr(RepeatwA(&x86::X86Assembler::xchgb,
-                     "xchgb {mem}, %{reg}"), "xchgb");
+TEST_F(AssemblerX86Test, XchgbReg) {
+  DriverStr(Repeatww(&x86::X86Assembler::xchgb, "xchgb %{reg2}, %{reg1}"), "xchgb");
 }
 
-TEST_F(AssemblerX86Test, Xchgw) {
-  DriverStr(RepeatrA(&x86::X86Assembler::xchgw,
-                     "xchgw {mem}, %{reg}"), "xchgw");
+TEST_F(AssemblerX86Test, XchgbMem) {
+  DriverStr(RepeatwA(&x86::X86Assembler::xchgb, "xchgb {mem}, %{reg}"), "xchgb");
 }
 
-TEST_F(AssemblerX86Test, Xchgl) {
-  DriverStr(RepeatRA(&x86::X86Assembler::xchgl,
-                     "xchgl {mem}, %{reg}"), "xchgl");
+TEST_F(AssemblerX86Test, XchgwReg) {
+  DriverStr(Repeatrr(&x86::X86Assembler::xchgw, "xchgw %{reg2}, %{reg1}"), "xchgw");
+}
+
+TEST_F(AssemblerX86Test, XchgwMem) {
+  DriverStr(RepeatrA(&x86::X86Assembler::xchgw, "xchgw {mem}, %{reg}"), "xchgw");
+}
+
+TEST_F(AssemblerX86Test, XchglReg) {
+  DriverStr(RepeatRR(&x86::X86Assembler::xchgl, "xchgl %{reg2}, %{reg1}"), "xchgl");
+}
+
+TEST_F(AssemblerX86Test, XchglMem) {
+  DriverStr(RepeatRA(&x86::X86Assembler::xchgl, "xchgl {mem}, %{reg}"), "xchgl");
 }
 
 TEST_F(AssemblerX86Test, Cmpxchgb) {