tests/JitX64: Improve thumb instruction test coverage

src/common/x64/abi.h

@@ -56,4 +56,4 @@
 
 #define ABI_ALL_CALLEE_SAVED (~ABI_ALL_CALLER_SAVED)
 
-#define ABI_RETURN RAX
+#define ABI_RETURN ::Gen::RAX

src/core/arm/decoder/thumb.cpp

@@ -224,7 +224,7 @@ static const std::array<Instruction, 27> thumb_instruction_table = { {
         // LDR Rd, [PC, #]
         Register Rd = bits<8, 10>(instruction);
         u32 imm8 = bits<0, 7>(instruction);
-        v->LDR_imm(0xE, /*P=*/1, /*U=*/1, /*W=*/0, 15, Rd, imm8 << 2);
+        v->LDR_imm(0xE, /*P=*/1, /*U=*/1, /*W=*/0, 15, Rd, imm8 * 4);
     })},
     { "load/store reg offset",   MakeMatcher("0101oooxxxxxxxxx", [](Visitor* v, u32 instruction) {
         u32 opcode = bits<9, 11>(instruction);
@@ -404,7 +404,7 @@ static const std::array<Instruction, 27> thumb_instruction_table = { {
         if (!L) { // STMIA Rn!, { reglist }
             v->STM(0xE, /*P=*/0, /*U=*/1, /*W=*/1, Rn, reglist);
         } else { // LDMIA Rn!, { reglist }
-            bool w = reglist & (1 << Rn);
+            bool w = (reglist & (1 << Rn)) == 0;
             v->LDM(0xE, /*P=*/0, /*U=*/1, /*W=*/w, Rn, reglist);
         }
     })},

src/core/arm/jit_x64/instructions/load_store.cpp

@@ -864,13 +864,13 @@ static void LoadAndStoreMultiple_DecrementAfter(XEmitter* code, RegAlloc& reg_al
 }
 
 static void LoadAndStoreMultiple_DecrementBefore(XEmitter* code, RegAlloc& reg_alloc, bool W, ArmReg Rn_index, ArmRegList list, std::function<void()> call) {
-    if (W && (list & (1 << Rn_index))) {
+    if (W && !(list & (1 << Rn_index))) {
         X64Reg Rn = reg_alloc.BindArmForReadWrite(Rn_index);
         code->SUB(32, R(Rn), Imm32(4 * Common::CountSetBits(list)));
         code->MOV(32, R(ABI_PARAM1), R(Rn));
         reg_alloc.UnlockArm(Rn_index);
         call();
-    } else if (W && (list & (1 << Rn_index))) {
+    } else if (W) {
         X64Reg Rn = reg_alloc.BindArmForReadWrite(Rn_index);
         code->MOV(32, R(ABI_PARAM1), R(Rn));
         code->SUB(32, R(ABI_PARAM1), Imm32(4 * Common::CountSetBits(list)));
@@ -898,6 +898,12 @@ static void LoadAndStoreMultiple_Helper(XEmitter* code, RegAlloc& reg_alloc, boo
     reg_alloc.FlushX64(ABI_PARAM3);
     reg_alloc.LockX64(ABI_PARAM3);
 
+    for (int i = 0; i < 15; i++) {
+        if (list & (1 << i)) {
+            reg_alloc.FlushArm(i);
+        }
+    }
+
     code->MOV(32, R(ABI_PARAM2), Imm32(list));
     code->MOV(64, R(ABI_PARAM3), R(reg_alloc.JitStateReg()));
 

src/core/arm/jit_x64/jit_x64.cpp

@@ -163,6 +163,7 @@ void JitX64::CompileCallHost(const void* const fn) {
     // There is no need to setup the stack as the stored RSP has already been properly aligned.
 
     reg_alloc.FlushABICallerSaved();
+    reg_alloc.FlushX64(RSP);
 
     ASSERT(reg_alloc.JitStateReg() != RSP);
     code->MOV(64, R(RSP), MJitStateHostReturnRSP());

src/core/arm/jit_x64/reg_alloc.cpp

@@ -121,10 +121,14 @@ void RegAlloc::FlushArm(ArmReg arm_reg) {
     ASSERT(arm_reg >= 0 && arm_reg <= 15);
 
     ArmState& arm_state = arm_gpr[arm_reg];
+    ASSERT(!arm_state.locked);
+    if (!arm_state.location.IsSimpleReg()) {
+        return;
+    }
+
     Gen::X64Reg x64_reg = GetX64For(arm_reg);
     X64State& x64_state = x64_gpr[x64_reg_to_index.at(x64_reg)];
 
-    ASSERT(!arm_state.locked);
     ASSERT(!x64_state.locked);
     ASSERT(x64_state.state == X64State::State::CleanArmReg || x64_state.state == X64State::State::DirtyArmReg);
     ASSERT(x64_state.arm_reg == arm_reg);

src/core/arm/jit_x64/reg_alloc.h

@@ -175,6 +175,9 @@ public:
      */
     void FlushABICallerSaved();
 
+    /// Ensures that the ARM register arm_reg is not in an x64 register.
+    void FlushArm(ArmReg arm_reg);
+
     // Debug:
 
     void AssertNoLocked();
@@ -182,8 +185,6 @@ public:
 private:
     /// INTERNAL: Gets the x64 register this ArmReg is currently bound to.
     Gen::X64Reg GetX64For(ArmReg arm_reg);
-    /// INTERNAL: Ensures that this ARM register is not in an x64 register.
-    void FlushArm(ArmReg arm_reg);
     /// INTERNAL: Is this ARM register currently in an x64 register?
     bool IsBoundToX64(ArmReg arm_reg);
     /// INTERNAL: Marks register as dirty. Ensures that it is written back to memory if it's in a x64 register.

src/tests/core/arm/jit_x64/fuzz_arm_common.cpp

@@ -4,6 +4,7 @@
 
 #include <cstdio>
 #include <cstring>
+#include <inttypes.h>
 #include <memory>
 #include <vector>
 
@@ -85,7 +86,7 @@ void FuzzJit(const int instruction_count, const int instructions_to_execute_coun
     SCOPE_EXIT({ Core::Shutdown(); });
 
     // Prepare memory
-    std::shared_ptr<TestMemory> test_mem = std::make_unique<TestMemory>();
+    std::shared_ptr<TestMemory> test_mem = std::make_shared<TestMemory>();
     Memory::MapIoRegion(0x00000000, 0x80000000, test_mem);
     Memory::MapIoRegion(0x80000000, 0x80000000, test_mem);
     SCOPE_EXIT({
@@ -162,9 +163,9 @@ void FuzzJit(const int instruction_count, const int instructions_to_execute_coun
                 size_t i = 0;
                 while (i < interp_mem_recording.size() || i < jit_mem_recording.size()) {
                     if (i < interp_mem_recording.size())
-                        printf("interp: %i %08x %08x\n", interp_mem_recording[i].size, interp_mem_recording[i].addr, interp_mem_recording[i].data);
+                        printf("interp: %zu %08x %08" PRIx64 "\n", interp_mem_recording[i].size, interp_mem_recording[i].addr, interp_mem_recording[i].data);
                     if (i < jit_mem_recording.size())
-                        printf("jit   : %i %08x %08x\n", jit_mem_recording[i].size, jit_mem_recording[i].addr, jit_mem_recording[i].data);
+                        printf("jit   : %zu %08x %08" PRIx64 "\n", jit_mem_recording[i].size, jit_mem_recording[i].addr, jit_mem_recording[i].data);
                     i++;
                 }
             }

src/tests/core/arm/jit_x64/fuzz_arm_load_store.cpp

@@ -56,7 +56,7 @@ TEST_CASE("Fuzz ARM load/store instructions (byte, half-word, word)", "[JitX64]"
     };
 
     SECTION("short blocks") {
-        FuzzJit(5, 6, 5000, instruction_select);
+        FuzzJit(5, 6, 1000, instruction_select);
     }
 }
 
@@ -118,7 +118,7 @@ TEST_CASE("Fuzz ARM load/store multiple instructions", "[JitX64]") {
         if (inst_index == 1 && (flags & 2)) {
             if (reg_list & (1 << Rn))
                 reg_list &= ~((1 << Rn) - 1);
-        } else if (inst_index == 1 && (flags & 2)) {
+        } else if (inst_index == 0 && (flags & 2)) {
             reg_list &= ~(1 << Rn);
         }
 
@@ -127,7 +127,5 @@ TEST_CASE("Fuzz ARM load/store multiple instructions", "[JitX64]") {
         return instructions[inst_index].first | (assemble_randoms & (~instructions[inst_index].second));
     };
 
-    SECTION("short blocks") {
+    FuzzJit(1, 1, 10000, instruction_select);
-        FuzzJit(1, 1, 5000, instruction_select);
-    }
 }

src/tests/core/arm/jit_x64/fuzz_thumb.cpp

@@ -4,10 +4,11 @@
 
 #include <cstdio>
 #include <cstring>
+#include <inttypes.h>
 
 #include <catch.hpp>
 
-#include "common/common_types.h"
+#include "common/make_unique.h"
 #include "common/scope_exit.h"
 
 #include "core/arm/dyncom/arm_dyncom.h"
@@ -40,16 +41,61 @@ std::pair<u16, u16> FromBitString16(const char* str) {
     return{ bits, mask };
 }
 
+class TestMemory final : public Memory::MMIORegion {
+public:
+    static constexpr size_t CODE_MEMORY_SIZE = 4096 * 2;
+    std::array<u16, CODE_MEMORY_SIZE> code_mem{};
+
+    u8 Read8(VAddr addr) override { return addr; }
+    u16 Read16(VAddr addr) override {
+        if (addr < CODE_MEMORY_SIZE) {
+            addr /= 2;
+            return code_mem[addr];
+        } else {
+            return addr;
+        }
+    }
+    u32 Read32(VAddr addr) override {
+        if (addr < CODE_MEMORY_SIZE) {
+            addr /= 2;
+            return code_mem[addr] | (code_mem[addr+1] << 16);
+        } else {
+            return addr;
+        }
+    }
+    u64 Read64(VAddr addr) override { return addr; }
+
+    struct WriteRecord {
+        WriteRecord(size_t size, VAddr addr, u64 data) : size(size), addr(addr), data(data) {}
+        size_t size;
+        VAddr addr;
+        u64 data;
+        bool operator==(const WriteRecord& o) const {
+            return std::tie(size, addr, data) == std::tie(o.size, o.addr, o.data);
+        }
+    };
+
+    std::vector<WriteRecord> recording;
+
+    void Write8(VAddr addr, u8 data) override { recording.emplace_back(1, addr, data); }
+    void Write16(VAddr addr, u16 data) override { recording.emplace_back(2, addr, data); }
+    void Write32(VAddr addr, u32 data) override { recording.emplace_back(4, addr, data); }
+    void Write64(VAddr addr, u64 data) override { recording.emplace_back(8, addr, data); }
+};
+
 void FuzzJitThumb(const int instruction_count, const int instructions_to_execute_count, const int run_count, const std::function<u16(int)> instruction_generator) {
     // Init core
     Core::Init();
     SCOPE_EXIT({ Core::Shutdown(); });
 
     // Prepare memory
-    constexpr size_t MEMORY_SIZE = 4096 * 2;
+    std::shared_ptr<TestMemory> test_mem = std::make_shared<TestMemory>();
-    std::array<u8, MEMORY_SIZE> test_mem{};
+    Memory::MapIoRegion(0x00000000, 0x80000000, test_mem);
-    Memory::MapMemoryRegion(0, MEMORY_SIZE, test_mem.data());
+    Memory::MapIoRegion(0x80000000, 0x80000000, test_mem);
-    SCOPE_EXIT({ Memory::UnmapRegion(0, MEMORY_SIZE); });
+    SCOPE_EXIT({
+        Memory::UnmapRegion(0x00000000, 0x80000000);
+    Memory::UnmapRegion(0x80000000, 0x80000000);
+    });
 
     // Prepare test subjects
     JitX64::ARM_Jit jit(PrivilegeMode::USER32MODE);
@@ -77,18 +123,23 @@ void FuzzJitThumb(const int instruction_count, const int instructions_to_execute
         interp.SetPC(0);
         jit.SetPC(0);
 
-        Memory::Write32(0, 0xFAFFFFFF); // blx +#4 // Jump to the following code (switch to thumb)
+        test_mem->code_mem[0] = 0xFFFF;
+        test_mem->code_mem[1] = 0xFAFF; // blx +#4 // Jump to the following code (switch to thumb)
 
         for (int i = 0; i < instruction_count; i++) {
             u16 inst = instruction_generator(i);
-
+            test_mem->code_mem[2 + i] = inst;
-            Memory::Write16(4 + i * 2, inst);
         }
 
-        Memory::Write16(4 + instruction_count * 2, 0xE7FE); // b +#0 // busy wait loop
+        test_mem->code_mem[2 + instruction_count] = 0xE7FE; // b +#0 // busy wait loop
 
+        test_mem->recording.clear();
         interp.ExecuteInstructions(instructions_to_execute_count);
+        auto interp_mem_recording = test_mem->recording;
+
+        test_mem->recording.clear();
         jit.ExecuteInstructions(instructions_to_execute_count);
+        auto jit_mem_recording = test_mem->recording;
 
         bool pass = true;
 
@@ -96,13 +147,14 @@ void FuzzJitThumb(const int instruction_count, const int instructions_to_execute
         for (int i = 0; i <= 15; i++) {
             if (interp.GetReg(i) != jit.GetReg(i)) pass = false;
         }
+        if (interp_mem_recording != jit_mem_recording) pass = false;
 
         if (!pass) {
             printf("Failed at execution number %i\n", run_number);
 
             printf("\nInstruction Listing: \n");
             for (int i = 0; i < instruction_count; i++) {
-                printf("%04x\n", Memory::Read16(4 + i * 2));
+                printf("%04x\n", test_mem->code_mem[2 + i]);
             }
 
             printf("\nFinal Register Listing: \n");
@@ -111,6 +163,18 @@ void FuzzJitThumb(const int instruction_count, const int instructions_to_execute
             }
             printf("CPSR: %08x %08x %s\n", interp.GetCPSR(), jit.GetCPSR(), interp.GetCPSR() != jit.GetCPSR() ? "*" : "");
 
+            if (interp_mem_recording != jit_mem_recording) {
+                printf("memory write recording mismatch *\n");
+                size_t i = 0;
+                while (i < interp_mem_recording.size() || i < jit_mem_recording.size()) {
+                    if (i < interp_mem_recording.size())
+                        printf("interp: %zu %08x %08" PRIx64 "\n", interp_mem_recording[i].size, interp_mem_recording[i].addr, interp_mem_recording[i].data);
+                    if (i < jit_mem_recording.size())
+                        printf("jit   : %zu %08x %08" PRIx64 "\n", jit_mem_recording[i].size, jit_mem_recording[i].addr, jit_mem_recording[i].data);
+                    i++;
+                }
+            }
+
             printf("\nInterpreter walkthrough:\n");
             interp.ClearCache();
             interp.SetPC(0);
@@ -141,21 +205,14 @@ void FuzzJitThumb(const int instruction_count, const int instructions_to_execute
 }
 
 // Things not yet tested:
-// FromBitString16("01001xxxxxxxxxxx"), // LDR Rd, [PC, #]
+//
-// FromBitString16("101101100101x000"), // SETEND
 // FromBitString16("10111110xxxxxxxx"), // BKPT
-// FromBitString16("0101oooxxxxxxxxx"), // LDR/STR
-// FromBitString16("011xxxxxxxxxxxxx"), // loads/stores
-// FromBitString16("1000xxxxxxxxxxxx"), // loads/stores
-// FromBitString16("1001xxxxxxxxxxxx"), // loads/stores
-// FromBitString16("1011x10xxxxxxxxx"), // push/pop
 // FromBitString16("10110110011x0xxx"), // CPS
-// FromBitString16("1100xxxxxxxxxxxx"), // STMIA/LDMIA
 // FromBitString16("11011111xxxxxxxx"), // SWI
-// FromBitString16("1101xxxxxxxxxxxx"), // B<cond>
+// FromBitString16("1011x101xxxxxxxx"), // PUSH/POP (R = 1)
 
-TEST_CASE("Fuzz Thumb instructions set 1 (pure computation)", "[JitX64][Thumb]") {
+TEST_CASE("Fuzz Thumb instructions set 1", "[JitX64][Thumb]") {
-    const std::array<std::pair<u16, u16>, 16> instructions = {{
+    const std::array<std::pair<u16, u16>, 24> instructions = {{
         FromBitString16("00000xxxxxxxxxxx"), // LSL <Rd>, <Rm>, #<imm5>
         FromBitString16("00001xxxxxxxxxxx"), // LSR <Rd>, <Rm>, #<imm5>
         FromBitString16("00010xxxxxxxxxxx"), // ASR <Rd>, <Rm>, #<imm5>
@@ -172,14 +229,38 @@ TEST_CASE("Fuzz Thumb instructions set 1 (pure computation)", "[JitX64][Thumb]")
         FromBitString16("1011101000xxxxxx"), // REV
         FromBitString16("1011101001xxxxxx"), // REV16
         FromBitString16("1011101011xxxxxx"), // REVSH
+        FromBitString16("01001xxxxxxxxxxx"), // LDR Rd, [PC, #]
+        FromBitString16("0101oooxxxxxxxxx"), // LDR/STR Rd, [Rn, Rm]
+        FromBitString16("011xxxxxxxxxxxxx"), // LDR(B)/STR(B) Rd, [Rn, #]
+        FromBitString16("1000xxxxxxxxxxxx"), // LDRH/STRH Rd, [Rn, #offset]
+        FromBitString16("1001xxxxxxxxxxxx"), // LDR/STR Rd, [SP, #]
+        FromBitString16("1011x100xxxxxxxx"), // PUSH/POP (R = 0)
+        FromBitString16("1100xxxxxxxxxxxx"), // STMIA/LDMIA
+        FromBitString16("101101100101x000"), // SETEND
     }};
 
     auto instruction_select = [&](int) -> u16 {
         size_t inst_index = RandInt<size_t>(0, instructions.size() - 1);
 
-        u16 random = RandInt<u16>(0, 0xFFFF);
+        if (inst_index == 22) {
-
+            u16 L = RandInt<u16>(0, 1);
-        return instructions[inst_index].first | (random &~ instructions[inst_index].second);
+            u16 Rn = RandInt<u16>(0, 7);
+            u16 reg_list = RandInt<u16>(1, 0xFF);
+            if (!L && (reg_list & (1 << Rn))) {
+                reg_list &= ~((1 << Rn) - 1);
+                if (reg_list == 0) reg_list = 0x80;
+            }
+            u16 random = (L << 11) | (Rn << 8) | reg_list;
+            return instructions[inst_index].first | (random &~instructions[inst_index].second);
+        } else if (inst_index == 21) {
+            u16 L = RandInt<u16>(0, 1);
+            u16 reg_list = RandInt<u16>(1, 0xFF);
+            u16 random = (L << 11) | reg_list;
+            return instructions[inst_index].first | (random &~instructions[inst_index].second);
+        } else {
+            u16 random = RandInt<u16>(0, 0xFFFF);
+            return instructions[inst_index].first | (random &~instructions[inst_index].second);
+        }
     };
 
     SECTION("short blocks") {
@@ -192,12 +273,25 @@ TEST_CASE("Fuzz Thumb instructions set 1 (pure computation)", "[JitX64][Thumb]")
 }
 
 TEST_CASE("Fuzz Thumb instructions set 2 (affects PC)", "[JitX64][Thumb]") {
-    const std::array<std::pair<u16, u16>, 5> instructions = {{
+    const std::array<std::pair<u16, u16>, 18> instructions = {{
         FromBitString16("01000111xxxxx000"), // BLX/BX
         FromBitString16("1010oxxxxxxxxxxx"), // add to pc/sp
         FromBitString16("11100xxxxxxxxxxx"), // B
         FromBitString16("01000100h0xxxxxx"), // ADD (high registers)
         FromBitString16("01000110h0xxxxxx"), // MOV (high registers)
+        FromBitString16("11010001xxxxxxxx"), // B<cond>
+        FromBitString16("11010010xxxxxxxx"), // B<cond>
+        FromBitString16("11010011xxxxxxxx"), // B<cond>
+        FromBitString16("11010100xxxxxxxx"), // B<cond>
+        FromBitString16("11010101xxxxxxxx"), // B<cond>
+        FromBitString16("11010110xxxxxxxx"), // B<cond>
+        FromBitString16("11010111xxxxxxxx"), // B<cond>
+        FromBitString16("11011000xxxxxxxx"), // B<cond>
+        FromBitString16("11011001xxxxxxxx"), // B<cond>
+        FromBitString16("11011010xxxxxxxx"), // B<cond>
+        FromBitString16("11011011xxxxxxxx"), // B<cond>
+        FromBitString16("11011100xxxxxxxx"), // B<cond>
+        FromBitString16("11011110xxxxxxxx"), // B<cond>
     }};
 
     auto instruction_select = [&](int) -> u16 {