From 5cc6eff62ce75b4f2d41886c905d0af7fdcf414e Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sun, 3 Sep 2023 02:04:22 -0700 Subject: [PATCH] OpcodeDispatcher: Remove final assumptions about small IR operating sizes With Or, Orlshl, Bfe, and Bfi there were some assumptions made that i8 and i16 operations made sense. Which required us to disable the IR validation for these operations when it was just added. This removes the final assumptions about these IR operations supporting these small operating sizes allowing us to enable the IR validation. Also a very minor optimization by moving a couple extracts from source before trying to BFI from it, making RA more optimal. --- .../Interface/Core/OpcodeDispatcher.cpp | 24 +++++++++---------- .../Interface/Core/OpcodeDispatcher/Flags.cpp | 2 +- FEXCore/Source/Interface/IR/IR.json | 20 ++++------------ 3 files changed, 17 insertions(+), 29 deletions(-) diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index 27899184c..3027d3020 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -2571,19 +2571,18 @@ void OpDispatchBuilder::RCROp8x1Bit(OpcodeArgs) { CalculateDeferredFlags(); OrderedNode *Dest = LoadSource(GPRClass, Op, Op->Dest, Op->Flags, -1); - const auto Size = GetSrcSize(Op); - const auto SizeBit = Size * 8; + const auto SizeBit = GetSrcBitSize(Op); auto CF = GetRFLAG(FEXCore::X86State::RFLAG_CF_LOC); uint32_t Shift = 1; - // Rotate and insert CF in the upper bit - OrderedNode *Res = _Bfe(OpSize::i32Bit, 7, 1, Dest); - Res = _Bfi(IR::SizeToOpSize(Size), 1, 7, Res, CF); - // Our new CF will be bit (Shift - 1) of the source auto NewCF = _Bfe(OpSize::i32Bit, 1, Shift - 1, Dest); + // Rotate and insert CF in the upper bit + OrderedNode *Res = _Bfe(OpSize::i32Bit, 7, 1, Dest); + Res = _Bfi(OpSize::i32Bit, 1, 7, Res, CF); + StoreResult(GPRClass, Op, Res, -1); SetRFLAG(NewCF); @@ -2761,16 +2760,17 @@ void OpDispatchBuilder::RCLOp1Bit(OpcodeArgs) { OrderedNode *Dest = LoadSource(GPRClass, Op, Op->Dest, Op->Flags, -1); const auto Size = GetSrcBitSize(Op); + const auto OpSize = Size == 64 ? OpSize::i64Bit : OpSize::i32Bit; auto CF = GetRFLAG(FEXCore::X86State::RFLAG_CF_LOC); uint32_t Shift = 1; - // Rotate left and insert CF in to lowest bit - OrderedNode *Res = _Lshl(Size == 64 ? OpSize::i64Bit : OpSize::i32Bit, Dest, _Constant(Size, 1)); - Res = _Or(IR::SizeToOpSize(std::max(4u, std::max(GetOpSize(Res), GetOpSize(CF)))), Res, CF); - // Our new CF will be the top bit of the source - auto NewCF = _Bfe(OpSizeFromDst(Op), 1, Size - 1, Dest); + auto NewCF = _Bfe(OpSize, 1, Size - 1, Dest); + + // Rotate left and insert CF in to lowest bit + OrderedNode *Res = _Lshl(OpSize, Dest, _Constant(Size, 1)); + Res = _Or(OpSize, Res, CF); StoreResult(GPRClass, Op, Res, -1); @@ -2779,7 +2779,7 @@ void OpDispatchBuilder::RCLOp1Bit(OpcodeArgs) { if (Shift == 1) { // OF is the top two MSBs XOR'd together // Top two MSBs is CF and top bit of result - SetRFLAG(_Xor(IR::SizeToOpSize(std::max(4u, GetOpSize(Res))), _Bfe(IR::SizeToOpSize(std::max(4u, GetOpSize(Res))), 1, Size - 1, Res), NewCF)); + SetRFLAG(_Xor(OpSize::i32Bit, _Bfe(OpSize, 1, Size - 1, Res), NewCF)); } } diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp index f56406e1e..1e8c18779 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp @@ -1181,7 +1181,7 @@ void OpDispatchBuilder::CalculateFlags_TZCNT(OrderedNode *Src) { // Set flags SetRFLAG(ZFResult); - SetRFLAG(_Bfe(IR::SizeToOpSize(GetOpSize(Src)), 1, 0, Src)); + SetRFLAG(_Bfe(OpSize::i32Bit, 1, 0, Src)); } void OpDispatchBuilder::CalculateFlags_LZCNT(uint8_t SrcSize, OrderedNode *Src) { diff --git a/FEXCore/Source/Interface/IR/IR.json b/FEXCore/Source/Interface/IR/IR.json index 5f6600419..9fbcb60fa 100644 --- a/FEXCore/Source/Interface/IR/IR.json +++ b/FEXCore/Source/Interface/IR/IR.json @@ -923,23 +923,17 @@ "GPR = Or OpSize:#Size, GPR:$Src1, GPR:$Src2": { "Desc": ["Integer binary or" ], - "Comment": [ - "TODO: There is an assumption that Or of operating size < 32-bit makes sense" - ], "DestSize": "Size", "EmitValidation": [ - "true || Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit" + "Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit" ] }, "GPR = Orlshl OpSize:#Size, GPR:$Src1, GPR:$Src2, u8:$BitShift": { "Desc": ["Integer binary or with logical shift left" ], - "Comment": [ - "TODO: There is an assumption that Orlshl of operating size < 32-bit makes sense" - ], "DestSize": "Size", "EmitValidation": [ - "true || Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit" + "Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit" ] }, "GPR = Orlshr OpSize:#Size, GPR:$Src1, GPR:$Src2, u8:$BitShift": { @@ -1084,12 +1078,9 @@ "The source bitfield is from Src[Width:0]", "The bitfield is copied in to Dest[(Width + lsb):lsb]" ], - "Comment": [ - "TODO: There is an assumption that BFI of operating size < 32-bit makes sense" - ], "DestSize": "Size", "EmitValidation": [ - "true || Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit" + "Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit" ] }, "GPR = Bfe OpSize:#Size, u8:$Width, u8:$lsb, GPR:$Src": { @@ -1097,12 +1088,9 @@ "The source bitfield is from Src[Width:0]", "The bitfield is then zero extended" ], - "Comment": [ - "TODO: There is an assumption that BFE of operating size < 32-bit makes sense" - ], "DestSize": "Size", "EmitValidation": [ - "true || Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit" + "Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit" ] }, "GPR = Sbfe OpSize:#Size, u8:$Width, u8:$lsb, GPR:$Src": {