FEXCore: Stop leaking AVX configuration state

The dispatcher was saving AVX state even though FEX doesn't support it
currently. This is due to it checking for the config option rather than
the HostFeatures option.

The `EnableAVX` config option is supposed to be used to inform FEXCore
if we want AVX disabled or not when the host supports the feature. In
this case it is universally enabled because we haven't encountered any
games that have issues with AVX state being saved with signals. (We know
they exist, we just don't have configurations for them).

The HostFeatures option `SupportsAVX` is the option that is supposed to
be getting used for determining if the runtime AVX feature is enabled.
This also had an issue though that this was **also** always enabled if
running on an x86 host with AVX, or an ARM host with SVE2-256bit.
It was then disabled if the config option was disabled; But, since
FEX-Emu doesn't support AVX fully yet, we need to ensure this isn't yet
enabled.

But this only solves half the problem. In order for our CI to test AVX
features before fully supporting AVX, it needs to be able to enable AVX
so that the CPU state is correctly saved.

So we need to change the default configuration option to be false, and
have CI enable it for the tests that matter before AVX is fully
implemented.
This commit is contained in:
Ryan Houdek 2023-04-11 14:55:50 -07:00
parent 18154183ad
commit 44e06185b7
7 changed files with 16 additions and 13 deletions

View File

@ -14,6 +14,7 @@ env:
CC: clang
CXX: clang++
FEX_FORCE32BITALLOCATOR: 1
FEX_ENABLEAVX: 1
jobs:
build:

View File

@ -13,6 +13,7 @@ env:
BUILD_TYPE: Release
CC: clang
CXX: clang++
FEX_ENABLEAVX: 1
jobs:
build:

View File

@ -53,7 +53,7 @@
},
"EnableAVX": {
"Type": "bool",
"Default": "true",
"Default": "false",
"Desc": [
"Determines whether or not we use the expanded register file for AVX or not"
]

View File

@ -237,7 +237,6 @@ namespace FEXCore::Context {
FEX_CONFIG_OPT(ParanoidTSO, PARANOIDTSO);
FEX_CONFIG_OPT(CacheObjectCodeCompilation, CACHEOBJECTCODECOMPILATION);
FEX_CONFIG_OPT(x87ReducedPrecision, X87REDUCEDPRECISION);
FEX_CONFIG_OPT(EnableAVX, ENABLEAVX);
} Config;
FEXCore::HostFeatures HostFeatures;

View File

@ -153,10 +153,6 @@ namespace FEXCore::Context {
if (Config.CacheObjectCodeCompilation() != FEXCore::Config::ConfigObjectCodeHandler::CONFIG_NONE) {
CodeObjectCacheService = fextl::make_unique<FEXCore::CodeSerialize::CodeObjectSerializeService>(this);
}
if (!Config.EnableAVX) {
HostFeatures.SupportsAVX = false;
}
if (!Config.Is64BitMode()) {
// When operating in 32-bit mode, the virtual memory we care about is only the lower 32-bits.
Config.VirtualMemSize = 1ULL << 32;

View File

@ -104,7 +104,7 @@ ArchHelpers::Context::ContextBackup* Dispatcher::StoreThreadState(FEXCore::Core:
}
void Dispatcher::RestoreFrame_ia32(ArchHelpers::Context::ContextBackup* Context, FEXCore::Core::CpuStateFrame *Frame, void *ucontext) {
const bool IsAVXEnabled = CTX->Config.EnableAVX;
const bool IsAVXEnabled = CTX->HostFeatures.SupportsAVX;
SigFrame_i32 *guest_uctx = reinterpret_cast<SigFrame_i32*>(Context->UContextLocation);
// If the guest modified the RIP then we need to take special precautions here
@ -187,7 +187,7 @@ void Dispatcher::RestoreFrame_ia32(ArchHelpers::Context::ContextBackup* Context,
}
void Dispatcher::RestoreRTFrame_ia32(ArchHelpers::Context::ContextBackup* Context, FEXCore::Core::CpuStateFrame *Frame, void *ucontext) {
const bool IsAVXEnabled = CTX->Config.EnableAVX;
const bool IsAVXEnabled = CTX->HostFeatures.SupportsAVX;
RTSigFrame_i32 *guest_uctx = reinterpret_cast<RTSigFrame_i32*>(Context->UContextLocation);
// If the guest modified the RIP then we need to take special precautions here
@ -272,7 +272,7 @@ void Dispatcher::RestoreRTFrame_ia32(ArchHelpers::Context::ContextBackup* Contex
void Dispatcher::RestoreThreadState(FEXCore::Core::InternalThreadState *Thread, void *ucontext, RestoreType Type) {
// Pulling from context here
const bool Is64BitMode = CTX->Config.Is64BitMode;
const bool IsAVXEnabled = CTX->Config.EnableAVX;
const bool IsAVXEnabled = CTX->HostFeatures.SupportsAVX;
uint64_t OldSP{};
if (Type == RestoreType::TYPE_PAUSE) [[unlikely]] {
@ -436,7 +436,7 @@ uint64_t Dispatcher::SetupFrame_ia32(
GuestSigAction *GuestAction, stack_t *GuestStack,
uint64_t NewGuestSP, const uint32_t eflags) {
const bool IsAVXEnabled = CTX->Config.EnableAVX;
const bool IsAVXEnabled = CTX->HostFeatures.SupportsAVX;
const uint64_t SignalReturn = reinterpret_cast<uint64_t>(CTX->VDSOPointers.VDSO_kernel_sigreturn);
NewGuestSP -= sizeof(uint64_t);
@ -578,7 +578,7 @@ uint64_t Dispatcher::SetupRTFrame_ia32(
GuestSigAction *GuestAction, stack_t *GuestStack,
uint64_t NewGuestSP, const uint32_t eflags) {
const bool IsAVXEnabled = CTX->Config.EnableAVX;
const bool IsAVXEnabled = CTX->HostFeatures.SupportsAVX;
const uint64_t SignalReturn = reinterpret_cast<uint64_t>(CTX->VDSOPointers.VDSO_kernel_rt_sigreturn);
NewGuestSP -= sizeof(uint64_t);
@ -769,7 +769,7 @@ uint64_t Dispatcher::SetupRTFrame_ia32(
}
void Dispatcher::RestoreFrame_x64(ArchHelpers::Context::ContextBackup* Context, FEXCore::Core::CpuStateFrame *Frame, void *ucontext) {
const bool IsAVXEnabled = CTX->Config.EnableAVX;
const bool IsAVXEnabled = CTX->HostFeatures.SupportsAVX;
auto *guest_uctx = reinterpret_cast<FEXCore::x86_64::ucontext_t*>(Context->UContextLocation);
[[maybe_unused]] auto *guest_siginfo = reinterpret_cast<siginfo_t*>(Context->SigInfoLocation);
@ -852,7 +852,7 @@ uint64_t Dispatcher::SetupFrame_x64(
// 32-bit doesn't have a redzone
NewGuestSP -= 128;
const bool IsAVXEnabled = CTX->Config.EnableAVX;
const bool IsAVXEnabled = CTX->HostFeatures.SupportsAVX;
// On 64-bit the kernel sets up the siginfo_t and ucontext_t regardless of SA_SIGINFO set.
// This allows the application to /always/ get the siginfo and ucontext even if it didn't set this flag.

View File

@ -160,5 +160,11 @@ HostFeatures::HostFeatures() {
SupportsCLZERO = DCZID_Bytes == CPUIDEmu::CACHELINE_SIZE;
}
#endif
// Disable AVX if the configuration explicitly has disabled it.
FEX_CONFIG_OPT(EnableAVX, ENABLEAVX);
if (!EnableAVX) {
SupportsAVX = false;
}
}
}