From 40cad15da92cda26f8404cae41d3c9dfc7c4d3a2 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Tue, 10 Sep 2019 18:35:40 -0700 Subject: [PATCH] Cleanup old IR emitter functions Argumentless IR emitter functions were prone to generating invalid code. Remove them from the python emitter and change the branch instructions that were using them to a new version instead. Adds NumUse tracking as well. --- Scripts/json_ir_generator.py | 37 +--------------------- Source/Interface/Core/OpcodeDispatcher.cpp | 31 +++++++----------- Source/Interface/Core/OpcodeDispatcher.h | 16 ++++++++++ 3 files changed, 29 insertions(+), 55 deletions(-) diff --git a/Scripts/json_ir_generator.py b/Scripts/json_ir_generator.py index 7b62a761c..23cd2ee95 100644 --- a/Scripts/json_ir_generator.py +++ b/Scripts/json_ir_generator.py @@ -217,39 +217,6 @@ def print_ir_allocator_helpers(ops, defines): output_file.write("\t\treturn HeaderOp->HasDest;\n") output_file.write("\t}\n\n") - for op_key, op_vals in ops.items(): - if not ("Last" in op_vals): - HasDest = False - HasFixedDestSize = False - FixedDestSize = 0 - HasDestSize = False; - DestSize = "" - - if ("HasDest" in op_vals and op_vals["HasDest"] == True): - HasDest = True - - if ("FixedDestSize" in op_vals): - HasFixedDestSize = True - FixedDestSize = int(op_vals["FixedDestSize"]) - - if ("DestSize" in op_vals): - HasDestSize = True - DestSize = op_vals["DestSize"]; - - output_file.write("\tIRPair _%s() {\n" % (op_key, op_key)) - - output_file.write("\t\tauto Op = AllocateOp();\n" % (op_key, op_key.upper())) - - if (HasDest): - if (HasFixedDestSize): - output_file.write("\t\tOp.first->Header.Size = %d;\n" % FixedDestSize) - - output_file.write("\t\tOp.first->Header.HasDest = true;\n") - - output_file.write("\t\treturn Op;\n") - - output_file.write("\t}\n\n") - # Generate helpers with operands for op_key, op_vals in ops.items(): if not ("Last" in op_vals): @@ -268,9 +235,6 @@ def print_ir_allocator_helpers(ops, defines): if ("Args" in op_vals and len(op_vals["Args"]) != 0): HasArgs = True - if not (HasArgs or SSAArgs != 0): - continue - if ("HelperGen" in op_vals and op_vals["HelperGen"] == False): continue; @@ -316,6 +280,7 @@ def print_ir_allocator_helpers(ops, defines): if (SSAArgs != 0): for i in range(0, SSAArgs): output_file.write("\t\tOp.first->Header.Args[%d] = ssa%d->Wrapped(ListData.Begin());\n" % (i, i)) + output_file.write("\t\tssa%d->AddUse();\n" % (i)) if (HasArgs): for i in range(1, len(op_vals["Args"]), 2): diff --git a/Source/Interface/Core/OpcodeDispatcher.cpp b/Source/Interface/Core/OpcodeDispatcher.cpp index ea3a6c835..0ec9fe55c 100644 --- a/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/Source/Interface/Core/OpcodeDispatcher.cpp @@ -517,9 +517,7 @@ void OpDispatchBuilder::CondJUMPOp(OpcodeArgs) { // XXX: Test GetPackedRFLAG(false); - auto CondJump = _CondJump(); - CondJump.first->Header.NumArgs = 1; - CondJump.first->Header.Args[0] = SrcCond.Node->Wrapped(ListData.Begin()); + auto CondJump = _CondJump(SrcCond); auto RIPOffset = LoadSource(Op, Op->Src1, Op->Flags); auto RIPTargetConst = _Constant(Op->PC + Op->InstSize); @@ -535,7 +533,7 @@ void OpDispatchBuilder::CondJUMPOp(OpcodeArgs) { // Make sure to start a new block after ending this one auto JumpTarget = _BeginBlock(); // This very explicitly avoids the isDest path for Ops. We want the actual destination here - CondJump.first->Header.Args[1] = JumpTarget.Node->Wrapped(ListData.Begin()); + SetJumpTarget(CondJump, JumpTarget); } } @@ -1551,7 +1549,7 @@ void OpDispatchBuilder::STOSOp(OpcodeArgs) { // Make sure to start a new block after ending this one auto LoopStart = _BeginBlock(); - JumpStart.first->Header.Args[0] = LoopStart.Node->Wrapped(ListData.Begin()); + SetJumpTarget(JumpStart, LoopStart); OrderedNode *Counter = _LoadContext(8, offsetof(FEXCore::Core::CPUState, gregs[FEXCore::X86State::REG_RCX])); @@ -1564,9 +1562,7 @@ void OpDispatchBuilder::STOSOp(OpcodeArgs) { auto CanLeaveCond = _Select(FEXCore::IR::COND_EQ, Counter, ZeroConst, OneConst, ZeroConst); - auto CondJump = _CondJump(); - CondJump.first->Header.NumArgs = 1; - CondJump.first->Header.Args[0] = CanLeaveCond.Node->Wrapped(ListData.Begin()); + auto CondJump = _CondJump(CanLeaveCond); // Decrement counter Counter = _Sub(Counter, OneConst); @@ -1583,7 +1579,7 @@ void OpDispatchBuilder::STOSOp(OpcodeArgs) { _EndBlock(0); // Make sure to start a new block after ending this one auto LoopEnd = _BeginBlock(); - CondJump.first->Header.Args[1] = LoopEnd.Node->Wrapped(ListData.Begin()); + SetJumpTarget(CondJump, LoopEnd); } void OpDispatchBuilder::MOVSOp(OpcodeArgs) { @@ -1615,7 +1611,7 @@ void OpDispatchBuilder::CMPSOp(OpcodeArgs) { _EndBlock(0); // Make sure to start a new block after ending this one auto LoopStart = _BeginBlock(); - JumpStart.first->Header.Args[0] = LoopStart.Node->Wrapped(ListData.Begin()); + SetJumpTarget(JumpStart, LoopStart); OrderedNode *Counter = _LoadContext(8, offsetof(FEXCore::Core::CPUState, gregs[FEXCore::X86State::REG_RCX])); OrderedNode *Dest_RDI = _LoadContext(8, offsetof(FEXCore::Core::CPUState, gregs[FEXCore::X86State::REG_RDI])); @@ -1631,9 +1627,7 @@ void OpDispatchBuilder::CMPSOp(OpcodeArgs) { auto CanLeaveCond = _Select(FEXCore::IR::COND_EQ, Counter, ZeroConst, OneConst, ZeroConst); - auto CondJump = _CondJump(); - CondJump.first->Header.NumArgs = 1; - CondJump.first->Header.Args[0] = CanLeaveCond.Node->Wrapped(ListData.Begin()); + auto CondJump = _CondJump(CanLeaveCond); // Decrement counter Counter = _Sub(Counter, OneConst); @@ -1654,7 +1648,7 @@ void OpDispatchBuilder::CMPSOp(OpcodeArgs) { _EndBlock(0); // Make sure to start a new block after ending this one auto LoopEnd = _BeginBlock(); - CondJump.first->Header.Args[1] = LoopEnd.Node->Wrapped(ListData.Begin()); + SetJumpTarget(CondJump, LoopEnd); } @@ -2469,7 +2463,7 @@ void OpDispatchBuilder::ResetWorkingList() { ListData.Reset(); CurrentWriteCursor = nullptr; // This is necessary since we do "null" pointer checks - ListData.Allocate(sizeof(OrderedNode)); + InvalidNode = reinterpret_cast(ListData.Allocate(sizeof(OrderedNode))); DecodeFailure = false; Information.HadUnconditionalExit = false; ShouldDump = false; @@ -3045,16 +3039,15 @@ void OpDispatchBuilder::INTOp(OpcodeArgs) { if (Op->OP == 0xCE) { // Conditional to only break if Overflow == 1 auto Flag = GetRFLAG(FEXCore::X86State::RFLAG_OF_LOC); - auto CondJump = _CondJump(); - CondJump.first->Header.NumArgs = 1; + // If condition doesn't hold then keep going - CondJump.first->Header.Args[0] = _Xor(Flag, _Constant(1)).Node->Wrapped(ListData.Begin()); + auto CondJump = _CondJump(_Xor(Flag, _Constant(1))); _Break(Reason, Literal); _EndBlock(0); // Make sure to start a new block after ending this one auto JumpTarget = _BeginBlock(); - CondJump.first->Header.Args[1] = JumpTarget.Node->Wrapped(ListData.Begin()); + SetJumpTarget(CondJump, JumpTarget); } else { _Break(Reason, Literal); diff --git a/Source/Interface/Core/OpcodeDispatcher.h b/Source/Interface/Core/OpcodeDispatcher.h index 308b96dbb..dfc7146d6 100644 --- a/Source/Interface/Core/OpcodeDispatcher.h +++ b/Source/Interface/Core/OpcodeDispatcher.h @@ -212,6 +212,21 @@ public: IRPair _VUShr(uint8_t RegisterSize, uint8_t ElementSize, OrderedNode *ssa0, OrderedNode *ssa1) { return _VUShr(ssa0, ssa1, RegisterSize, ElementSize); } + + IRPair _Jump() { + return _Jump(InvalidNode); + } + IRPair _CondJump(OrderedNode *ssa0) { + return _CondJump(ssa0, InvalidNode); + } + + void SetJumpTarget(IRPair Op, OrderedNode *Target) { + Op.first->Header.Args[0].NodeOffset = Target->Wrapped(ListData.Begin()).NodeOffset; + } + void SetJumpTarget(IRPair Op, OrderedNode *Target) { + Op.first->Header.Args[1].NodeOffset = Target->Wrapped(ListData.Begin()).NodeOffset; + } + /** @} */ bool IsValueConstant(NodeWrapper ssa, uint64_t *Constant) { @@ -309,6 +324,7 @@ private: IntrusiveAllocator Data; IntrusiveAllocator ListData; + OrderedNode *InvalidNode; }; void InstallOpcodeHandlers();