x86_64: Allow test to skip register pairs. Fix XCHG and enable tests.
Previously tests were disabled for `xchgl reg, reg` variant with all
register pairs, although the problematic case is only `xchg eax, eax`.
This commit allows one to pass an exception list to a testing function
and skip the problematic register pairs instead of disabling the whole
test. The patch adds exception lists to all testing functions that run
over register pairs, although XCHG only needs it for `Repeatrr`.
Enabling the test revealed a few small errors in the XCHG implementation
(namely, source and destination registers were swapped, which does not
affect the result). This commit fixes the implementation so that the
tests pass.
Bug: 65872996
Test: m test-art-host-gtest
Change-Id: Iaa759861330bcfb30db1a8219b805cc479cc3280
diff --git a/compiler/utils/assembler_test.h b/compiler/utils/assembler_test.h
index 9fffda5..bb22fe5 100644
--- a/compiler/utils/assembler_test.h
+++ b/compiler/utils/assembler_test.h
@@ -88,13 +88,16 @@
fmt);
}
- std::string RepeatRR(void (Ass::*f)(Reg, Reg), const std::string& fmt) {
+ std::string RepeatRR(void (Ass::*f)(Reg, Reg),
+ const std::string& fmt,
+ const std::vector<std::pair<Reg, Reg>>* except = nullptr) {
return RepeatTemplatedRegisters<Reg, Reg>(f,
GetRegisters(),
GetRegisters(),
&AssemblerTest::GetRegName<RegisterView::kUsePrimaryName>,
&AssemblerTest::GetRegName<RegisterView::kUsePrimaryName>,
- fmt);
+ fmt,
+ except);
}
std::string RepeatRRNoDupes(void (Ass::*f)(Reg, Reg), const std::string& fmt) {
@@ -106,31 +109,40 @@
fmt);
}
- std::string Repeatrr(void (Ass::*f)(Reg, Reg), const std::string& fmt) {
+ std::string Repeatrr(void (Ass::*f)(Reg, Reg),
+ const std::string& fmt,
+ const std::vector<std::pair<Reg, Reg>>* except = nullptr) {
return RepeatTemplatedRegisters<Reg, Reg>(f,
GetRegisters(),
GetRegisters(),
&AssemblerTest::GetRegName<RegisterView::kUseSecondaryName>,
&AssemblerTest::GetRegName<RegisterView::kUseSecondaryName>,
- fmt);
+ fmt,
+ except);
}
- std::string Repeatww(void (Ass::*f)(Reg, Reg), const std::string& fmt) {
+ std::string Repeatww(void (Ass::*f)(Reg, Reg),
+ const std::string& fmt,
+ const std::vector<std::pair<Reg, Reg>>* except = nullptr) {
return RepeatTemplatedRegisters<Reg, Reg>(f,
GetRegisters(),
GetRegisters(),
&AssemblerTest::GetRegName<RegisterView::kUseTertiaryName>,
&AssemblerTest::GetRegName<RegisterView::kUseTertiaryName>,
- fmt);
+ fmt,
+ except);
}
- std::string Repeatbb(void (Ass::*f)(Reg, Reg), const std::string& fmt) {
+ std::string Repeatbb(void (Ass::*f)(Reg, Reg),
+ const std::string& fmt,
+ const std::vector<std::pair<Reg, Reg>>* except = nullptr) {
return RepeatTemplatedRegisters<Reg, Reg>(f,
GetRegisters(),
GetRegisters(),
&AssemblerTest::GetRegName<RegisterView::kUseQuaternaryName>,
&AssemblerTest::GetRegName<RegisterView::kUseQuaternaryName>,
- fmt);
+ fmt,
+ except);
}
std::string RepeatRRR(void (Ass::*f)(Reg, Reg, Reg), const std::string& fmt) {
@@ -144,22 +156,28 @@
fmt);
}
- std::string Repeatrb(void (Ass::*f)(Reg, Reg), const std::string& fmt) {
+ std::string Repeatrb(void (Ass::*f)(Reg, Reg),
+ const std::string& fmt,
+ const std::vector<std::pair<Reg, Reg>>* except = nullptr) {
return RepeatTemplatedRegisters<Reg, Reg>(f,
GetRegisters(),
GetRegisters(),
&AssemblerTest::GetRegName<RegisterView::kUseSecondaryName>,
&AssemblerTest::GetRegName<RegisterView::kUseQuaternaryName>,
- fmt);
+ fmt,
+ except);
}
- std::string RepeatRr(void (Ass::*f)(Reg, Reg), const std::string& fmt) {
+ std::string RepeatRr(void (Ass::*f)(Reg, Reg),
+ const std::string& fmt,
+ const std::vector<std::pair<Reg, Reg>>* except = nullptr) {
return RepeatTemplatedRegisters<Reg, Reg>(f,
GetRegisters(),
GetRegisters(),
&AssemblerTest::GetRegName<RegisterView::kUsePrimaryName>,
&AssemblerTest::GetRegName<RegisterView::kUseSecondaryName>,
- fmt);
+ fmt,
+ except);
}
std::string RepeatRI(void (Ass::*f)(Reg, const Imm&), size_t imm_bytes, const std::string& fmt) {
@@ -1266,12 +1284,21 @@
const std::vector<Reg2*> reg2_registers,
std::string (AssemblerTest::*GetName1)(const Reg1&),
std::string (AssemblerTest::*GetName2)(const Reg2&),
- const std::string& fmt) {
+ const std::string& fmt,
+ const std::vector<std::pair<Reg1, Reg2>>* except = nullptr) {
WarnOnCombinations(reg1_registers.size() * reg2_registers.size());
std::string str;
for (auto reg1 : reg1_registers) {
for (auto reg2 : reg2_registers) {
+ // Check if this register pair is on the exception list. If so, skip it.
+ if (except != nullptr) {
+ const auto& pair = std::make_pair(*reg1, *reg2);
+ if (std::find(except->begin(), except->end(), pair) != except->end()) {
+ continue;
+ }
+ }
+
if (f != nullptr) {
(assembler_.get()->*f)(*reg1, *reg2);
}
diff --git a/compiler/utils/x86_64/assembler_x86_64.cc b/compiler/utils/x86_64/assembler_x86_64.cc
index 61d7736..35c68b8 100644
--- a/compiler/utils/x86_64/assembler_x86_64.cc
+++ b/compiler/utils/x86_64/assembler_x86_64.cc
@@ -3826,9 +3826,9 @@
}
// General case.
- EmitOptionalRex32(src, dst);
+ EmitOptionalRex32(dst, src);
EmitUint8(0x87);
- EmitRegisterOperand(src.LowBits(), dst.LowBits());
+ EmitRegisterOperand(dst.LowBits(), src.LowBits());
}
@@ -3851,9 +3851,9 @@
}
// General case.
- EmitRex64(src, dst);
+ EmitRex64(dst, src);
EmitUint8(0x87);
- EmitRegisterOperand(src.LowBits(), dst.LowBits());
+ EmitRegisterOperand(dst.LowBits(), src.LowBits());
}
diff --git a/compiler/utils/x86_64/assembler_x86_64_test.cc b/compiler/utils/x86_64/assembler_x86_64_test.cc
index 7785511..562301c 100644
--- a/compiler/utils/x86_64/assembler_x86_64_test.cc
+++ b/compiler/utils/x86_64/assembler_x86_64_test.cc
@@ -952,13 +952,16 @@
}
TEST_F(AssemblerX86_64Test, Xchgq) {
- DriverStr(RepeatRR(&x86_64::X86_64Assembler::xchgq, "xchgq %{reg1}, %{reg2}"), "xchgq");
+ DriverStr(RepeatRR(&x86_64::X86_64Assembler::xchgq, "xchgq %{reg2}, %{reg1}"), "xchgq");
}
TEST_F(AssemblerX86_64Test, Xchgl) {
- // TODO: Test is disabled because GCC generates 0x87 0xC0 for xchgl eax, eax. All other cases
- // are the same. Anyone know why it doesn't emit a simple 0x90? It does so for xchgq rax, rax...
- // DriverStr(Repeatrr(&x86_64::X86_64Assembler::xchgl, "xchgl %{reg2}, %{reg1}"), "xchgl");
+ // Exclude `xcghl eax, eax` because the reference implementation generates 0x87 0xC0 (contrary to
+ // the intel manual saying that this should be a `nop` 0x90). All other cases are the same.
+ static const std::vector<std::pair<x86_64::CpuRegister, x86_64::CpuRegister>> except = {
+ std::make_pair(x86_64::CpuRegister(x86_64::RAX), x86_64::CpuRegister(x86_64::RAX))
+ };
+ DriverStr(Repeatrr(&x86_64::X86_64Assembler::xchgl, "xchgl %{reg2}, %{reg1}", &except), "xchgl");
}
TEST_F(AssemblerX86_64Test, Cmpxchgb) {
diff --git a/compiler/utils/x86_64/constants_x86_64.h b/compiler/utils/x86_64/constants_x86_64.h
index cd61e6d..301c8fc 100644
--- a/compiler/utils/x86_64/constants_x86_64.h
+++ b/compiler/utils/x86_64/constants_x86_64.h
@@ -35,6 +35,9 @@
constexpr Register AsRegister() const {
return reg_;
}
+ bool operator==(const CpuRegister& other) const {
+ return reg_ == other.reg_;
+ }
constexpr uint8_t LowBits() const {
return reg_ & 7;
}
@@ -59,7 +62,7 @@
constexpr bool NeedsRex() const {
return reg_ > 7;
}
- bool operator==(XmmRegister& other) {
+ bool operator==(const XmmRegister& other) const {
return reg_ == other.reg_;
}
private: