diff --git a/FEXCore/Source/CMakeLists.txt b/FEXCore/Source/CMakeLists.txt index 3fd60a4b1..3e2e4b232 100644 --- a/FEXCore/Source/CMakeLists.txt +++ b/FEXCore/Source/CMakeLists.txt @@ -138,7 +138,6 @@ set (SRCS Interface/IR/Passes/RAValidation.cpp Interface/IR/Passes/LongDivideRemovalPass.cpp Interface/IR/Passes/ValueDominanceValidation.cpp - Interface/IR/Passes/PhiValidation.cpp Interface/IR/Passes/RedundantFlagCalculationElimination.cpp Interface/IR/Passes/DeadStoreElimination.cpp Interface/IR/Passes/RegisterAllocationPass.cpp diff --git a/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.cpp b/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.cpp index 1612affef..2f96f64fa 100644 --- a/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.cpp +++ b/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.cpp @@ -173,8 +173,6 @@ constexpr OpHandlerArray InterpreterOpHandlers = [] { REGISTER_OP(GUESTOPCODE, NoOp); REGISTER_OP(FENCE, Fence); REGISTER_OP(BREAK, Break); - REGISTER_OP(PHI, NoOp); - REGISTER_OP(PHIVALUE, NoOp); REGISTER_OP(PRINT, Print); REGISTER_OP(GETROUNDINGMODE, GetRoundingMode); REGISTER_OP(SETROUNDINGMODE, SetRoundingMode); diff --git a/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h b/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h index da4e539b8..604550017 100644 --- a/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h +++ b/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h @@ -204,8 +204,6 @@ namespace FEXCore::CPU { DEF_OP(EndBlock); DEF_OP(Fence); DEF_OP(Break); - DEF_OP(Phi); - DEF_OP(PhiValue); DEF_OP(Print); DEF_OP(GetRoundingMode); DEF_OP(SetRoundingMode); diff --git a/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp b/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp index 50d86d8ee..63a568aa6 100644 --- a/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp +++ b/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp @@ -993,8 +993,6 @@ CPUBackend::CompiledCode Arm64JITCore::CompileCode(uint64_t Entry, REGISTER_OP(GUESTOPCODE, GuestOpcode); REGISTER_OP(FENCE, Fence); REGISTER_OP(BREAK, Break); - REGISTER_OP(PHI, NoOp); - REGISTER_OP(PHIVALUE, NoOp); REGISTER_OP(PRINT, Print); REGISTER_OP(GETROUNDINGMODE, GetRoundingMode); REGISTER_OP(SETROUNDINGMODE, SetRoundingMode); diff --git a/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h b/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h index 36719e124..ee62b5753 100644 --- a/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h +++ b/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h @@ -361,8 +361,6 @@ private: DEF_OP(GuestOpcode); DEF_OP(Fence); DEF_OP(Break); - DEF_OP(Phi); - DEF_OP(PhiValue); DEF_OP(Print); DEF_OP(GetRoundingMode); DEF_OP(SetRoundingMode); diff --git a/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h b/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h index 3d99a8490..9c7bd8d03 100644 --- a/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h +++ b/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h @@ -363,8 +363,6 @@ private: DEF_OP(GuestOpcode); DEF_OP(Fence); DEF_OP(Break); - DEF_OP(Phi); - DEF_OP(PhiValue); DEF_OP(Print); DEF_OP(GetRoundingMode); DEF_OP(SetRoundingMode); diff --git a/FEXCore/Source/Interface/Core/JIT/x86_64/MiscOps.cpp b/FEXCore/Source/Interface/Core/JIT/x86_64/MiscOps.cpp index 9b8dc68e4..fc7ab5bf7 100644 --- a/FEXCore/Source/Interface/Core/JIT/x86_64/MiscOps.cpp +++ b/FEXCore/Source/Interface/Core/JIT/x86_64/MiscOps.cpp @@ -176,8 +176,6 @@ void X86JITCore::RegisterMiscHandlers() { REGISTER_OP(GUESTOPCODE, GuestOpcode); REGISTER_OP(FENCE, Fence); REGISTER_OP(BREAK, Break); - REGISTER_OP(PHI, NoOp); - REGISTER_OP(PHIVALUE, NoOp); REGISTER_OP(PRINT, Print); REGISTER_OP(GETROUNDINGMODE, GetRoundingMode); REGISTER_OP(SETROUNDINGMODE, SetRoundingMode); diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index eb31c5e3a..5433c5975 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -4064,7 +4064,7 @@ void OpDispatchBuilder::CMPSOp(OpcodeArgs) { // Decrement counter TailCounter = _Sub(OpSize::i64Bit, TailCounter, _Constant(1)); - // Store the counter so we don't have to deal with PHI here + // Store the counter since we don't have phis StoreGPRRegister(X86State::REG_RCX, TailCounter); // Offset the pointer @@ -4180,7 +4180,7 @@ void OpDispatchBuilder::LODSOp(OpcodeArgs) { // Decrement counter TailCounter = _Sub(OpSize::i64Bit, TailCounter, _Constant(1)); - // Store the counter so we don't have to deal with PHI here + // Store the counter since we don't have phis StoreGPRRegister(X86State::REG_RCX, TailCounter); // Offset the pointer @@ -4293,7 +4293,7 @@ void OpDispatchBuilder::SCASOp(OpcodeArgs) { // Decrement counter TailCounter = _Sub(OpSize::i64Bit, TailCounter, _Constant(1)); - // Store the counter so we don't have to deal with PHI here + // Store the counter since we don't have phis StoreGPRRegister(X86State::REG_RCX, TailCounter); // Offset the pointer diff --git a/FEXCore/Source/Interface/IR/IR.json b/FEXCore/Source/Interface/IR/IR.json index 9fbcb60fa..b1fda81cd 100644 --- a/FEXCore/Source/Interface/IR/IR.json +++ b/FEXCore/Source/Interface/IR/IR.json @@ -328,16 +328,6 @@ "EmitValidation": [ "Size == FEXCore::IR::OpSize::i64Bit || Size == FEXCore::IR::OpSize::i128Bit" ] - }, - "SSA = Phi SSA:$PhiBegin, SSA:$PhiEnd, RegisterClass:$Class": { - "DestSize": "~0", - "ArgPrinter": false, - "RAOverride": 0 - }, - - "PhiValue SSA:$Value, SSA:$Block, SSA:$Next": { - "RAOverride": 0, - "DestSize": "GetOpSize(_Value)" } }, "StaticRA": { diff --git a/FEXCore/Source/Interface/IR/IRDumper.cpp b/FEXCore/Source/Interface/IR/IRDumper.cpp index 5c96f3be7..60f279b08 100644 --- a/FEXCore/Source/Interface/IR/IRDumper.cpp +++ b/FEXCore/Source/Interface/IR/IRDumper.cpp @@ -278,15 +278,7 @@ void Dump(fextl::stringstream *out, IRListView const* IR, IR::RegisterAllocation const auto ID = IR->GetID(CodeNode); const auto Name = FEXCore::IR::GetName(IROp->Op); - bool Skip{}; - switch (IROp->Op) { - case IR::OP_PHIVALUE: - Skip = true; - break; - default: break; - } - - if (!Skip) { + { AddIndent(); if (GetHasDest(IROp->Op)) { @@ -351,28 +343,6 @@ void Dump(fextl::stringstream *out, IRListView const* IR, IR::RegisterAllocation #define IROP_ARGPRINTER_HELPER #include - case IR::OP_PHI: { - auto Op = IROp->C(); - auto NodeBegin = IR->at(Op->PhiBegin); - *out << " "; - - while (NodeBegin != NodeBegin.Invalid()) { - auto [NodeNode, IROp] = NodeBegin(); - auto PhiOp = IROp->C(); - *out << "[ "; - PrintArg(out, IR, PhiOp->Value, RAData); - *out << ", "; - PrintArg(out, IR, PhiOp->Block, RAData); - *out << " ]"; - - if (PhiOp->Next.ID().IsValid()) { - *out << ", "; - } - - NodeBegin = IR->at(PhiOp->Next); - } - break; - } default: *out << ""; break; } diff --git a/FEXCore/Source/Interface/IR/PassManager.cpp b/FEXCore/Source/Interface/IR/PassManager.cpp index 68e114901..07418234f 100644 --- a/FEXCore/Source/Interface/IR/PassManager.cpp +++ b/FEXCore/Source/Interface/IR/PassManager.cpp @@ -94,7 +94,6 @@ void PassManager::AddDefaultPasses(FEXCore::Context::ContextImpl *ctx, bool Inli void PassManager::AddDefaultValidationPasses() { #if defined(ASSERTIONS_ENABLED) && ASSERTIONS_ENABLED - InsertValidationPass(Validation::CreatePhiValidation()); InsertValidationPass(Validation::CreateIRValidation(), "IRValidation"); InsertValidationPass(Validation::CreateRAValidation()); InsertValidationPass(Validation::CreateValueDominanceValidation()); diff --git a/FEXCore/Source/Interface/IR/Passes.h b/FEXCore/Source/Interface/IR/Passes.h index 26d0b91d5..f917b6870 100644 --- a/FEXCore/Source/Interface/IR/Passes.h +++ b/FEXCore/Source/Interface/IR/Passes.h @@ -26,7 +26,6 @@ fextl::unique_ptr CreateLongDivideEliminationPass(); namespace Validation { fextl::unique_ptr CreateIRValidation(); fextl::unique_ptr CreateRAValidation(); -fextl::unique_ptr CreatePhiValidation(); fextl::unique_ptr CreateValueDominanceValidation(); } diff --git a/FEXCore/Source/Interface/IR/Passes/PhiValidation.cpp b/FEXCore/Source/Interface/IR/Passes/PhiValidation.cpp deleted file mode 100644 index e0250de3c..000000000 --- a/FEXCore/Source/Interface/IR/Passes/PhiValidation.cpp +++ /dev/null @@ -1,77 +0,0 @@ -/* -$info$ -tags: ir|opts -desc: Sanity checking pass -$end_info$ -*/ - -#include -#include -#include -#include -#include -#include - -#include "Interface/IR/PassManager.h" - -#include - -namespace FEXCore::IR::Validation { - -class PhiValidation final : public FEXCore::IR::Pass { -public: - bool Run(IREmitter *IREmit) override; -}; - -bool PhiValidation::Run(IREmitter *IREmit) { - FEXCORE_PROFILE_SCOPED("PassManager::PHIValidation"); - - bool HadError = false; - auto CurrentIR = IREmit->ViewIR(); - - fextl::ostringstream Errors; - - // Walk the list and calculate the control flow - for (auto [BlockNode, BlockHeader] : CurrentIR.GetBlocks()) { - - bool FoundNonPhi{}; - - for (auto [CodeNode, IROp] : CurrentIR.GetCode(BlockNode)) { - - switch (IROp->Op) { - // BEGINBLOCK doesn't matter for us - case IR::OP_BEGINBLOCK: break; - case IR::OP_PHIVALUE: - case IR::OP_PHI: { - if (FoundNonPhi) { - // If we have found a non-phi IR op and then had a Phi or PhiValue value then this is a programming mistake - // PHI values MUST be defined at the top of the block only - HadError |= true; - Errors << "Phi %" << CurrentIR.GetID(CodeNode) << ": Was defined after non-phi operations. Which is invalid!" << std::endl; - } - - // Check all the phi values to ensure they have the same type - break; - } - default: - FoundNonPhi = true; - break; - } - } - } - - if (HadError) { - fextl::stringstream Out; - FEXCore::IR::Dump(&Out, &CurrentIR, nullptr); - Out << "Errors:" << std::endl << Errors.str() << std::endl; - LogMan::Msg::EFmt("{}", Out.str()); - } - - return false; -} - -fextl::unique_ptr CreatePhiValidation() { - return fextl::make_unique(); -} - -} diff --git a/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp b/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp index 755a6e0f4..d81563234 100644 --- a/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp +++ b/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp @@ -57,7 +57,7 @@ namespace { struct VolatileHeader { IR::NodeID BlockID{UINT32_MAX}; uint32_t SpillSlot{UINT32_MAX}; - RegisterNode *PhiPartner{nullptr}; + uint64_t Padding; }; VolatileHeader Head; @@ -163,43 +163,6 @@ namespace { Graph->AllocData->Map[Node.Value].Class = Class.Val; } - void SetNodePartner(RegisterGraph *Graph, IR::NodeID Node, IR::NodeID Partner) { - Graph->Nodes[Node.Value].Head.PhiPartner = &Graph->Nodes[Partner.Value]; - } - - - #if 0 - bool IsConflict(RegisterGraph *Graph, PhysicalRegister RegAndClass, PhysicalRegister ConflictRegAndClass) { - uint32_t Index = (ConflictRegAndClass.Class << 8) | RegAndClass.Raw; - return (Graph->Set.Conflicts[Index] >> ConflictRegAndClass.Reg) & 1; - } - - // PHI nodes currently unsupported - /** - * @brief Individual node interference check - */ - bool DoesNodeInterfereWithRegister(RegisterGraph *Graph, RegisterNode const *Node, PhysicalRegister RegAndClass) { - // Walk the node's interference list and see if it interferes with this register - return Node->Interferences.Find([Graph, RegAndClass](IR::NodeID InterferenceNodeId) { - auto InterferenceRegAndClass = Graph->AllocData->Map[InterferenceNodeId]; - return IsConflict(Graph, InterferenceRegAndClass, RegAndClass); - }); - } - - /** - * @brief Node set walking for PHI node interference checking - */ - bool DoesNodeSetInterfereWithRegister(RegisterGraph *Graph, fextl::vector const &Nodes, PhysicalRegister RegAndClass) { - for (auto it : Nodes) { - if (DoesNodeInterfereWithRegister(Graph, it, RegAndClass)) { - return true; - } - } - - return false; - } - #endif - FEXCore::IR::RegisterClassType GetRegClassFromNode(FEXCore::IR::IRListView *IR, FEXCore::IR::IROp_Header *IROp) { using namespace FEXCore; @@ -235,17 +198,6 @@ namespace { return Op->Class; break; } - case IR::OP_PHIVALUE: { - // Unwrap the PHIValue to get the class - auto Op = IROp->C(); - return GetRegClassFromNode(IR, IR->GetOp(Op->Value)); - } - case IR::OP_PHI: { - // Class is defined from the values passed in - // All Phi nodes should have its class be the same (Validation should confirm this - auto Op = IROp->C(); - return GetRegClassFromNode(IR, IR->GetOp(Op->PhiBegin)); - } default: break; } @@ -432,10 +384,6 @@ namespace { case IR::OP_FILLREGISTER: return DEFAULT_REMAT_COST + 1; - // We want PHI to be very expensive to spill - case IR::OP_PHI: - return DEFAULT_REMAT_COST * 10; - default: return DEFAULT_REMAT_COST; } @@ -516,26 +464,6 @@ namespace { Graph->VisitedNodePredecessors[ArgNode]); } } - - if (IROp->Op == IR::OP_PHI) { - // Special case the PHI op, all of the nodes in the argument need to have the same virtual register affinity - // Walk through all of them and set affinities for each other - auto Op = IROp->C(); - auto NodeBegin = IR->at(Op->PhiBegin); - - auto CurrentSourcePartner = Node; - while (NodeBegin != NodeBegin.Invalid()) { - const auto [ValueNode, ValueHeader] = NodeBegin(); - const auto ValueOp = ValueHeader->CW(); - const auto ValueID = ValueOp->Value.ID(); - - // Set the node partner to the current one - // This creates a singly linked list of node partners to follow - SetNodePartner(Graph, CurrentSourcePartner, ValueID); - CurrentSourcePartner = ValueID; - NodeBegin = IR->at(ValueOp->Next); - } - } } } } @@ -961,71 +889,34 @@ namespace { auto RegAndClass = PhysicalRegister::Invalid(); RegisterClass *RAClass = &Graph->Set.Classes[RegClass]; - if (CurrentNode->Head.PhiPartner) { - LOGMAN_MSG_A_FMT("Phi nodes not supported"); - #if 0 - // In the case that we have a list of nodes that need the same register allocated we need to do something special - // We need to gather the data from the forward linked list and make sure they all match the virtual register - fextl::vector Nodes; - auto CurrentPartner = CurrentNode; - while (CurrentPartner) { - Nodes.emplace_back(CurrentPartner); - CurrentPartner = CurrentPartner->Head.PhiPartner; - } + if (!LiveRange->PrefferedRegister.IsInvalid()) { + RegAndClass = LiveRange->PrefferedRegister; + } else { + uint32_t RegisterConflicts = 0; + CurrentNode->Interferences.Iterate([&](const IR::NodeID InterferenceNode) { + RegisterConflicts |= GetConflicts(Graph, Graph->AllocData->Map[InterferenceNode.Value], {RegClass}); + }); - for (uint32_t ri = 0; ri < RAClass->Count; ++ri) { - uint64_t RegisterToCheck = (static_cast(RegClass) << 32) + ri; - if (!DoesNodeSetInterfereWithRegister(Graph, Nodes, RegisterToCheck)) { - RegAndClass = RegisterToCheck; - break; - } - } + RegisterConflicts = (~RegisterConflicts) & RAClass->CountMask; - // If we failed to find a virtual register then allocate more space for them - if (RegAndClass == ~0ULL) { - RegAndClass = (static_cast(RegClass.Val) << 32); - RegAndClass |= INVALID_REG; + int Reg = FindFirstSetBit(RegisterConflicts); + if (Reg != 0) { + RegAndClass = PhysicalRegister({RegClass}, Reg-1); } - - TopRAPressure[RegClass] = std::max((uint32_t)RegAndClass + 1, TopRAPressure[RegClass]); - - // Walk the partners and ensure they are all set to the same register now - for (auto Partner : Nodes) { - Partner->Head.RegAndClass = RegAndClass; - } - #endif } - else { - if (!LiveRange->PrefferedRegister.IsInvalid()) { - RegAndClass = LiveRange->PrefferedRegister; - } else { - uint32_t RegisterConflicts = 0; - CurrentNode->Interferences.Iterate([&](const IR::NodeID InterferenceNode) { - RegisterConflicts |= GetConflicts(Graph, Graph->AllocData->Map[InterferenceNode.Value], {RegClass}); - }); - - RegisterConflicts = (~RegisterConflicts) & RAClass->CountMask; - - int Reg = FindFirstSetBit(RegisterConflicts); - if (Reg != 0) { - RegAndClass = PhysicalRegister({RegClass}, Reg-1); - } - } - - // If we failed to find a virtual register then use INVALID_REG and mark allocation as failed - if (RegAndClass.IsInvalid()) { - RegAndClass = IR::PhysicalRegister(RegClass, INVALID_REG); - HadFullRA = false; - SpillPointId = IR::NodeID{i}; - - CurrentRegAndClass = RegAndClass; - // Must spill and restart - return; - } + // If we failed to find a virtual register then use INVALID_REG and mark allocation as failed + if (RegAndClass.IsInvalid()) { + RegAndClass = IR::PhysicalRegister(RegClass, INVALID_REG); + HadFullRA = false; + SpillPointId = IR::NodeID{i}; CurrentRegAndClass = RegAndClass; + // Must spill and restart + return; } + + CurrentRegAndClass = RegAndClass; } } @@ -1415,11 +1306,8 @@ namespace { const uint32_t SpillSlot = FindSpillSlot(*InterferenceNode, InterferenceRegClass); #if defined(ASSERTIONS_ENABLED) && ASSERTIONS_ENABLED - RegisterNode *InterferenceRegisterNode = &Graph->Nodes[InterferenceNode->Value]; LOGMAN_THROW_A_FMT(SpillSlot != UINT32_MAX, "Interference Node doesn't have a spill slot!"); - //LOGMAN_THROW_A_FMT(InterferenceRegisterNode->Head.RegAndClass.Reg != INVALID_REG, "Interference node never assigned a register?"); LOGMAN_THROW_A_FMT(InterferenceRegClass != UINT32_MAX, "Interference node never assigned a register class?"); - LOGMAN_THROW_A_FMT(InterferenceRegisterNode->Head.PhiPartner == nullptr, "We don't support spilling PHI nodes currently"); #endif // This is the op that we need to dump diff --git a/FEXCore/include/FEXCore/IR/IREmitter.h b/FEXCore/include/FEXCore/IR/IREmitter.h index 1f411a98c..266fa2657 100644 --- a/FEXCore/include/FEXCore/IR/IREmitter.h +++ b/FEXCore/include/FEXCore/IR/IREmitter.h @@ -91,22 +91,6 @@ friend class FEXCore::IR::PassManager; return InvalidNode; } - void AddPhiValue(IR::IROp_Phi *Phi, OrderedNode *Value) { - // Got to do some bookkeeping first - Value->AddUse(); - auto ValueIROp = Value->Op(DualListData.DataBegin())->C()->Value.GetNode(DualListData.ListBegin())->Op(DualListData.DataBegin()); - Phi->Header.Size = ValueIROp->Size; - Phi->Header.ElementSize = ValueIROp->ElementSize; - - if (Phi->PhiBegin.ID().IsInvalid()) { - Phi->PhiBegin = Phi->PhiEnd = Value->Wrapped(DualListData.ListBegin()); - return; - } - auto PhiValueEndNode = Phi->PhiEnd.GetNode(DualListData.ListBegin()); - auto PhiValueEndOp = PhiValueEndNode->Op(DualListData.DataBegin())->CW(); - PhiValueEndOp->Next = Value->Wrapped(DualListData.ListBegin()); - } - void SetJumpTarget(IR::IROp_Jump *Op, OrderedNode *Target) { LOGMAN_THROW_A_FMT(Target->Op(DualListData.DataBegin())->Op == OP_CODEBLOCK, "Tried setting Jump target to %{} {}",