From 70b6bc2baeaebb76a8221843378b6ef457fd458f Mon Sep 17 00:00:00 2001 From: Paulo Matos Date: Mon, 27 Oct 2025 21:21:48 +0100 Subject: [PATCH] Remove InterpretAsFloat from x87StackOptimizationPass The InterpretAsFloat was never properly made use of. There's a couple of issues that are fixed more easily with this gone, so lets remove it. If there's a specific optimization that requires this, we can bring it back at a later time. This should not have any effect on the current code generation. --- .../Interface/Core/OpcodeDispatcher/X87.cpp | 15 +++++++------ .../Core/OpcodeDispatcher/X87F64.cpp | 12 +++++------ FEXCore/Source/Interface/IR/IR.json | 15 +++++-------- .../IR/Passes/x87StackOptimizationPass.cpp | 21 ++++++++++--------- 4 files changed, 29 insertions(+), 34 deletions(-) diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher/X87.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher/X87.cpp index 657f86ffb..96f61c197 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher/X87.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher/X87.cpp @@ -17,7 +17,6 @@ $end_info$ #include #include -#include #include #include @@ -69,7 +68,7 @@ void OpDispatchBuilder::FLD(OpcodeArgs, IR::OpSize Width) { if (Width == OpSize::i32Bit || Width == OpSize::i64Bit) { ConvertedData = _F80CVTTo(Data, ReadWidth); } - _PushStack(ConvertedData, Data, ReadWidth, true); + _PushStack(ConvertedData, Data, ReadWidth); } // Float LoaD operation with memory operand @@ -81,7 +80,7 @@ void OpDispatchBuilder::FBLD(OpcodeArgs) { // Read from memory Ref Data = LoadSourceFPR_WithOpSize(Op, Op->Src[0], OpSize::f80Bit, Op->Flags); Ref ConvertedData = _F80BCDLoad(Data); - _PushStack(ConvertedData, Data, OpSize::i128Bit, true); + _PushStack(ConvertedData, Data, OpSize::i128Bit); } void OpDispatchBuilder::FBSTP(OpcodeArgs) { @@ -93,7 +92,7 @@ void OpDispatchBuilder::FBSTP(OpcodeArgs) { void OpDispatchBuilder::FLD_Const(OpcodeArgs, NamedVectorConstant K) { // Update TOP Ref Data = LoadAndCacheNamedVectorConstant(OpSize::i128Bit, K); - _PushStack(Data, Data, OpSize::i128Bit, true); + _PushStack(Data, Data, OpSize::i128Bit); } void OpDispatchBuilder::FILD(OpcodeArgs) { @@ -124,7 +123,7 @@ void OpDispatchBuilder::FILD(OpcodeArgs) { auto upper = _Or(OpSize::i64Bit, sign, zeroed_exponent); Ref ConvertedData = _VLoadTwoGPRs(shifted, upper); - _PushStack(ConvertedData, Data, ReadWidth, false); + _PushStack(ConvertedData, Invalid(), ReadWidth); } void OpDispatchBuilder::FST(OpcodeArgs, IR::OpSize Width) { @@ -132,7 +131,7 @@ void OpDispatchBuilder::FST(OpcodeArgs, IR::OpSize Width) { AddressMode A = DecodeAddress(Op, Op->Dest, MemoryAccessType::DEFAULT, false); A = SelectAddressMode(this, A, GetGPROpSize(), CTX->HostFeatures.SupportsTSOImm9, false, false, Width); - _StoreStackMem(SourceSize, Width, A.Base, A.Index, OpSize::iInvalid, A.IndexType, A.IndexScale, /*Float=*/true); + _StoreStackMem(SourceSize, Width, A.Base, A.Index, OpSize::iInvalid, A.IndexType, A.IndexScale); if (Op->TableInfo->Flags & X86Tables::InstFlags::FLAGS_POP) { _PopStackDestroy(); @@ -878,8 +877,8 @@ void OpDispatchBuilder::X87FXTRACT(OpcodeArgs) { _PopStackDestroy(); auto Exp = _F80XTRACT_EXP(Top); auto Sig = _F80XTRACT_SIG(Top); - _PushStack(Exp, Exp, OpSize::f80Bit, true); - _PushStack(Sig, Sig, OpSize::f80Bit, true); + _PushStack(Exp, Invalid(), OpSize::f80Bit); + _PushStack(Sig, Invalid(), OpSize::f80Bit); } } // namespace FEXCore::IR diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher/X87F64.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher/X87F64.cpp index b3162eab6..3b92cc00a 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher/X87F64.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher/X87F64.cpp @@ -68,7 +68,7 @@ void OpDispatchBuilder::FLDF64(OpcodeArgs, IR::OpSize Width) { } else if (Width == OpSize::f80Bit) { ConvertedData = _F80CVT(OpSize::i64Bit, Data); } - _PushStack(ConvertedData, Data, ReadWidth, true); + _PushStack(ConvertedData, Data, ReadWidth); } void OpDispatchBuilder::FBLDF64(OpcodeArgs) { @@ -76,7 +76,7 @@ void OpDispatchBuilder::FBLDF64(OpcodeArgs) { Ref Data = LoadSourceFPR_WithOpSize(Op, Op->Src[0], OpSize::f80Bit, Op->Flags); Ref ConvertedData = _F80BCDLoad(Data); ConvertedData = _F80CVT(OpSize::i64Bit, ConvertedData); - _PushStack(ConvertedData, Data, OpSize::i64Bit, true); + _PushStack(ConvertedData, Data, OpSize::i64Bit); } void OpDispatchBuilder::FBSTPF64(OpcodeArgs) { @@ -88,7 +88,7 @@ void OpDispatchBuilder::FBSTPF64(OpcodeArgs) { void OpDispatchBuilder::FLDF64_Const(OpcodeArgs, uint64_t Num) { auto Data = _VCastFromGPR(OpSize::i64Bit, OpSize::i64Bit, Constant(Num)); - _PushStack(Data, Data, OpSize::i64Bit, true); + _PushStack(Data, Data, OpSize::i64Bit); } void OpDispatchBuilder::FILDF64(OpcodeArgs) { @@ -100,7 +100,7 @@ void OpDispatchBuilder::FILDF64(OpcodeArgs) { Data = _Sbfe(OpSize::i64Bit, IR::OpSizeAsBits(ReadWidth), 0, Data); } auto ConvertedData = _Float_FromGPR_S(OpSize::i64Bit, ReadWidth == OpSize::i32Bit ? OpSize::i32Bit : OpSize::i64Bit, Data); - _PushStack(ConvertedData, Data, ReadWidth, false); + _PushStack(ConvertedData, Invalid(), ReadWidth); } void OpDispatchBuilder::FISTF64(OpcodeArgs, bool Truncate) { @@ -397,7 +397,7 @@ void OpDispatchBuilder::X87FXTRACTF64(OpcodeArgs) { Ref Exp = _NZCVSelectV(OpSize::i64Bit, CondClass::EQ, ExpZV, ExpNZV); _PopStackDestroy(); - _PushStack(Exp, Exp, OpSize::i64Bit, true); - _PushStack(Sig, Sig, OpSize::i64Bit, true); + _PushStack(Exp, Invalid(), OpSize::i64Bit); + _PushStack(Sig, Invalid(), OpSize::i64Bit); } } // namespace FEXCore::IR diff --git a/FEXCore/Source/Interface/IR/IR.json b/FEXCore/Source/Interface/IR/IR.json index 0405bcdb8..0a02cbd0f 100644 --- a/FEXCore/Source/Interface/IR/IR.json +++ b/FEXCore/Source/Interface/IR/IR.json @@ -2818,17 +2818,13 @@ "X87": true, "HasSideEffects": true }, - "PushStack FPR:$X80Src, SSA:$OriginalValue, OpSize:$LoadSize, i1:$Float": { + "PushStack FPR:$X80Src, FPR:$OriginalValue, OpSize:$LoadSize": { "Desc": [ "Pushes the provided X80Src source on to the x87 stack.", - "Tracks OriginalValue as the original value of X80Src.", + "Tracks OriginalValue as the original value of X80Src. OriginalValue can be Invalid() in which case no tracking is done.", "Opsize is 128bit for F80 values, 64-bit for low precision.", "LoadSize the original load size, i.e. of size of OriginalValue.", - "Float: 80-bit, 64-bit, 32-bit", - "Int: 64-bit, 32-bit, 16-bit" - ], - "EmitValidation": [ - "WalkFindRegClass($OriginalValue) == RegClass::FPR || WalkFindRegClass($OriginalValue) == RegClass::GPR" + "Float: 80-bit, 64-bit, 32-bit" ], "HasSideEffects": true, "X87": true @@ -2840,13 +2836,12 @@ "HasSideEffects": true, "X87": true }, - "StoreStackMem OpSize:$SourceSize, OpSize:$StoreSize, GPR:$Addr, GPR:$Offset, OpSize:$Align, MemOffsetType:$OffsetType, u8:$OffsetScale, i1:$Float": { + "StoreStackMem OpSize:$SourceSize, OpSize:$StoreSize, GPR:$Addr, GPR:$Offset, OpSize:$Align, MemOffsetType:$OffsetType, u8:$OffsetScale": { "Desc": [ "Takes the top value off the x87 stack and stores it to memory.", "SourceSize is 128bit for F80 values, 64-bit for low precision.", "StoreSize is the store size for conversion:", - "Float: 80-bit, 64-bit, or 32-bit", - "Int: 64-bit, 32-bit, 16-bit" + "Float: 80-bit, 64-bit, or 32-bit" ], "HasSideEffects": true, "X87": true diff --git a/FEXCore/Source/Interface/IR/Passes/x87StackOptimizationPass.cpp b/FEXCore/Source/Interface/IR/Passes/x87StackOptimizationPass.cpp index 74a67ea1e..db276ab76 100644 --- a/FEXCore/Source/Interface/IR/Passes/x87StackOptimizationPass.cpp +++ b/FEXCore/Source/Interface/IR/Passes/x87StackOptimizationPass.cpp @@ -6,7 +6,6 @@ #include "Interface/IR/PassManager.h" #include "FEXCore/IR/IR.h" #include "FEXCore/Utils/Profiler.h" -#include "FEXCore/Utils/MathUtils.h" #include "FEXCore/Core/HostFeatures.h" #include "Interface/Core/Addressing.h" @@ -293,10 +292,9 @@ private: StackMemberInfo() {} StackMemberInfo(Ref Data) : StackDataNode(Data) {} - StackMemberInfo(Ref Data, Ref Source, OpSize Size, bool Float) + StackMemberInfo(Ref Data, Ref Source, OpSize Size) : StackDataNode(Data) - , Source({Size, Source}) - , InterpretAsFloat(Float) {} + , Source({Size, Source}) {} Ref StackDataNode {}; // Reference to the data in the Stack. // This is the source data node in the stack format, possibly converted to 64/80 bits. struct StackMemberData final { @@ -306,7 +304,6 @@ private: // Tuple is only valid if we have information about the Source of the Stack Data Node. // In it's valid then OpSize is the original source size and Ref is the original source node. std::optional Source {}; - bool InterpretAsFloat {false}; // True if this is a floating point value, false if integer }; // StackData, TopCache need to be always properly set to ensure @@ -927,8 +924,13 @@ void X87StackOptimization::Run(IREmitter* Emit) { StoreStackValueAtOffset_Slow(SourceNode); } else { auto* SourceNode = CurrentIR.GetNode(Op->X80Src); - auto* OriginalNode = CurrentIR.GetNode(Op->OriginalValue); - StackData.push(StackMemberInfo {SourceNode, OriginalNode, Op->LoadSize, Op->Float}); + if (Op->OriginalValue.IsInvalid()) { + // No original value to track - just push the converted data + StackData.push(StackMemberInfo {SourceNode}); + } else { + auto* OriginalNode = CurrentIR.GetNode(Op->OriginalValue); + StackData.push(StackMemberInfo {SourceNode, OriginalNode, Op->LoadSize}); + } } break; } @@ -993,9 +995,8 @@ void X87StackOptimization::Run(IREmitter* Emit) { // str w2, [x1] // or similar. As long as the source size and dest size are one and the same. // This will avoid any conversions between source and stack element size and conversion back. - if (!SlowPath && Value->Source && Value->Source->Size == Op->StoreSize && Value->InterpretAsFloat) { - const auto ClassType = Value->InterpretAsFloat ? RegClass::FPR : RegClass::GPR; - IREmit->_StoreMem(ClassType, Op->StoreSize, Value->Source->Node, AddrNode, Offset, Align, OffsetType, OffsetScale); + if (!SlowPath && Value->Source && Value->Source->Size == Op->StoreSize) { + IREmit->_StoreMemFPR(Op->StoreSize, Value->Source->Node, AddrNode, Offset, Align, OffsetType, OffsetScale); break; }