Merge pull request #3064 from alyssarosenzweig/ir/rm-phi

IR: Remove phi nodes

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

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);

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);

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);

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);

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);

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);

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

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": {

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 <FEXCore/IR/IRDefines.inc>
-        case IR::OP_PHI: {
-          auto Op = IROp->C<IR::IROp_Phi>();
-          auto NodeBegin = IR->at(Op->PhiBegin);
-          *out << " ";
-
-          while (NodeBegin != NodeBegin.Invalid()) {
-            auto [NodeNode, IROp] = NodeBegin();
-            auto PhiOp  = IROp->C<IR::IROp_PhiValue>();
-            *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 << "<Unknown Args>"; break;
         }
 

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());

FEXCore/Source/Interface/IR/Passes.h

@@ -26,7 +26,6 @@ fextl::unique_ptr<FEXCore::IR::Pass> CreateLongDivideEliminationPass();
 namespace Validation {
 fextl::unique_ptr<FEXCore::IR::Pass> CreateIRValidation();
 fextl::unique_ptr<FEXCore::IR::Pass> CreateRAValidation();
-fextl::unique_ptr<FEXCore::IR::Pass> CreatePhiValidation();
 fextl::unique_ptr<FEXCore::IR::Pass> CreateValueDominanceValidation();
 }
 

FEXCore/Source/Interface/IR/Passes/PhiValidation.cpp

@@ -1,77 +0,0 @@
-/*
-$info$
-tags: ir|opts
-desc: Sanity checking pass
-$end_info$
-*/
-
-#include <FEXCore/IR/IR.h>
-#include <FEXCore/IR/IREmitter.h>
-#include <FEXCore/IR/IntrusiveIRList.h>
-#include <FEXCore/Utils/LogManager.h>
-#include <FEXCore/Utils/Profiler.h>
-#include <FEXCore/fextl/sstream.h>
-
-#include "Interface/IR/PassManager.h"
-
-#include <memory>
-
-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<FEXCore::IR::Pass> CreatePhiValidation() {
-  return fextl::make_unique<PhiValidation>();
-}
-
-}

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<RegisterNode*> 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<IR::IROp_PhiValue>();
-        return GetRegClassFromNode(IR, IR->GetOp<IR::IROp_Header>(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<IR::IROp_Phi>();
-        return GetRegClassFromNode(IR, IR->GetOp<IR::IROp_Header>(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<IR::IROp_Phi>();
-          auto NodeBegin = IR->at(Op->PhiBegin);
-
-          auto CurrentSourcePartner = Node;
-          while (NodeBegin != NodeBegin.Invalid()) {
-            const auto [ValueNode, ValueHeader] = NodeBegin();
-            const auto ValueOp = ValueHeader->CW<IROp_PhiValue>();
-            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<RegisterNode *> 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<uint64_t>(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<uint64_t>(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

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<IR::IROp_PhiValue>()->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<IR::IROp_PhiValue>();
-    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 %{} {}",