From 6834fe32e4e1d36b33a00666e075fa8981bd8d29 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Tue, 1 Aug 2023 19:44:17 -0700 Subject: [PATCH] IR: Adds support for GPRPair to Select IR op This IR operation was limited to GPR only previously for the values getting compared. This adds support for GPRPair (and technically FPR) so that it can be used directly with GPR pairs. I say technically FPR because the IREmitter disallowed FPRs for the comparison, but this was already supported in all of the backends, we just didn't ever use it. Some minor changes to the constant prop pass to ensure that we don't try to propagate a select in to a CondJump, otherwise pair comparisons would be duplicated. This code is expecting to be able to merge a simple comparison in to a `cbnz`, which doesn't happen with GPR pairs. --- .../Interface/Core/Interpreter/ALUOps.cpp | 23 ++++++++++++++----- .../Core/Interpreter/InterpreterOps.h | 9 ++++++++ .../Interface/Core/JIT/Arm64/ALUOps.cpp | 12 +++++++--- .../Source/Interface/Core/JIT/Arm64/JIT.cpp | 6 +++++ .../Interface/Core/JIT/Arm64/JITClass.h | 1 + .../Interface/Core/JIT/x86_64/ALUOps.cpp | 23 ++++++++++++++++++- .../Source/Interface/Core/JIT/x86_64/JIT.cpp | 8 +++++-- .../Interface/Core/JIT/x86_64/JITClass.h | 1 + External/FEXCore/Source/Interface/IR/IR.json | 7 ++++-- .../Source/Interface/IR/Passes/ConstProp.cpp | 5 ++++ .../FEXCore/include/FEXCore/IR/IREmitter.h | 4 ++++ 11 files changed, 85 insertions(+), 14 deletions(-) diff --git a/External/FEXCore/Source/Interface/Core/Interpreter/ALUOps.cpp b/External/FEXCore/Source/Interface/Core/Interpreter/ALUOps.cpp index 309dc4280..53cec3cd0 100644 --- a/External/FEXCore/Source/Interface/Core/Interpreter/ALUOps.cpp +++ b/External/FEXCore/Source/Interface/Core/Interpreter/ALUOps.cpp @@ -885,12 +885,8 @@ DEF_OP(Select) { auto Op = IROp->C(); const uint8_t OpSize = IROp->Size; - const uint64_t Src1 = *GetSrc(Data->SSAData, Op->Cmp1); - const uint64_t Src2 = *GetSrc(Data->SSAData, Op->Cmp2); - uint64_t ArgTrue; uint64_t ArgFalse; - if (OpSize == 4) { ArgTrue = *GetSrc(Data->SSAData, Op->TrueVal); ArgFalse = *GetSrc(Data->SSAData, Op->FalseVal); @@ -901,10 +897,25 @@ DEF_OP(Select) { bool CompResult; - if (Op->CompareSize == 4) + if (Op->CompareSize == 4) { + const auto Src1 = *GetSrc(Data->SSAData, Op->Cmp1); + const auto Src2 = *GetSrc(Data->SSAData, Op->Cmp2); CompResult = IsConditionTrue(Op->Cond.Val, Src1, Src2); - else + } + else if (Op->CompareSize == 8) { + const auto Src1 = *GetSrc(Data->SSAData, Op->Cmp1); + const auto Src2 = *GetSrc(Data->SSAData, Op->Cmp2); CompResult = IsConditionTrue(Op->Cond.Val, Src1, Src2); + } + else if (Op->CompareSize == 16) { + const auto Src1 = *GetSrc<__uint128_t*>(Data->SSAData, Op->Cmp1); + const auto Src2 = *GetSrc<__uint128_t*>(Data->SSAData, Op->Cmp2); + CompResult = IsConditionTrue<__uint128_t, __int128_t, double>(Op->Cond.Val, Src1, Src2); + } + else { + LOGMAN_MSG_A_FMT("Unknown select size: {}", Op->CompareSize); + FEX_UNREACHABLE; + } GD = CompResult ? ArgTrue : ArgFalse; } diff --git a/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h b/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h index c2895abb3..fce3ec7eb 100644 --- a/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h +++ b/External/FEXCore/Source/Interface/Core/Interpreter/InterpreterOps.h @@ -350,6 +350,15 @@ namespace FEXCore::CPU { template [[nodiscard]] static bool IsConditionTrue(uint8_t Cond, uint64_t Src1, uint64_t Src2) { bool CompResult = false; + if constexpr (sizeof(unsigned_type) == 16) { + LOGMAN_THROW_A_FMT(Cond != FEXCore::IR::COND_FLU && + Cond != FEXCore::IR::COND_FGE && + Cond != FEXCore::IR::COND_FLEU && + Cond != FEXCore::IR::COND_FGT && + Cond != FEXCore::IR::COND_FU && + Cond != FEXCore::IR::COND_FNU, "Unsupported comparison for 128-bit floats"); + } + switch (Cond) { case FEXCore::IR::COND_EQ: CompResult = static_cast(Src1) == static_cast(Src2); diff --git a/External/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp b/External/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp index 3cc7a4b61..6fac8724a 100644 --- a/External/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp +++ b/External/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp @@ -1129,6 +1129,7 @@ DEF_OP(Select) { const auto CompareEmitSize = Op->CompareSize == 8 ? ARMEmitter::Size::i64Bit : ARMEmitter::Size::i32Bit; uint64_t Const; + auto cc = MapSelectCC(Op->Cond); if (IsGPR(Op->Cmp1.ID())) { const auto Src1 = GetReg(Op->Cmp1.ID()); @@ -1139,7 +1140,14 @@ DEF_OP(Select) { const auto Src2 = GetReg(Op->Cmp2.ID()); cmp(CompareEmitSize, Src1, Src2); } - } else if (IsFPR(Op->Cmp1.ID())) { + } + else if (IsGPRPair(Op->Cmp1.ID())) { + const auto Src1 = GetRegPair(Op->Cmp1.ID()); + const auto Src2 = GetRegPair(Op->Cmp2.ID()); + cmp(EmitSize, Src1.first, Src2.first); + ccmp(EmitSize, Src1.second, Src2.second, ARMEmitter::StatusFlags::None, cc); + } + else if (IsFPR(Op->Cmp1.ID())) { const auto Src1 = GetVReg(Op->Cmp1.ID()); const auto Src2 = GetVReg(Op->Cmp2.ID()); fcmp(Op->CompareSize == 8 ? ARMEmitter::ScalarRegSize::i64Bit : ARMEmitter::ScalarRegSize::i32Bit, Src1, Src2); @@ -1147,8 +1155,6 @@ DEF_OP(Select) { LOGMAN_MSG_A_FMT("Select: Expected GPR or FPR"); } - auto cc = MapSelectCC(Op->Cond); - uint64_t const_true, const_false; bool is_const_true = IsInlineConstant(Op->TrueVal, &const_true); bool is_const_false = IsInlineConstant(Op->FalseVal, &const_false); diff --git a/External/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp b/External/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp index b1ae84127..654c8c749 100644 --- a/External/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp +++ b/External/FEXCore/Source/Interface/Core/JIT/Arm64/JIT.cpp @@ -724,6 +724,12 @@ bool Arm64JITCore::IsGPR(IR::NodeID Node) const { return Class == IR::GPRClass || Class == IR::GPRFixedClass; } +bool Arm64JITCore::IsGPRPair(IR::NodeID Node) const { + auto Class = GetRegClass(Node); + + return Class == IR::GPRPairClass; +} + CPUBackend::CompiledCode Arm64JITCore::CompileCode(uint64_t Entry, FEXCore::IR::IRListView const *IR, FEXCore::Core::DebugData *DebugData, diff --git a/External/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h b/External/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h index 047883c02..e3b7c1c3c 100644 --- a/External/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h +++ b/External/FEXCore/Source/Interface/Core/JIT/Arm64/JITClass.h @@ -112,6 +112,7 @@ private: [[nodiscard]] bool IsFPR(IR::NodeID Node) const; [[nodiscard]] bool IsGPR(IR::NodeID Node) const; + [[nodiscard]] bool IsGPRPair(IR::NodeID Node) const; [[nodiscard]] FEXCore::ARMEmitter::ExtendedMemOperand GenerateMemOperand(uint8_t AccessSize, FEXCore::ARMEmitter::Register Base, diff --git a/External/FEXCore/Source/Interface/Core/JIT/x86_64/ALUOps.cpp b/External/FEXCore/Source/Interface/Core/JIT/x86_64/ALUOps.cpp index 6bd9bcd0a..ae8cecc08 100644 --- a/External/FEXCore/Source/Interface/Core/JIT/x86_64/ALUOps.cpp +++ b/External/FEXCore/Source/Interface/Core/JIT/x86_64/ALUOps.cpp @@ -1140,7 +1140,28 @@ DEF_OP(Select) { } else { cmp(GRCMP(Op->Cmp1.ID()), GRCMP(Op->Cmp2.ID())); } - } else if (IsFPR(Op->Cmp1.ID())) { + } + else if (IsGPRPair(Op->Cmp1.ID())) { + if (Op->CompareSize == 4) { + const auto Src1 = GetSrcPair(Op->Cmp1.ID()); + const auto Src2 = GetSrcPair(Op->Cmp2.ID()); + mov (TMP1.cvt32(), Src1.first); + mov (TMP2.cvt32(), Src1.second); + xor_(TMP1.cvt32(), Src2.first); + xor_(TMP2.cvt32(), Src2.second); + or_(TMP1.cvt32(), TMP2.cvt32()); + } + else { + const auto Src1 = GetSrcPair(Op->Cmp1.ID()); + const auto Src2 = GetSrcPair(Op->Cmp2.ID()); + mov (TMP1, Src1.first); + mov (TMP2, Src1.second); + xor_(TMP1, Src2.first); + xor_(TMP2, Src2.second); + or_(TMP1, TMP2); + } + } + else if (IsFPR(Op->Cmp1.ID())) { if (Op->CompareSize == 4) ucomiss(GetSrc(Op->Cmp1.ID()), GetSrc(Op->Cmp2.ID())); else diff --git a/External/FEXCore/Source/Interface/Core/JIT/x86_64/JIT.cpp b/External/FEXCore/Source/Interface/Core/JIT/x86_64/JIT.cpp index bd96d36ed..41baf89bb 100644 --- a/External/FEXCore/Source/Interface/Core/JIT/x86_64/JIT.cpp +++ b/External/FEXCore/Source/Interface/Core/JIT/x86_64/JIT.cpp @@ -484,11 +484,15 @@ IR::PhysicalRegister X86JITCore::GetPhys(IR::NodeID Node) const { } bool X86JITCore::IsFPR(IR::NodeID Node) const { - return RAData->GetNodeRegister(Node).Class == IR::FPRClass.Val; + return RAData->GetNodeRegister(Node).Class == IR::FPRClass; } bool X86JITCore::IsGPR(IR::NodeID Node) const { - return RAData->GetNodeRegister(Node).Class == IR::GPRClass.Val; + return RAData->GetNodeRegister(Node).Class == IR::GPRClass; +} + +bool X86JITCore::IsGPRPair(IR::NodeID Node) const { + return RAData->GetNodeRegister(Node).Class == IR::GPRPairClass; } template diff --git a/External/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h b/External/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h index 846ec2573..b5abf3ffb 100644 --- a/External/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h +++ b/External/FEXCore/Source/Interface/Core/JIT/x86_64/JITClass.h @@ -169,6 +169,7 @@ private: [[nodiscard]] bool IsFPR(IR::NodeID Node) const; [[nodiscard]] bool IsGPR(IR::NodeID Node) const; + [[nodiscard]] bool IsGPRPair(IR::NodeID Node) const; template [[nodiscard]] Xbyak::Reg GetSrc(IR::NodeID Node) const; diff --git a/External/FEXCore/Source/Interface/IR/IR.json b/External/FEXCore/Source/Interface/IR/IR.json index 9ad62c729..1bb07f996 100644 --- a/External/FEXCore/Source/Interface/IR/IR.json +++ b/External/FEXCore/Source/Interface/IR/IR.json @@ -885,12 +885,15 @@ ], "DestSize": "8" }, - "GPR = Select CondClass:$Cond, GPR:$Cmp1, GPR:$Cmp2, GPR:$TrueVal, GPR:$FalseVal, u8:$CompareSize": { + "GPR = Select CondClass:$Cond, SSA:$Cmp1, SSA:$Cmp2, GPR:$TrueVal, GPR:$FalseVal, u8:$CompareSize": { "Desc": ["Ternary selection of GPRs", "op:", "Dest = Cmp1 Cmp2 ? TrueVal : FalseVal" ], - "DestSize": "std::max(4, std::max(GetOpSize(_TrueVal), GetOpSize(_FalseVal)))" + "DestSize": "std::max(4, std::max(GetOpSize(_TrueVal), GetOpSize(_FalseVal)))", + "EmitValidation": [ + "WalkFindRegClass($Cmp1) == WalkFindRegClass($Cmp2)" + ] }, "GPR = Extr GPR:$Upper, GPR:$Lower, u8:$LSB": { "Desc": ["Concats the two GPRs to create a value that is the size of the full two GPRs", diff --git a/External/FEXCore/Source/Interface/IR/Passes/ConstProp.cpp b/External/FEXCore/Source/Interface/IR/Passes/ConstProp.cpp index 239dafa67..9b5cc63b3 100644 --- a/External/FEXCore/Source/Interface/IR/Passes/ConstProp.cpp +++ b/External/FEXCore/Source/Interface/IR/Passes/ConstProp.cpp @@ -906,6 +906,11 @@ bool ConstProp::ConstantPropagation(IREmitter *IREmit, const IRListView& Current // Fold the select into the CondJump if possible. Could handle more complex cases, too. if (Op->Cond.Val == COND_NEQ && IREmit->IsValueConstant(Op->Cmp2, &Constant) && Constant == 0 && Select->Op == OP_SELECT) { + const auto SelectCmpClass = IREmit->WalkFindRegClass(Select->Args[0]); + if (SelectCmpClass == GPRPairClass) { + // If the comparison class is a GPRPair then don't fold the select since it isn't free. + break; + } uint64_t Constant1{}; uint64_t Constant2{}; diff --git a/External/FEXCore/include/FEXCore/IR/IREmitter.h b/External/FEXCore/include/FEXCore/IR/IREmitter.h index 8612e382e..6cc36bb4f 100644 --- a/External/FEXCore/include/FEXCore/IR/IREmitter.h +++ b/External/FEXCore/include/FEXCore/IR/IREmitter.h @@ -171,6 +171,10 @@ friend class FEXCore::IR::PassManager; } /** @} */ + FEXCore::IR::RegisterClassType WalkFindRegClass(OrderedNodeWrapper ssa) { + OrderedNode *RealNode = ssa.GetNode(DualListData.ListBegin()); + return WalkFindRegClass(RealNode); + } bool IsValueConstant(OrderedNodeWrapper ssa, uint64_t *Constant = nullptr) { OrderedNode *RealNode = ssa.GetNode(DualListData.ListBegin());