x86: fix helper function that displaces address by a given amount.
Change the function from a non-const method to a static member to avoid
giving an impression that it modifies the old address.
Add a gtest that constructs addresses with some initial displacement and
checks that they are equal to addresses constructed in the same way only
without displacement, and then displaced.
The added gtest reveals errors in the displace function. Some of the
added test cases crashed on checks, others failed test assertions. This
shouldn't affect real world as the use of the helper function in the
compiler is limited to a few working cases.
Checks failed because some of the address constructors do not expect ESP
as base register. It is possible to construct such addresses with
another constructor like `x86::Address(x86::ESP, 0))`, but an attempt to
displace this address would try to reassemble it using a forbidden
constructor and hit the check.
Some of the failed test assertions were for Address::Absolute, such as:
Expected equality of these values:
x86::Address::displace(x86::Address::Absolute(0), 42)
Which is: 42(%ebp)
x86::Address::Absolute(42)
Which is: (%ebp)
Other failed test assertions were due to the fact that one and the same
address can be encoded in more than one way, e.g. 32-bit displacement
normally requires mod 11b, but can also be achieved with mod 00b if EBP
is used as r/m or base. Example of a test failure:
Expected equality of these values:
x86::Address::displace(x86::Address(EAX, TIMES_1, 42), -42)
Which is: 0(%ebp,%eax,1)
x86::Address(EAX, TIMES_1, 0)
Which is: 0(,%eax,1)
Bug: 65872996
Test: m test-art-host-gtest # changes covered by the new test
Test: art/test.py --host -r
Change-Id: I8dcdc968d7bd9da2c0a16ef0afeb13cf9a168359
diff --git a/compiler/utils/x86/assembler_x86_test.cc b/compiler/utils/x86/assembler_x86_test.cc
index 030479c..c6a181a 100644
--- a/compiler/utils/x86/assembler_x86_test.cc
+++ b/compiler/utils/x86/assembler_x86_test.cc
@@ -1324,4 +1324,47 @@
DriverStr(RepeatR(&x86::X86Assembler::notl, "notl %{reg}"), "notl");
}
+// Test that displacing an existing address is the same as constructing a new one with the same
+// initial displacement.
+TEST_F(AssemblerX86Test, AddressDisplaceBy) {
+ // Test different displacements, including some 8-bit and 32-bit ones, so that changing
+ // displacement may require a different addressing mode.
+ static const std::vector<int32_t> displacements = {0, 42, -42, 140, -140};
+ // Test with all scale factors.
+ static const std::vector<ScaleFactor> scales = {TIMES_1, TIMES_2, TIMES_4, TIMES_8};
+
+ for (int32_t disp0 : displacements) { // initial displacement
+ for (int32_t disp : displacements) { // extra displacement
+ for (const x86::Register *reg : GetRegisters()) {
+ // Test non-SIB addressing.
+ EXPECT_EQ(x86::Address::displace(x86::Address(*reg, disp0), disp),
+ x86::Address(*reg, disp0 + disp));
+
+ // Test SIB addressing with EBP base.
+ if (*reg != x86::ESP) {
+ for (ScaleFactor scale : scales) {
+ EXPECT_EQ(x86::Address::displace(x86::Address(*reg, scale, disp0), disp),
+ x86::Address(*reg, scale, disp0 + disp));
+ }
+ }
+
+ // Test SIB addressing with different base.
+ for (const x86::Register *index : GetRegisters()) {
+ if (*index == x86::ESP) {
+ continue; // Skip ESP as it cannot be used with this address constructor.
+ }
+ for (ScaleFactor scale : scales) {
+ EXPECT_EQ(x86::Address::displace(x86::Address(*reg, *index, scale, disp0), disp),
+ x86::Address(*reg, *index, scale, disp0 + disp));
+ }
+ }
+
+ // Test absolute addressing.
+ EXPECT_EQ(x86::Address::displace(x86::Address::Absolute(disp0), disp),
+ x86::Address::Absolute(disp0 + disp));
+ }
+ }
+ }
+}
+
} // namespace art