From 2fde2140ef3bf0810036f1aa80511e86cb468062 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 26 Aug 2023 16:06:51 -0700 Subject: [PATCH 1/9] Arm64: Ensure assert is testing correct array --- FEXCore/Source/Interface/Core/JIT/Arm64/MemoryOps.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/FEXCore/Source/Interface/Core/JIT/Arm64/MemoryOps.cpp b/FEXCore/Source/Interface/Core/JIT/Arm64/MemoryOps.cpp index 20e15114c..199360d21 100644 --- a/FEXCore/Source/Interface/Core/JIT/Arm64/MemoryOps.cpp +++ b/FEXCore/Source/Interface/Core/JIT/Arm64/MemoryOps.cpp @@ -133,7 +133,7 @@ DEF_OP(LoadRegister) { [[maybe_unused]] const auto regId = (Op->Offset / Core::CPUState::GPR_REG_SIZE) - 1; const auto regOffs = Op->Offset & 7; - LOGMAN_THROW_A_FMT(regId < StaticFPRegisters.size(), "out of range regId"); + LOGMAN_THROW_A_FMT(regId < StaticRegisters.size(), "out of range regId"); switch (OpSize) { case 1: @@ -297,7 +297,7 @@ DEF_OP(LoadRegisterSRA) { const auto regId = (Op->Offset - offsetof(Core::CpuStateFrame, State.gregs[0])) / Core::CPUState::GPR_REG_SIZE; const auto regOffs = Op->Offset & 7; - LOGMAN_THROW_A_FMT(regId < StaticFPRegisters.size(), "out of range regId"); + LOGMAN_THROW_A_FMT(regId < StaticRegisters.size(), "out of range regId"); const auto reg = StaticRegisters[regId]; From a01e69092d042af73fadb69f6265b494fdfc1834 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 26 Aug 2023 16:07:22 -0700 Subject: [PATCH 2/9] Arm64: Ensure Bfe and Sbfe operate at 32-bit or 64-bit op size For Sbfe at least it ensures the upper bits don't get filled with garbage. Bfe it doesn't change behaviour but best to be correct. --- FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp b/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp index 86b464826..37f125597 100644 --- a/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp +++ b/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp @@ -1128,19 +1128,24 @@ DEF_OP(Bfe) { auto Op = IROp->C(); LOGMAN_THROW_AA_FMT(IROp->Size <= 8, "OpSize is too large for BFE: {}", IROp->Size); LOGMAN_THROW_AA_FMT(Op->Width != 0, "Invalid BFE width of 0"); + const uint8_t OpSize = IROp->Size; + const auto EmitSize = OpSize == 8 ? ARMEmitter::Size::i64Bit : ARMEmitter::Size::i32Bit; const auto Dst = GetReg(Node); const auto Src = GetReg(Op->Src.ID()); - ubfx(ARMEmitter::Size::i64Bit, Dst, Src, Op->lsb, Op->Width); + ubfx(EmitSize, Dst, Src, Op->lsb, Op->Width); } DEF_OP(Sbfe) { auto Op = IROp->C(); + const uint8_t OpSize = IROp->Size; + const auto EmitSize = OpSize == 8 ? ARMEmitter::Size::i64Bit : ARMEmitter::Size::i32Bit; + const auto Dst = GetReg(Node); const auto Src = GetReg(Op->Src.ID()); - sbfx(ARMEmitter::Size::i64Bit, Dst, Src, Op->lsb, Op->Width); + sbfx(EmitSize, Dst, Src, Op->lsb, Op->Width); } ARMEmitter::Condition MapSelectCC(IR::CondClassType Cond) { From a4ac21a4e4d54ab38b5df9f2f51aaf5f91d90d78 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 26 Aug 2023 16:58:32 -0700 Subject: [PATCH 3/9] OpcodeDispatcher: Fixes bug in GetRFLAG with CachedNZCV This was only operating at byte size but it was attempting to get bit offsets at greater than operating size. Change operating size over to 64-bit. --- FEXCore/Source/Interface/Core/OpcodeDispatcher.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.h b/FEXCore/Source/Interface/Core/OpcodeDispatcher.h index 16850407b..4b1687f0e 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.h +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.h @@ -1195,7 +1195,7 @@ private: OrderedNode *GetRFLAG(unsigned BitOffset) { if (IsNZCV(BitOffset)) { if (!CachedNZCV || (PossiblySetNZCVBits & (1u << IndexNZCV(BitOffset)))) - return _Bfe(1, 1, IndexNZCV(BitOffset), GetNZCV()); + return _Bfe(8, 1, IndexNZCV(BitOffset), GetNZCV()); else return _Constant(0); } else { From eace648fa94829a9ed8496e6c0ea7ad73667ae0f Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 26 Aug 2023 16:59:57 -0700 Subject: [PATCH 4/9] OpcodeDispatcher: Fixes bug in UMUL This was trying to operating on a 32-bit value but BFE the upper 32-bits. Actually fixes this so it is operating on the 64-bit multiply result. --- FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index d2e459cca..1149c2976 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -3281,7 +3281,6 @@ void OpDispatchBuilder::IMULOp(OpcodeArgs) { void OpDispatchBuilder::MULOp(OpcodeArgs) { const uint8_t Size = GetSrcSize(Op); - const uint8_t GPRSize = CTX->GetGPRSize(); OrderedNode *Src1 = LoadSource(GPRClass, Op, Op->Dest, Op->Flags, -1); OrderedNode* Src2 = LoadGPRRegister(X86State::REG_RAX); @@ -3308,8 +3307,8 @@ void OpDispatchBuilder::MULOp(OpcodeArgs) { else if (Size == 4) { // 32bits stored in EAX // 32bits stored in EDX - OrderedNode *ResultLow = _Bfe(GPRSize, 32, 0, Result); - ResultHigh = _Bfe(GPRSize, 32, 32, Result); + OrderedNode *ResultLow = _Bfe(8, 32, 0, Result); + ResultHigh = _Bfe(8, 32, 32, Result); StoreGPRRegister(X86State::REG_RAX, ResultLow); StoreGPRRegister(X86State::REG_RDX, ResultHigh); } From 8f7925d06ff8302ea8a77ee35d6b1bc83c5ec01f Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 26 Aug 2023 17:01:35 -0700 Subject: [PATCH 5/9] Arm64: Simple typo fix --- FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp b/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp index 37f125597..982c56d0d 100644 --- a/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp +++ b/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp @@ -1138,7 +1138,7 @@ DEF_OP(Bfe) { } DEF_OP(Sbfe) { - auto Op = IROp->C(); + auto Op = IROp->C(); const uint8_t OpSize = IROp->Size; const auto EmitSize = OpSize == 8 ? ARMEmitter::Size::i64Bit : ARMEmitter::Size::i32Bit; From fcc37bf6a86651a8e294ebee9d22620c3b8096bd Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 26 Aug 2023 17:35:29 -0700 Subject: [PATCH 6/9] OpcodeDispatcher: Fixes RCR and ADOX 32-bit Automatic size inheritance was breaking these operations. --- FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp | 8 +++++--- FEXCore/Source/Interface/Core/OpcodeDispatcher.h | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index 1149c2976..7e9f76acf 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -2474,6 +2474,8 @@ void OpDispatchBuilder::PEXT(OpcodeArgs) { } void OpDispatchBuilder::ADXOp(OpcodeArgs) { + const auto OperandSize = GetSrcSize(Op); + // Calculate flags early. CalculateDeferredFlags(); @@ -2483,9 +2485,9 @@ void OpDispatchBuilder::ADXOp(OpcodeArgs) { auto* Flag = [&]() -> OrderedNode* { if (IsADCX) { - return GetRFLAG(X86State::RFLAG_CF_LOC); + return GetRFLAG(X86State::RFLAG_CF_LOC, OperandSize); } else { - return GetRFLAG(X86State::RFLAG_OF_LOC); + return GetRFLAG(X86State::RFLAG_OF_LOC, OperandSize); } }(); @@ -2516,7 +2518,7 @@ void OpDispatchBuilder::RCROp1Bit(OpcodeArgs) { OrderedNode *Dest = LoadSource(GPRClass, Op, Op->Dest, Op->Flags, -1); const auto Size = GetSrcBitSize(Op); - auto CF = GetRFLAG(FEXCore::X86State::RFLAG_CF_LOC); + auto CF = GetRFLAG(FEXCore::X86State::RFLAG_CF_LOC, GetSrcSize(Op)); uint32_t Shift = 1; diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.h b/FEXCore/Source/Interface/Core/OpcodeDispatcher.h index 4b1687f0e..65b9bf17e 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.h +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.h @@ -1192,10 +1192,10 @@ private: void ZeroMultipleFlags(uint32_t BitMask); - OrderedNode *GetRFLAG(unsigned BitOffset) { + OrderedNode *GetRFLAG(unsigned BitOffset, int32_t Size = 8) { if (IsNZCV(BitOffset)) { if (!CachedNZCV || (PossiblySetNZCVBits & (1u << IndexNZCV(BitOffset)))) - return _Bfe(8, 1, IndexNZCV(BitOffset), GetNZCV()); + return _Bfe(Size, 1, IndexNZCV(BitOffset), GetNZCV()); else return _Constant(0); } else { From 572d6cd3e61cc25fca9f8bbea98105b916236a07 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 26 Aug 2023 17:53:50 -0700 Subject: [PATCH 7/9] OpcodeDispatcher: Fixes ADC and SBB --- FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index 7e9f76acf..7d5361385 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -447,7 +447,7 @@ void OpDispatchBuilder::ADCOp(OpcodeArgs) { OrderedNode *Src = LoadSource(GPRClass, Op, Op->Src[SrcIndex], Op->Flags, -1); uint8_t Size = GetDstSize(Op); - auto CF = GetRFLAG(FEXCore::X86State::RFLAG_CF_LOC); + auto CF = GetRFLAG(FEXCore::X86State::RFLAG_CF_LOC, Size); auto ALUOp = _Add(Src, CF); OrderedNode *Result{}; @@ -478,7 +478,7 @@ void OpDispatchBuilder::SBBOp(OpcodeArgs) { OrderedNode *Src = LoadSource(GPRClass, Op, Op->Src[SrcIndex], Op->Flags, -1); auto Size = GetDstSize(Op); - auto CF = GetRFLAG(FEXCore::X86State::RFLAG_CF_LOC); + auto CF = GetRFLAG(FEXCore::X86State::RFLAG_CF_LOC, Size); auto ALUOp = _Add(Src, CF); OrderedNode *Result{}; From 6cb1afc2ad36adaeccd4e4bc7c951af8d1181c97 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 26 Aug 2023 17:54:10 -0700 Subject: [PATCH 8/9] unittests/asm: Adds ADC/SBB test --- unittests/32Bit_ASM/FEX_bugs/adc.asm | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 unittests/32Bit_ASM/FEX_bugs/adc.asm diff --git a/unittests/32Bit_ASM/FEX_bugs/adc.asm b/unittests/32Bit_ASM/FEX_bugs/adc.asm new file mode 100644 index 000000000..e5cb0a3e0 --- /dev/null +++ b/unittests/32Bit_ASM/FEX_bugs/adc.asm @@ -0,0 +1,26 @@ +%ifdef CONFIG +{ + "RegData": { + "RAX": "0x00000000fffffffe", + "RBX": "0x0000000000000001" + }, + "Mode": "32BIT" +} +%endif + +; FEX had a bug where ADD or SUB with carry was generating results with garbage in the upper 32-bits. + +mov eax, -1 +mov ebx, -1 +mov edx, -1 + +clc +adc eax, edx +adc eax, edx +adc eax, edx + +clc +sbb ebx, edx +sbb ebx, edx +sbb ebx, edx +hlt From 714669136086cee0d2cc4dfb479e26b204206c37 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 26 Aug 2023 15:59:26 -0700 Subject: [PATCH 9/9] IR/Passes/RA: Enable SRA for 32-bit GPRs Noticed that we hadn't ever enabled this, which was a concern when our GPR operations weren't as strict about leaving garbage in the upper bits when operating as a 32-bit operation. Now that our ALU operations are more strict about enforcing upper bit zeroing we can enable this. This causes Half-Life: Source FPS to get to > 200FPS finally. Causes significant performance improvements for 32-bit games because we're no longer redundantly moving registers before and after every operation. Causing a bunch of 3-4 instruction sequences to convert to 1. --- FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp b/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp index 40cf617ac..755a6e0f4 100644 --- a/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp +++ b/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp @@ -548,7 +548,7 @@ namespace { auto IsPreWritable = [](uint8_t Size, RegisterClassType StaticClass) { LOGMAN_THROW_A_FMT(StaticClass == GPRFixedClass || StaticClass == FPRFixedClass, "Unexpected static class {}", StaticClass); if (StaticClass == GPRFixedClass) { - return Size == 8; + return Size == 8 || Size == 4; } else if (StaticClass == FPRFixedClass) { return Size == 16; } @@ -560,7 +560,7 @@ namespace { LOGMAN_THROW_A_FMT(StaticClass == GPRFixedClass || StaticClass == FPRFixedClass, "Unexpected static class {}", StaticClass); if (StaticClass == GPRFixedClass) { // We need more meta info to support not-size-of-reg - return (Size == 8 /*|| Size == 4*/) && ((Offset & 7) == 0); + return (Size == 8 || Size == 4) && ((Offset & 7) == 0); } else if (StaticClass == FPRFixedClass) { // We need more meta info to support not-size-of-reg return (Size == 16 /*|| Size == 8 || Size == 4*/) && ((Offset & 15) == 0);