From cd85b7fdaac28be99173a9380c4120c5d299d1e8 Mon Sep 17 00:00:00 2001 From: Merry Date: Mon, 11 Jul 2022 15:57:14 +0100 Subject: [PATCH] emit_x64: Fix bugs in fast dispatcher * We failed to invalidate entries if there are no patches required for a location descriptor. * Bug in A64 hashing code (rbx instead of rbp). * Bug in A32 and A64 lookup code (inconsistent choice of key: PC vs IR::LocationDescriptor). * Test case added. --- src/dynarmic/backend/x64/a32_emit_x64.cpp | 6 +- src/dynarmic/backend/x64/a64_emit_x64.cpp | 5 +- src/dynarmic/backend/x64/emit_x64.cpp | 9 +- tests/A64/test_invalidation.cpp | 113 ++++++++++++++++++++++ tests/CMakeLists.txt | 3 +- 5 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 tests/A64/test_invalidation.cpp diff --git a/src/dynarmic/backend/x64/a32_emit_x64.cpp b/src/dynarmic/backend/x64/a32_emit_x64.cpp index 4a3c3a35..2a6424b9 100644 --- a/src/dynarmic/backend/x64/a32_emit_x64.cpp +++ b/src/dynarmic/backend/x64/a32_emit_x64.cpp @@ -235,8 +235,9 @@ void A32EmitX64::GenTerminalHandlers() { calculate_location_descriptor(); code.L(rsb_cache_miss); code.mov(r12, reinterpret_cast(fast_dispatch_table.data())); + code.mov(rbp, rbx); if (code.HasHostFeature(HostFeature::SSE42)) { - code.crc32(ebp, r12d); + code.crc32(rbp, r12); } code.and_(ebp, fast_dispatch_table_mask); code.lea(rbp, ptr[r12 + rbp]); @@ -254,11 +255,12 @@ void A32EmitX64::GenTerminalHandlers() { fast_dispatch_table_lookup = code.getCurr(); code.mov(code.ABI_PARAM2, reinterpret_cast(fast_dispatch_table.data())); if (code.HasHostFeature(HostFeature::SSE42)) { - code.crc32(code.ABI_PARAM1.cvt32(), code.ABI_PARAM2.cvt32()); + code.crc32(code.ABI_PARAM1, code.ABI_PARAM2); } code.and_(code.ABI_PARAM1.cvt32(), fast_dispatch_table_mask); code.lea(code.ABI_RETURN, code.ptr[code.ABI_PARAM1 + code.ABI_PARAM2]); code.ret(); + PerfMapRegister(fast_dispatch_table_lookup, code.getCurr(), "a32_fast_dispatch_table_lookup"); } } diff --git a/src/dynarmic/backend/x64/a64_emit_x64.cpp b/src/dynarmic/backend/x64/a64_emit_x64.cpp index eba49dfc..e0cd8c45 100644 --- a/src/dynarmic/backend/x64/a64_emit_x64.cpp +++ b/src/dynarmic/backend/x64/a64_emit_x64.cpp @@ -193,8 +193,9 @@ void A64EmitX64::GenTerminalHandlers() { calculate_location_descriptor(); code.L(rsb_cache_miss); code.mov(r12, reinterpret_cast(fast_dispatch_table.data())); + code.mov(rbp, rbx); if (code.HasHostFeature(HostFeature::SSE42)) { - code.crc32(rbx, r12d); + code.crc32(rbp, r12); } code.and_(ebp, fast_dispatch_table_mask); code.lea(rbp, ptr[r12 + rbp]); @@ -215,7 +216,7 @@ void A64EmitX64::GenTerminalHandlers() { code.crc32(code.ABI_PARAM1, code.ABI_PARAM2); } code.and_(code.ABI_PARAM1.cvt32(), fast_dispatch_table_mask); - code.lea(code.ABI_RETURN, code.ptr[code.ABI_PARAM1 + code.ABI_PARAM2]); + code.lea(code.ABI_RETURN, code.ptr[code.ABI_PARAM2 + code.ABI_PARAM1]); code.ret(); PerfMapRegister(fast_dispatch_table_lookup, code.getCurr(), "a64_fast_dispatch_table_lookup"); } diff --git a/src/dynarmic/backend/x64/emit_x64.cpp b/src/dynarmic/backend/x64/emit_x64.cpp index eb8196a0..ab160eca 100644 --- a/src/dynarmic/backend/x64/emit_x64.cpp +++ b/src/dynarmic/backend/x64/emit_x64.cpp @@ -325,7 +325,9 @@ void EmitX64::Patch(const IR::LocationDescriptor& target_desc, CodePtr target_co } void EmitX64::Unpatch(const IR::LocationDescriptor& target_desc) { - Patch(target_desc, nullptr); + if (patch_information.count(target_desc)) { + Patch(target_desc, nullptr); + } } void EmitX64::ClearCache() { @@ -345,9 +347,8 @@ void EmitX64::InvalidateBasicBlocks(const tsl::robin_set continue; } - if (patch_information.count(descriptor)) { - Unpatch(descriptor); - } + Unpatch(descriptor); + block_descriptors.erase(it); } } diff --git a/tests/A64/test_invalidation.cpp b/tests/A64/test_invalidation.cpp new file mode 100644 index 00000000..c1ac6665 --- /dev/null +++ b/tests/A64/test_invalidation.cpp @@ -0,0 +1,113 @@ +/* This file is part of the dynarmic project. + * Copyright (c) 2018 MerryMage + * SPDX-License-Identifier: 0BSD + */ + +#include + +#include "./testenv.h" +#include "dynarmic/interface/A64/a64.h" + +using namespace Dynarmic; + +TEST_CASE("ensure fast dispatch entry is cleared even when a block does not have any patching requirements", "[a64]") { + A64TestEnv env; + + A64::UserConfig conf{&env}; + A64::Jit jit{conf}; + + REQUIRE(conf.HasOptimization(OptimizationFlag::FastDispatch)); + + env.code_mem_start_address = 100; + env.code_mem.clear(); + env.code_mem.emplace_back(0xd2800d80); // MOV X0, 108 + env.code_mem.emplace_back(0xd61f0000); // BR X0 + env.code_mem.emplace_back(0xd2800540); // MOV X0, 42 + env.code_mem.emplace_back(0x14000000); // B . + + jit.SetPC(100); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 42); + + jit.SetPC(100); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 42); + + jit.InvalidateCacheRange(108, 4); + + jit.SetPC(100); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 42); + + env.code_mem[2] = 0xd28008a0; // MOV X0, 69 + + jit.SetPC(100); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 42); + + jit.InvalidateCacheRange(108, 4); + + jit.SetPC(100); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 69); + + jit.SetPC(100); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 69); +} + +TEST_CASE("ensure fast dispatch entry is cleared even when a block does not have any patching requirements 2", "[a64]") { + A64TestEnv env; + + A64::UserConfig conf{&env}; + A64::Jit jit{conf}; + + REQUIRE(conf.HasOptimization(OptimizationFlag::FastDispatch)); + + env.code_mem.emplace_back(0xd2800100); // MOV X0, 8 + env.code_mem.emplace_back(0xd61f0000); // BR X0 + env.code_mem.emplace_back(0xd2800540); // MOV X0, 42 + env.code_mem.emplace_back(0x14000000); // B . + + jit.SetPC(0); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 42); + + jit.SetPC(0); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 42); + + jit.InvalidateCacheRange(8, 4); + + jit.SetPC(0); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 42); + + env.code_mem[2] = 0xd28008a0; // MOV X0, 69 + + jit.SetPC(0); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 42); + + jit.InvalidateCacheRange(8, 4); + + jit.SetPC(0); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 69); + + jit.SetPC(0); + env.ticks_left = 4; + jit.Run(); + REQUIRE(jit.GetRegister(0) == 69); +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3d0013e8..05380ab3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -20,6 +20,8 @@ endif() if ("A64" IN_LIST DYNARMIC_FRONTENDS) target_sources(dynarmic_tests PRIVATE A64/a64.cpp + A64/misaligned_page_table.cpp + A64/test_invalidation.cpp A64/testenv.h ) endif() @@ -44,7 +46,6 @@ if (DYNARMIC_TESTS_USE_UNICORN) if ("A64" IN_LIST DYNARMIC_FRONTENDS) target_sources(dynarmic_tests PRIVATE A64/fuzz_with_unicorn.cpp - A64/misaligned_page_table.cpp A64/verify_unicorn.cpp unicorn_emu/a64_unicorn.cpp unicorn_emu/a64_unicorn.h