From d58952d1ef9b697b8a2d4731fd191df4443973b2 Mon Sep 17 00:00:00 2001 From: lioncash Date: Fri, 12 Nov 2021 08:41:04 -0500 Subject: [PATCH] OpcodeDispatcher: Make selection flag enum an enum class Makes the enum and member variable strongly typed, so that it's harder to implicitly use incorrect values. --- .../Interface/Core/OpcodeDispatcher.cpp | 17 +++++------- .../Source/Interface/Core/OpcodeDispatcher.h | 26 ++++++++++--------- .../Core/OpcodeDispatcher/Vector.cpp | 2 +- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index a14586449..0c8bee846 100644 --- a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -886,7 +886,7 @@ OrderedNode *OpDispatchBuilder::SelectCC(uint8_t OP, OrderedNode *TrueValue, Ord } // Try folding the flags generation in the select op - if (flagsOp == FLAGS_OP_CMP) { + if (flagsOp == SelectionFlag::CMP) { switch(OP) { // SGT case 0xF: SrcCond = _Select(FEXCore::IR::COND_SGT, flagsOpDestSigned, flagsOpSrcSigned, TrueValue, FalseValue, flagsOpSize); break; @@ -918,13 +918,13 @@ OrderedNode *OpDispatchBuilder::SelectCC(uint8_t OP, OrderedNode *TrueValue, Ord //default: printf("Missed Condition %04X OP_CMP\n", OP); break; } } - else if (flagsOp == FLAGS_OP_AND) { + else if (flagsOp == SelectionFlag::AND) { switch(OP) { case 0x4: SrcCond = _Select(FEXCore::IR::COND_EQ, flagsOpDest, ZeroConst, TrueValue, FalseValue, flagsOpSize); break; case 0x5: SrcCond = _Select(FEXCore::IR::COND_NEQ, flagsOpDest, ZeroConst, TrueValue, FalseValue, flagsOpSize); break; //default: printf("Missed Condition %04X OP_AND\n", OP); break; } - } else if (flagsOp == FLAGS_OP_FCMP) { + } else if (flagsOp == SelectionFlag::FCMP) { /* x86:ZCP unordered { 11 1 } @@ -1253,13 +1253,11 @@ void OpDispatchBuilder::TESTOp(OpcodeArgs) { auto Size = GetDstSize(Op); - if (Size >=4) { - flagsOp = FLAGS_OP_AND; - flagsOpDest = ALUOp; + flagsOp = SelectionFlag::AND; + flagsOpDest = ALUOp; + if (Size >= 4) { flagsOpSize = Size; } else { - flagsOp = FLAGS_OP_AND; - flagsOpDest = ALUOp; flagsOpSize = 4; // assuming ZEXT semantics here } } @@ -1325,14 +1323,13 @@ void OpDispatchBuilder::CMPOp(OpcodeArgs) { GenerateFlags_SUB(Op, Result, Dest, Src); + flagsOp = SelectionFlag::CMP; if (Size >= 4) { flagsOpSize = Size; - flagsOp = FLAGS_OP_CMP; flagsOpDestSigned = flagsOpDest = Dest; flagsOpSrcSigned = flagsOpSrc = Src; } else { flagsOpSize = 4; - flagsOp = FLAGS_OP_CMP; flagsOpDestSigned = _Sext(Size * 8, flagsOpDest = Dest); flagsOpSrcSigned = _Sext(Size * 8, flagsOpSrc = Src); } diff --git a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.h b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.h index 771e40ab6..5dace9b48 100644 --- a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.h +++ b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.h @@ -26,18 +26,20 @@ class OpDispatchBuilder final : public IREmitter { friend class FEXCore::IR::Pass; friend class FEXCore::IR::PassManager; -enum { - FLAGS_OP_NONE, // must rely on x86 flags - FLAGS_OP_CMP, // flags were set by a CMP between flagsOpDest/flagsOpDestSigned and flagsOpSrc/flagsOpSrcSigned with flagsOpSize size - FLAGS_OP_AND, // flags were set by an AND/TEST, flagsOpDest contains the resulting value of flagsOpSize size - FLAGS_OP_FCMP, // flags were set by a ucomis* / comis* +enum class SelectionFlag { + Nothing, // must rely on x86 flags + CMP, // flags were set by a CMP between flagsOpDest/flagsOpDestSigned and flagsOpSrc/flagsOpSrcSigned with flagsOpSize size + AND, // flags were set by an AND/TEST, flagsOpDest contains the resulting value of flagsOpSize size + FCMP, // flags were set by a ucomis* / comis* }; public: - int flagsOp; - uint8_t flagsOpSize; - OrderedNode* flagsOpDest, *flagsOpSrc; - OrderedNode* flagsOpDestSigned, *flagsOpSrcSigned; + SelectionFlag flagsOp{}; + uint8_t flagsOpSize{}; + OrderedNode* flagsOpDest{}; + OrderedNode* flagsOpSrc{}; + OrderedNode* flagsOpDestSigned{}; + OrderedNode* flagsOpSrcSigned{}; FEXCore::Context::Context *CTX{}; bool ShouldDump {false}; @@ -69,7 +71,7 @@ public: } void StartNewBlock() { - flagsOp = FLAGS_OP_NONE; + flagsOp = SelectionFlag::Nothing; } bool FinishOp(uint64_t NextRIP, bool LastOp) { @@ -519,12 +521,12 @@ private: template void SetRFLAG(OrderedNode *Value) { - flagsOp = FLAGS_OP_NONE; + flagsOp = SelectionFlag::Nothing; _StoreFlag(_Bfe(1, 0, Value), BitOffset); } void SetRFLAG(OrderedNode *Value, unsigned BitOffset) { - flagsOp = FLAGS_OP_NONE; + flagsOp = SelectionFlag::Nothing; _StoreFlag(_Bfe(1, 0, Value), BitOffset); } diff --git a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher/Vector.cpp b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher/Vector.cpp index 8749023b8..4c4c3432a 100644 --- a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher/Vector.cpp +++ b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher/Vector.cpp @@ -1443,7 +1443,7 @@ void OpDispatchBuilder::UCOMISxOp(OpcodeArgs) { SetRFLAG(ZeroConst); SetRFLAG(ZeroConst); - flagsOp = FLAGS_OP_FCMP; + flagsOp = SelectionFlag::FCMP; flagsOpDest = Src1; flagsOpSrc = Src2; flagsOpSize = GetSrcSize(Op);