From d6f50bf7b07bc909c77202bcaf358a6dd7b38bda Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Thu, 2 Mar 2023 05:02:31 -0800 Subject: [PATCH 1/2] IR: Implement support for MemSet operation This operation directly matches what the x86 STOS instruction does without supporting its faulting behaviour. STOS faulting behaviour is that RCX and RDI get updated to the last word written. Which is something that FEX hasn't ever supported. --- .../Core/Interpreter/InterpreterOps.cpp | 1 + .../Core/Interpreter/InterpreterOps.h | 1 + .../Interface/Core/Interpreter/MemoryOps.cpp | 105 +++++++++++ .../Source/Interface/Core/JIT/Arm64/JIT.cpp | 2 + .../Interface/Core/JIT/Arm64/JITClass.h | 1 + .../Interface/Core/JIT/Arm64/MemoryOps.cpp | 163 ++++++++++++++++++ .../Interface/Core/JIT/x86_64/JITClass.h | 1 + .../Interface/Core/JIT/x86_64/MemoryOps.cpp | 89 ++++++++++ External/FEXCore/Source/Interface/IR/IR.json | 8 + 9 files changed, 371 insertions(+) diff --git a/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.cpp b/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.cpp index d8c3ceb5d..718b41ea8 100644 --- a/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.cpp +++ b/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.cpp @@ -154,6 +154,7 @@ constexpr OpHandlerArray InterpreterOpHandlers = [] { REGISTER_OP(STOREMEM, StoreMem); REGISTER_OP(LOADMEMTSO, LoadMem); REGISTER_OP(STOREMEMTSO, StoreMem); + REGISTER_OP(MEMSET, MemSet); REGISTER_OP(CACHELINECLEAR, CacheLineClear); REGISTER_OP(CACHELINECLEAN, CacheLineClean); REGISTER_OP(CACHELINEZERO, CacheLineZero); diff --git a/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h b/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h index bd6040cd6..2654b3d0b 100644 --- a/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h +++ b/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h @@ -181,6 +181,7 @@ namespace FEXCore::CPU { DEF_OP(StoreFlag); DEF_OP(LoadMem); DEF_OP(StoreMem); + DEF_OP(MemSet); DEF_OP(CacheLineClear); DEF_OP(CacheLineClean); DEF_OP(CacheLineZero); diff --git a/External/FEXCore/Source/Interface/Core/Interpreter/MemoryOps.cpp b/External/FEXCore/Source/Interface/Core/Interpreter/MemoryOps.cpp index ed8a5f1a4..a62cedc9a 100644 --- a/External/FEXCore/Source/Interface/Core/Interpreter/MemoryOps.cpp +++ b/External/FEXCore/Source/Interface/Core/Interpreter/MemoryOps.cpp @@ -288,6 +288,111 @@ DEF_OP(StoreMem) { } } +DEF_OP(MemSet) { + const auto Op = IROp->C(); + const int32_t Size = Op->Size; + + char *MemData = *GetSrc(Data->SSAData, Op->Addr); + const auto Value = *GetSrc(Data->SSAData, Op->Value); + const auto Length = *GetSrc(Data->SSAData, Op->Length); + const auto Direction = *GetSrc(Data->SSAData, Op->Direction); + + auto MemSetElements = [](auto* Memory, uint64_t Value, size_t Length) { + for (size_t i = 0; i < Length; ++i) { + Memory[i] = Value; + } + }; + + auto MemSetElementsInverse = [](auto* Memory, uint64_t Value, size_t Length) { + for (size_t i = 0; i < Length; ++i) { + Memory[-i] = Value; + } + }; + + if (Direction == 0) { // Forward + if (Op->IsAtomic) { + switch (Size) { + case 1: + MemSetElements(reinterpret_cast*>(MemData), Value, Length); + break; + case 2: + MemSetElements(reinterpret_cast*>(MemData), Value, Length); + break; + case 4: + MemSetElements(reinterpret_cast*>(MemData), Value, Length); + break; + case 8: + MemSetElements(reinterpret_cast*>(MemData), Value, Length); + break; + default: + LOGMAN_MSG_A_FMT("Unhandled {} size: {}", __func__, Size); + break; + } + } + else { + switch (Size) { + case 1: + MemSetElements(reinterpret_cast(MemData), Value, Length); + break; + case 2: + MemSetElements(reinterpret_cast(MemData), Value, Length); + break; + case 4: + MemSetElements(reinterpret_cast(MemData), Value, Length); + break; + case 8: + MemSetElements(reinterpret_cast(MemData), Value, Length); + break; + default: + LOGMAN_MSG_A_FMT("Unhandled {} size: {}", __func__, Size); + break; + } + } + GD = reinterpret_cast(MemData + (Length * Size)); + } + else { // Backward + if (Op->IsAtomic) { + switch (Size) { + case 1: + MemSetElementsInverse(reinterpret_cast*>(MemData), Value, Length); + break; + case 2: + MemSetElementsInverse(reinterpret_cast*>(MemData), Value, Length); + break; + case 4: + MemSetElementsInverse(reinterpret_cast*>(MemData), Value, Length); + break; + case 8: + MemSetElementsInverse(reinterpret_cast*>(MemData), Value, Length); + break; + default: + LOGMAN_MSG_A_FMT("Unhandled {} size: {}", __func__, Size); + break; + } + } + else { + switch (Size) { + case 1: + MemSetElementsInverse(reinterpret_cast(MemData), Value, Length); + break; + case 2: + MemSetElementsInverse(reinterpret_cast(MemData), Value, Length); + break; + case 4: + MemSetElementsInverse(reinterpret_cast(MemData), Value, Length); + break; + case 8: + MemSetElementsInverse(reinterpret_cast(MemData), Value, Length); + break; + default: + LOGMAN_MSG_A_FMT("Unhandled {} size: {}", __func__, Size); + break; + } + } + GD = reinterpret_cast(MemData - (Length * Size)); + } +} + DEF_OP(CacheLineClear) { auto Op = IROp->C(); diff --git a/External/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp b/External/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp index bf5a1a0f6..e2751c6dc 100644 --- a/External/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp +++ b/External/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp @@ -899,6 +899,8 @@ CPUBackend::CompiledCode Arm64JITCore::CompileCode(uint64_t Entry, Op_StoreMemTSO(IROp, ID); } break; + + REGISTER_OP(MEMSET, MemSet); REGISTER_OP(CACHELINECLEAR, CacheLineClear); REGISTER_OP(CACHELINECLEAN, CacheLineClean); REGISTER_OP(CACHELINEZERO, CacheLineZero); diff --git a/External/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h b/External/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h index 780ccb863..3a417c89d 100644 --- a/External/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h +++ b/External/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h @@ -349,6 +349,7 @@ private: DEF_OP(StoreMem); DEF_OP(LoadMemTSO); DEF_OP(StoreMemTSO); + DEF_OP(MemSet); DEF_OP(ParanoidLoadMemTSO); DEF_OP(ParanoidStoreMemTSO); DEF_OP(CacheLineClear); diff --git a/External/FEXCore/Source/Interface/Core/JIT/Arm64/MemoryOps.cpp b/External/FEXCore/Source/Interface/Core/JIT/Arm64/MemoryOps.cpp index 2ec961a90..707c6c0c6 100644 --- a/External/FEXCore/Source/Interface/Core/JIT/Arm64/MemoryOps.cpp +++ b/External/FEXCore/Source/Interface/Core/JIT/Arm64/MemoryOps.cpp @@ -1340,6 +1340,169 @@ DEF_OP(StoreMemTSO) { } } +DEF_OP(MemSet) { + // TODO: A future looking task would be to support this with ARM's MOPS instructions. + // The 8-bit non-atomic forward path directly matches ARM's SETP/SETM/SETE instruction, + // while the backward version needs some fixup to convert it to a forward direction. + // + // Assuming non-atomicity and non-faulting behaviour, this can accelerate this implementation. + // Additionally: This is commonly used as a memset to zero. If we know up-front with an inline constant + // that the value is zero, we can optimize any operation larger than 8-bit down to 8-bit to use the MOPS implementation. + const auto Op = IROp->C(); + + const int32_t Size = Op->Size; + const auto MemReg = GetReg(Op->Addr.ID()); + const auto Value = GetReg(Op->Value.ID()); + const auto Length = GetReg(Op->Length.ID()); + const auto Direction = GetReg(Op->Direction.ID()); + const auto Dst = GetReg(Node); + + // If Direction == 0 then: + // MemReg is incremented (by size) + // else: + // MemReg is decremented (by size) + // + // Counter is decremented regardless. + + ARMEmitter::ForwardLabel BackwardImpl{}; + ARMEmitter::ForwardLabel Done{}; + + mov(TMP1, Length.X()); + if (Op->Prefix.IsInvalid()) { + mov(TMP2, MemReg.X()); + } + else { + const auto Prefix = GetReg(Op->Prefix.ID()); + add(TMP2, Prefix.X(), MemReg.X()); + } + + // Backward or forwards implementation depends on flag + cbnz(ARMEmitter::Size::i64Bit, Direction, &BackwardImpl); + + auto MemStore = [this](auto Value, uint32_t OpSize, int32_t Size) { + switch (OpSize) { + case 1: + strb(Value.W(), TMP2, Size); + break; + case 2: + strh(Value.W(), TMP2, Size); + break; + case 4: + str(Value.W(), TMP2, Size); + break; + case 8: + str(Value.X(), TMP2, Size); + break; + default: + LOGMAN_MSG_A_FMT("Unhandled {} size: {}", __func__, Size); + break; + } + }; + + auto MemStoreTSO = [this](auto Value, uint32_t OpSize, int32_t Size) { + if (OpSize == 1) { + // 8bit load is always aligned to natural alignment + stlrb(Value.W(), TMP2); + } + else { + nop(); + switch (OpSize) { + case 2: + stlrh(Value.W(), TMP2); + break; + case 4: + stlr(Value.W(), TMP2); + break; + case 8: + stlr(Value.X(), TMP2); + break; + default: + LOGMAN_MSG_A_FMT("Unhandled {} size: {}", __func__, Size); + break; + } + nop(); + } + + if (Size >= 0) { + add(ARMEmitter::Size::i64Bit, TMP2, TMP2, OpSize); + } + else { + sub(ARMEmitter::Size::i64Bit, TMP2, TMP2, OpSize); + } + }; + + // Emit forward direction memset then backward direction memset. + for (int32_t Direction : { 1, -1 }) { + const int32_t OpSize = Size; + const int32_t SizeDirection = Size * Direction; + + ARMEmitter::BackwardLabel AgainInternal{}; + ARMEmitter::ForwardLabel DoneInternal{}; + + // Early exit if zero count. + cbz(ARMEmitter::Size::i64Bit, TMP1, &DoneInternal); + + Bind(&AgainInternal); + if (Op->IsAtomic) { + MemStoreTSO(Value, OpSize, SizeDirection); + } + else { + MemStore(Value, OpSize, SizeDirection); + } + sub(ARMEmitter::Size::i64Bit, TMP1, TMP1, 1); + cbnz(ARMEmitter::Size::i64Bit, TMP1, &AgainInternal); + + Bind(&DoneInternal); + + if (SizeDirection >= 0) { + switch (OpSize) { + case 1: + add(Dst.X(), MemReg.X(), Length.X()); + break; + case 2: + add(Dst.X(), MemReg.X(), Length.X(), ARMEmitter::ShiftType::LSL, 1); + break; + case 4: + add(Dst.X(), MemReg.X(), Length.X(), ARMEmitter::ShiftType::LSL, 2); + break; + case 8: + add(Dst.X(), MemReg.X(), Length.X(), ARMEmitter::ShiftType::LSL, 3); + break; + default: + LOGMAN_MSG_A_FMT("Unhandled {} size: {}", __func__, OpSize); + break; + } + } + else { + switch (OpSize) { + case 1: + sub(Dst.X(), MemReg.X(), Length.X()); + break; + case 2: + sub(Dst.X(), MemReg.X(), Length.X(), ARMEmitter::ShiftType::LSL, 1); + break; + case 4: + sub(Dst.X(), MemReg.X(), Length.X(), ARMEmitter::ShiftType::LSL, 2); + break; + case 8: + sub(Dst.X(), MemReg.X(), Length.X(), ARMEmitter::ShiftType::LSL, 3); + break; + default: + LOGMAN_MSG_A_FMT("Unhandled {} size: {}", __func__, OpSize); + break; + } + } + + if (Direction == 1) { + b(&Done); + Bind(&BackwardImpl); + } + } + + Bind(&Done); + // Destination already set to the final pointer. +} + DEF_OP(ParanoidLoadMemTSO) { const auto Op = IROp->C(); const auto OpSize = IROp->Size; diff --git a/External/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h b/External/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h index 4ccbba1a2..46fa1ef2a 100644 --- a/External/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h +++ b/External/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h @@ -344,6 +344,7 @@ private: DEF_OP(StoreFlag); DEF_OP(LoadMem); DEF_OP(StoreMem); + DEF_OP(MemSet); DEF_OP(CacheLineClear); DEF_OP(CacheLineClean); DEF_OP(CacheLineZero); diff --git a/External/FEXCore/Source/Interface/Core/JIT/x86_64/MemoryOps.cpp b/External/FEXCore/Source/Interface/Core/JIT/x86_64/MemoryOps.cpp index df81b2059..d5892ae00 100644 --- a/External/FEXCore/Source/Interface/Core/JIT/x86_64/MemoryOps.cpp +++ b/External/FEXCore/Source/Interface/Core/JIT/x86_64/MemoryOps.cpp @@ -766,6 +766,94 @@ DEF_OP(StoreMem) { } } +DEF_OP(MemSet) { + const auto Op = IROp->C(); + + const int32_t Size = Op->Size; + const auto MemReg = GetSrc(Op->Addr.ID()); + const auto Value = GetSrc(Op->Value.ID()); + const auto Length = GetSrc(Op->Length.ID()); + const auto Direction = GetSrc(Op->Direction.ID()); + const auto Dst = GetSrc(Node); + + // If Direction == 0 then: + // MemReg is incremented (by size) + // else: + // MemReg is decremented (by size) + // + // Counter is decremented regardless. + + // TMP1 = rax + // TMP2 = rcx + // TMP4 = rdi + // That leaves us with TMP3 and TMP5 + mov(rax, Value); + mov(rcx, Length); + mov(rdi, MemReg); + + { + mov(TMP3, Length); + auto CalculateDest = [&]() { + mov(Dst, MemReg); + switch (Size) { + case 1: + break; + case 2: + shl(TMP3, 1); + break; + case 4: + shl(TMP3, 2); + break; + case 8: + shl(TMP3, 3); + break; + default: + LOGMAN_MSG_A_FMT("Unhandled {} size: {}", __func__, Size); + break; + } + }; + + Label AfterDir; + Label BackwardDir; + + cmp(Direction, 0); + jne(BackwardDir); + // Incrementing DF flag. + cld(); + CalculateDest(); + add(Dst, TMP3); + jmp(AfterDir); + + L(BackwardDir); + // Decrementing DF flag. + std(); + CalculateDest(); + sub(Dst, TMP3); + + L(AfterDir); + } + + switch (Size) { + case 1: + rep(); stosb(); + break; + case 2: + rep(); stosw(); + break; + case 4: + rep(); stosd(); + break; + case 8: + rep(); stosq(); + break; + default: + LOGMAN_MSG_A_FMT("Unhandled {} size: {}", __func__, Size); + break; + } + // Ensure we set DF back to zero. Required by the ABI. + cld(); +} + DEF_OP(CacheLineClear) { auto Op = IROp->C(); @@ -820,6 +908,7 @@ void X86JITCore::RegisterMemoryHandlers() { REGISTER_OP(STOREMEM, StoreMem); REGISTER_OP(LOADMEMTSO, LoadMem); REGISTER_OP(STOREMEMTSO, StoreMem); + REGISTER_OP(MEMSET, MemSet); REGISTER_OP(CACHELINECLEAR, CacheLineClear); REGISTER_OP(CACHELINECLEAN, CacheLineClean); REGISTER_OP(CACHELINEZERO, CacheLineZero); diff --git a/External/FEXCore/Source/Interface/IR/IR.json b/External/FEXCore/Source/Interface/IR/IR.json index 9785a5246..4aa5ede74 100644 --- a/External/FEXCore/Source/Interface/IR/IR.json +++ b/External/FEXCore/Source/Interface/IR/IR.json @@ -476,6 +476,14 @@ ] }, + "GPR = MemSet i1:$IsAtomic, u8:$Size, GPR:$Prefix, GPR:$Addr, GPR:$Value, GPR:$Length, GPR:$Direction": { + "Desc": ["Duplicates behaviour of x86 STOS repeat", + "Returns the final address that gets generated without the prefix appended." + ], + "HasSideEffects": true, + "DestSize": "8" + }, + "CacheLineClear GPR:$Addr, i1:$Serialize": { "Desc": ["Does a 64 byte cacheline clear at the address specified", "Only clears the data cachelines. Doesn't do any zeroing", From 7e665080163803786181d3f27a0fff87604f908f Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Thu, 2 Mar 2023 05:04:25 -0800 Subject: [PATCH 2/2] OpcodeDispatcher: Optimize REP STOS to MemSet operation x86's REP STOS instruction is a memset (with element size!) with the ability to choose a direction of execution. Additionally it has a feature where if it faults part-way through the copy, an application can catch the fault and continue afterwards to know how many bytes got copied. RCX is the counter which decrements for each element, and RDI is the memory pointer. On fault these will reflect the last location that was attempted to be written. FEX doesn't support this behaviour which makes our lives easier. Without supporting that feature, this turns in to a directional memset by element size. Let's remove all the multiple blocks and just emit a single IR operation to improve performance of the JIT. Our generated code here was terrible, the IR was terrible, multiblock is always slow with RA. Just a general overall improvement. With profiling pressure-vessel this change deletes the hottest block that appeared in the trace. This instruction is very commonly used for memsetting a region to zero so it should be quite fast. We can also optimize REP MOVS in the future with a Memcpy IR operation in a similar fashion. Additionally in the future these can be optimized to use ARM's new MOPS instructions since the most common case is memset by byte. Which is when we should expose the "Fast REP STOS" CPUID bit. Both setp/setm/sete and cpyfp/cpyfm/cpyfe match `REP STOS` and `REP MOVS` respectively. --- .../Interface/Core/OpcodeDispatcher.cpp | 112 +++++------------- .../Source/Interface/Core/OpcodeDispatcher.h | 2 + 2 files changed, 31 insertions(+), 83 deletions(-) diff --git a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index 430ae4250..3a577bfb9 100644 --- a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -3793,73 +3793,21 @@ void OpDispatchBuilder::STOSOp(OpcodeArgs) { StoreGPRRegister(X86State::REG_RDI, TailDest); } else { - // Calculate deffered flags. - // This block is ending and it needs flag status - CalculateDeferredFlags(); + // FEX doesn't support partial faulting REP instructions. + // Converting this to a `MemSet` IR op optimizes this quite significantly in our codegen. + // If FEX is to gain support for faulting REP instructions, then this implementation needs to change significantly. + OrderedNode *Src = LoadSource(GPRClass, Op, Op->Src[0], Op->Flags, -1); + OrderedNode *Dest = LoadGPRRegister(X86State::REG_RDI); - // Create all our blocks - auto LoopHead = CreateNewCodeBlockAfter(GetCurrentBlock()); - auto LoopTail = CreateNewCodeBlockAfter(LoopHead); - auto LoopEnd = CreateNewCodeBlockAfter(LoopTail); + // Only ES prefix + auto Segment = GetSegment(0, FEXCore::X86Tables::DecodeFlags::FLAG_ES_PREFIX, true); - // At the time this was written, our RA can't handle accessing nodes across blocks. - // So we need to re-load and re-calculate essential values each iteration of the loop. - - // First thing we need to do is finish this block and jump to the start of the loop. - - // RA can now better allocate things, move these ops before the header, to avoid accessing - // DF on every iteration - auto SizeConst = _Constant(Size); - auto NegSizeConst = _Constant(-Size); - - // Calculate direction. + OrderedNode *Counter = LoadGPRRegister(X86State::REG_RCX); auto DF = GetRFLAG(FEXCore::X86State::RFLAG_DF_LOC); - auto PtrDir = _Select(FEXCore::IR::COND_EQ, - DF, _Constant(0), - SizeConst, NegSizeConst); - _Jump(LoopHead); - - SetCurrentCodeBlock(LoopHead); - { - OrderedNode *Counter = LoadGPRRegister(X86State::REG_RCX); - - // Can we end the block? - _CondJump(Counter, LoopEnd, LoopTail, {COND_EQ}); - } - - SetCurrentCodeBlock(LoopTail); - { - OrderedNode *Src = LoadSource(GPRClass, Op, Op->Src[0], Op->Flags, -1); - OrderedNode *Dest = LoadGPRRegister(X86State::REG_RDI); - - // Only ES prefix - Dest = AppendSegmentOffset(Dest, 0, FEXCore::X86Tables::DecodeFlags::FLAG_ES_PREFIX, true); - - // Store to memory where RDI points - _StoreMemAutoTSO(GPRClass, Size, Dest, Src, Size); - - OrderedNode *TailCounter = LoadGPRRegister(X86State::REG_RCX); - OrderedNode *TailDest = LoadGPRRegister(X86State::REG_RDI); - - // Decrement counter - TailCounter = _Sub(TailCounter, _Constant(1)); - - // Store the counter so we don't have to deal with PHI here - StoreGPRRegister(X86State::REG_RCX, TailCounter); - - - // Offset the pointer - TailDest = _Add(TailDest, PtrDir); - StoreGPRRegister(X86State::REG_RDI, TailDest); - - // Jump back to the start, we have more work to do - _Jump(LoopHead); - } - - // Make sure to start a new block after ending this one - - SetCurrentCodeBlock(LoopEnd); + auto Result = _MemSet(CTX->IsTSOEnabled(), Size, Segment ?: InvalidNode, Dest, Src, Counter, DF); + StoreGPRRegister(X86State::REG_RCX, _Constant(0)); + StoreGPRRegister(X86State::REG_RDI, Result); } } @@ -4832,20 +4780,19 @@ uint32_t OpDispatchBuilder::GetDstBitSize(X86Tables::DecodedOp Op) const { return GetDstSize(Op) * 8; } -OrderedNode *OpDispatchBuilder::AppendSegmentOffset(OrderedNode *Value, uint32_t Flags, uint32_t DefaultPrefix, bool Override) { +OrderedNode *OpDispatchBuilder::GetSegment(uint32_t Flags, uint32_t DefaultPrefix, bool Override) { const uint8_t GPRSize = CTX->GetGPRSize(); if (CTX->Config.Is64BitMode) { if (Flags & FEXCore::X86Tables::DecodeFlags::FLAG_FS_PREFIX) { - Value = _Add(Value, _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, fs_cached))); + return _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, fs_cached)); } else if (Flags & FEXCore::X86Tables::DecodeFlags::FLAG_GS_PREFIX) { - Value = _Add(Value, _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, gs_cached))); + return _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, gs_cached)); } // If there was any other segment in 64bit then it is ignored } else { - OrderedNode *Segment{}; uint32_t Prefix = Flags & FEXCore::X86Tables::DecodeFlags::FLAG_SEGMENTS; if (!Prefix || Override) { // If there was no prefix then use the default one if available @@ -4855,29 +4802,28 @@ OrderedNode *OpDispatchBuilder::AppendSegmentOffset(OrderedNode *Value, uint32_t // With the segment register optimization we store the GDT bases directly in the segment register to remove indexed loads switch (Prefix) { case FEXCore::X86Tables::DecodeFlags::FLAG_ES_PREFIX: - Segment = _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, es_cached)); - break; + return _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, es_cached)); case FEXCore::X86Tables::DecodeFlags::FLAG_CS_PREFIX: - Segment = _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, cs_cached)); - break; + return _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, cs_cached)); case FEXCore::X86Tables::DecodeFlags::FLAG_SS_PREFIX: - Segment = _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, ss_cached)); - break; + return _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, ss_cached)); case FEXCore::X86Tables::DecodeFlags::FLAG_DS_PREFIX: - Segment = _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, ds_cached)); - break; + return _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, ds_cached)); case FEXCore::X86Tables::DecodeFlags::FLAG_FS_PREFIX: - Segment = _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, fs_cached)); - break; + return _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, fs_cached)); case FEXCore::X86Tables::DecodeFlags::FLAG_GS_PREFIX: - Segment = _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, gs_cached)); - break; - default: break; // Do nothing + return _LoadContext(GPRSize, GPRClass, offsetof(FEXCore::Core::CPUState, gs_cached)); + default: + break; // Do nothing } + } + return nullptr; +} - if (Segment) { - Value = _Add(Value, Segment); - } +OrderedNode *OpDispatchBuilder::AppendSegmentOffset(OrderedNode *Value, uint32_t Flags, uint32_t DefaultPrefix, bool Override) { + auto Segment = GetSegment(Flags, DefaultPrefix, Override); + if (Segment) { + Value = _Add(Value, Segment); } return Value; diff --git a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.h b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.h index cef31c8ff..3eaa56faf 100644 --- a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.h +++ b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.h @@ -889,6 +889,8 @@ private: #undef OpcodeArgs OrderedNode *AppendSegmentOffset(OrderedNode *Value, uint32_t Flags, uint32_t DefaultPrefix = 0, bool Override = false); + OrderedNode *GetSegment(uint32_t Flags, uint32_t DefaultPrefix = 0, bool Override = false); + void UpdatePrefixFromSegment(OrderedNode *Segment, uint32_t SegmentReg); enum class MemoryAccessType {