From 6208fcd1e5bef0066c449bac4e21039b3b335891 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Wed, 24 Jul 2024 20:12:13 +0800 Subject: [PATCH 1/6] [RISCV] Move vmv.v.v peephole from SelectionDAG to RISCVVectorPeephole This is split off from #71764, and moves only the vmv.v.v part of performCombineVMergeAndVOps to work on MachineInstrs. In retrospect trying to handle PseudoVMV_V_V and PseudoVMERGE_VVM in the same function makes the code quite hard to read, so this just does it in a separate peephole. This turns out to be simpler since for PseudoVMV_V_V we don't need to convert the Src instruction to a masked variant, and we don't need to create a fake all ones mask. --- llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp | 84 ++--------- llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp | 141 +++++++++++++++++- 2 files changed, 154 insertions(+), 71 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp index aed10c2de4372..63ecb65457868 100644 --- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp +++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp @@ -3664,32 +3664,6 @@ static bool IsVMerge(SDNode *N) { return RISCV::getRVVMCOpcode(N->getMachineOpcode()) == RISCV::VMERGE_VVM; } -static bool IsVMv(SDNode *N) { - return RISCV::getRVVMCOpcode(N->getMachineOpcode()) == RISCV::VMV_V_V; -} - -static unsigned GetVMSetForLMul(RISCVII::VLMUL LMUL) { - switch (LMUL) { - case RISCVII::LMUL_F8: - return RISCV::PseudoVMSET_M_B1; - case RISCVII::LMUL_F4: - return RISCV::PseudoVMSET_M_B2; - case RISCVII::LMUL_F2: - return RISCV::PseudoVMSET_M_B4; - case RISCVII::LMUL_1: - return RISCV::PseudoVMSET_M_B8; - case RISCVII::LMUL_2: - return RISCV::PseudoVMSET_M_B16; - case RISCVII::LMUL_4: - return RISCV::PseudoVMSET_M_B32; - case RISCVII::LMUL_8: - return RISCV::PseudoVMSET_M_B64; - case RISCVII::LMUL_RESERVED: - llvm_unreachable("Unexpected LMUL"); - } - llvm_unreachable("Unknown VLMUL enum"); -} - // Try to fold away VMERGE_VVM instructions into their true operands: // // %true = PseudoVADD_VV ... @@ -3704,35 +3678,22 @@ static unsigned GetVMSetForLMul(RISCVII::VLMUL LMUL) { // If %true is masked, then we can use its mask instead of vmerge's if vmerge's // mask is all ones. // -// We can also fold a VMV_V_V into its true operand, since it is equivalent to a -// VMERGE_VVM with an all ones mask. -// // The resulting VL is the minimum of the two VLs. // // The resulting policy is the effective policy the vmerge would have had, // i.e. whether or not it's passthru operand was implicit-def. bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N) { SDValue Passthru, False, True, VL, Mask, Glue; - // A vmv.v.v is equivalent to a vmerge with an all-ones mask. - if (IsVMv(N)) { - Passthru = N->getOperand(0); - False = N->getOperand(0); - True = N->getOperand(1); - VL = N->getOperand(2); - // A vmv.v.v won't have a Mask or Glue, instead we'll construct an all-ones - // mask later below. - } else { - assert(IsVMerge(N)); - Passthru = N->getOperand(0); - False = N->getOperand(1); - True = N->getOperand(2); - Mask = N->getOperand(3); - VL = N->getOperand(4); - // We always have a glue node for the mask at v0. - Glue = N->getOperand(N->getNumOperands() - 1); - } - assert(!Mask || cast(Mask)->getReg() == RISCV::V0); - assert(!Glue || Glue.getValueType() == MVT::Glue); + assert(IsVMerge(N)); + Passthru = N->getOperand(0); + False = N->getOperand(1); + True = N->getOperand(2); + Mask = N->getOperand(3); + VL = N->getOperand(4); + // We always have a glue node for the mask at v0. + Glue = N->getOperand(N->getNumOperands() - 1); + assert(cast(Mask)->getReg() == RISCV::V0); + assert(Glue.getValueType() == MVT::Glue); // If the EEW of True is different from vmerge's SEW, then we can't fold. if (True.getSimpleValueType() != N->getSimpleValueType(0)) @@ -3780,7 +3741,7 @@ bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N) { // If True is masked then the vmerge must have either the same mask or an all // 1s mask, since we're going to keep the mask from True. - if (IsMasked && Mask) { + if (IsMasked) { // FIXME: Support mask agnostic True instruction which would have an // undef passthru operand. SDValue TrueMask = @@ -3810,11 +3771,9 @@ bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N) { SmallVector LoopWorklist; SmallPtrSet Visited; LoopWorklist.push_back(False.getNode()); - if (Mask) - LoopWorklist.push_back(Mask.getNode()); + LoopWorklist.push_back(Mask.getNode()); LoopWorklist.push_back(VL.getNode()); - if (Glue) - LoopWorklist.push_back(Glue.getNode()); + LoopWorklist.push_back(Glue.getNode()); if (SDNode::hasPredecessorHelper(True.getNode(), Visited, LoopWorklist)) return false; } @@ -3875,21 +3834,6 @@ bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N) { Glue = True->getOperand(True->getNumOperands() - 1); assert(Glue.getValueType() == MVT::Glue); } - // If we end up using the vmerge mask the vmerge is actually a vmv.v.v, create - // an all-ones mask to use. - else if (IsVMv(N)) { - unsigned TSFlags = TII->get(N->getMachineOpcode()).TSFlags; - unsigned VMSetOpc = GetVMSetForLMul(RISCVII::getLMul(TSFlags)); - ElementCount EC = N->getValueType(0).getVectorElementCount(); - MVT MaskVT = MVT::getVectorVT(MVT::i1, EC); - - SDValue AllOnesMask = - SDValue(CurDAG->getMachineNode(VMSetOpc, DL, MaskVT, VL, SEW), 0); - SDValue MaskCopy = CurDAG->getCopyToReg(CurDAG->getEntryNode(), DL, - RISCV::V0, AllOnesMask, SDValue()); - Mask = CurDAG->getRegister(RISCV::V0, MaskVT); - Glue = MaskCopy.getValue(1); - } unsigned MaskedOpc = Info->MaskedPseudo; #ifndef NDEBUG @@ -3968,7 +3912,7 @@ bool RISCVDAGToDAGISel::doPeepholeMergeVVMFold() { if (N->use_empty() || !N->isMachineOpcode()) continue; - if (IsVMerge(N) || IsVMv(N)) + if (IsVMerge(N)) MadeChange |= performCombineVMergeAndVOps(N); } return MadeChange; diff --git a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp index 979677ee92332..fbb238cd3eeac 100644 --- a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp +++ b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp @@ -65,6 +65,7 @@ class RISCVVectorPeephole : public MachineFunctionPass { bool convertToWholeRegister(MachineInstr &MI) const; bool convertToUnmasked(MachineInstr &MI) const; bool convertVMergeToVMv(MachineInstr &MI) const; + bool foldVMV_V_V(MachineInstr &MI); bool isAllOnesMask(const MachineInstr *MaskDef) const; std::optional getConstant(const MachineOperand &VL) const; @@ -324,6 +325,143 @@ bool RISCVVectorPeephole::convertToUnmasked(MachineInstr &MI) const { return true; } +/// Given two VL operands, returns the one known to be the smallest or nullptr +/// if unknown. +static const MachineOperand *getKnownMinVL(const MachineOperand *LHS, + const MachineOperand *RHS) { + if (LHS->isReg() && RHS->isReg() && LHS->getReg().isVirtual() && + LHS->getReg() == RHS->getReg()) + return LHS; + if (LHS->isImm() && LHS->getImm() == RISCV::VLMaxSentinel) + return RHS; + if (RHS->isImm() && RHS->getImm() == RISCV::VLMaxSentinel) + return LHS; + if (!LHS->isImm() || !RHS->isImm()) + return nullptr; + return LHS->getImm() <= RHS->getImm() ? LHS : RHS; +} + +/// Check if it's safe to move From down to To, checking that no physical +/// registers are clobbered. +static bool isSafeToMove(const MachineInstr &From, const MachineInstr &To) { + assert(From.getParent() == To.getParent() && !From.hasImplicitDef()); + SmallVector PhysUses; + for (const MachineOperand &MO : From.all_uses()) + if (MO.getReg().isPhysical()) + PhysUses.push_back(MO.getReg()); + bool SawStore = false; + for (auto II = From.getIterator(); II != To.getIterator(); II++) { + for (Register PhysReg : PhysUses) + if (II->definesRegister(PhysReg, nullptr)) + return false; + if (II->mayStore()) { + SawStore = true; + break; + } + } + return From.isSafeToMove(nullptr, SawStore); +} + +static const RISCV::RISCVMaskedPseudoInfo * +lookupMaskedPseudoInfo(const MachineInstr &MI) { + const RISCV::RISCVMaskedPseudoInfo *Info = + RISCV::lookupMaskedIntrinsicByUnmasked(MI.getOpcode()); + if (!Info) + Info = RISCV::getMaskedPseudoInfo(MI.getOpcode()); + return Info; +} + +/// If a PseudoVMV_V_V is the only user of it's input, fold its passthru and VL +/// into it. +/// +/// %x = PseudoVADD_V_V_M1 %passthru, %a, %b, %vl, sew, policy +/// %y = PseudoVMV_V_V_M1 %passthru, %x, %vl, sew, policy +/// +/// -> +/// +/// %y = PseudoVADD_V_V_M1 %passthru, %a, %b, %vl, sew, policy +bool RISCVVectorPeephole::foldVMV_V_V(MachineInstr &MI) { + if (RISCV::getRVVMCOpcode(MI.getOpcode()) != RISCV::VMV_V_V) + return false; + + MachineOperand &Passthru = MI.getOperand(1); + MachineInstr *Src = MRI->getVRegDef(MI.getOperand(2).getReg()); + + if (!MRI->hasOneUse(MI.getOperand(2).getReg())) + return false; + + if (!Src || Src->hasUnmodeledSideEffects() || + Src->getParent() != MI.getParent()) + return false; + + // Src needs to be a pseudo that's opted into this transform. + const RISCV::RISCVMaskedPseudoInfo *Info = lookupMaskedPseudoInfo(*Src); + if (!Info) + return false; + + assert(Src->getNumDefs() == 1 && + RISCVII::isFirstDefTiedToFirstUse(Src->getDesc()) && + RISCVII::hasVLOp(Src->getDesc().TSFlags) && + RISCVII::hasVecPolicyOp(Src->getDesc().TSFlags)); + + // Src needs to have the same passthru as VMV_V_V + if (Src->getOperand(1).getReg() != RISCV::NoRegister && + Src->getOperand(1).getReg() != Passthru.getReg()) + return false; + + // Because Src and MI have the same passthru, we can use either AVL as long as + // it's the smaller of the two. + // + // (src pt, ..., vl=5) x x x x x|. . . + // (vmv.v.v pt, src, vl=3) x x x|. . . . . + // -> + // (src pt, ..., vl=3) x x x|. . . . . + // + // (src pt, ..., vl=3) x x x|. . . . . + // (vmv.v.v pt, src, vl=6) x x x . . .|. . + // -> + // (src pt, ..., vl=3) x x x|. . . . . + MachineOperand &SrcVL = Src->getOperand(RISCVII::getVLOpNum(Src->getDesc())); + const MachineOperand *MinVL = getKnownMinVL(&MI.getOperand(3), &SrcVL); + if (!MinVL) + return false; + + bool VLChanged = !MinVL->isIdenticalTo(SrcVL); + bool RaisesFPExceptions = MI.getDesc().mayRaiseFPException() && + !MI.getFlag(MachineInstr::MIFlag::NoFPExcept); + if (VLChanged && (Info->ActiveElementsAffectResult || RaisesFPExceptions)) + return false; + + if (!isSafeToMove(*Src, MI)) + return false; + + // Move Src down to MI, then replace all uses of MI with it. + Src->moveBefore(&MI); + + Src->getOperand(1).setReg(Passthru.getReg()); + // If Src is masked then its passthru needs to be in VRNoV0. + if (Passthru.getReg() != RISCV::NoRegister) + MRI->constrainRegClass(Passthru.getReg(), + TII->getRegClass(Src->getDesc(), 1, TRI, + *Src->getParent()->getParent())); + + if (MinVL->isImm()) + SrcVL.ChangeToImmediate(MinVL->getImm()); + else if (MinVL->isReg()) + SrcVL.ChangeToRegister(MinVL->getReg(), false); + + // Use a conservative tu,mu policy, RISCVInsertVSETVLI will relax it if + // passthru is undef. + Src->getOperand(RISCVII::getVecPolicyOpNum(Src->getDesc())) + .setImm(RISCVII::TAIL_UNDISTURBED_MASK_UNDISTURBED); + + MRI->replaceRegWith(MI.getOperand(0).getReg(), Src->getOperand(0).getReg()); + MI.eraseFromParent(); + V0Defs.erase(&MI); + + return true; +} + bool RISCVVectorPeephole::runOnMachineFunction(MachineFunction &MF) { if (skipFunction(MF.getFunction())) return false; @@ -358,11 +496,12 @@ bool RISCVVectorPeephole::runOnMachineFunction(MachineFunction &MF) { } for (MachineBasicBlock &MBB : MF) { - for (MachineInstr &MI : MBB) { + for (MachineInstr &MI : make_early_inc_range(MBB)) { Changed |= convertToVLMAX(MI); Changed |= convertToUnmasked(MI); Changed |= convertToWholeRegister(MI); Changed |= convertVMergeToVMv(MI); + Changed |= foldVMV_V_V(MI); } } From 957d7f5477891a12355b8ccf28677c70808ff233 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Tue, 30 Jul 2024 12:44:23 +0800 Subject: [PATCH 2/6] Address some review comments (still more to come) - Update comments - Move one use check - Rename helper to make it more clear it's only being used to check ActiveElementsAffectResult - Only change passthru if needed --- llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp | 57 ++++++++++--------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp index fbb238cd3eeac..f9b162e47f82d 100644 --- a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp +++ b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp @@ -359,51 +359,46 @@ static bool isSafeToMove(const MachineInstr &From, const MachineInstr &To) { break; } } - return From.isSafeToMove(nullptr, SawStore); + return From.isSafeToMove(SawStore); } -static const RISCV::RISCVMaskedPseudoInfo * -lookupMaskedPseudoInfo(const MachineInstr &MI) { +static std::optional +lookupActiveElementsAffectsResult(const MachineInstr &MI) { const RISCV::RISCVMaskedPseudoInfo *Info = RISCV::lookupMaskedIntrinsicByUnmasked(MI.getOpcode()); if (!Info) Info = RISCV::getMaskedPseudoInfo(MI.getOpcode()); - return Info; + if (!Info) + return std::nullopt; + return Info->ActiveElementsAffectResult; } -/// If a PseudoVMV_V_V is the only user of it's input, fold its passthru and VL +/// If a PseudoVMV_V_V is the only user of its input, fold its passthru and VL /// into it. /// -/// %x = PseudoVADD_V_V_M1 %passthru, %a, %b, %vl, sew, policy -/// %y = PseudoVMV_V_V_M1 %passthru, %x, %vl, sew, policy +/// %x = PseudoVADD_V_V_M1 %passthru, %a, %b, %vl1, sew, policy +/// %y = PseudoVMV_V_V_M1 %passthru, %x, %vl2, sew, policy /// /// -> /// -/// %y = PseudoVADD_V_V_M1 %passthru, %a, %b, %vl, sew, policy +/// %y = PseudoVADD_V_V_M1 %passthru, %a, %b, min(vl1, vl2), sew, policy bool RISCVVectorPeephole::foldVMV_V_V(MachineInstr &MI) { if (RISCV::getRVVMCOpcode(MI.getOpcode()) != RISCV::VMV_V_V) return false; MachineOperand &Passthru = MI.getOperand(1); - MachineInstr *Src = MRI->getVRegDef(MI.getOperand(2).getReg()); if (!MRI->hasOneUse(MI.getOperand(2).getReg())) return false; + MachineInstr *Src = MRI->getVRegDef(MI.getOperand(2).getReg()); if (!Src || Src->hasUnmodeledSideEffects() || - Src->getParent() != MI.getParent()) - return false; - - // Src needs to be a pseudo that's opted into this transform. - const RISCV::RISCVMaskedPseudoInfo *Info = lookupMaskedPseudoInfo(*Src); - if (!Info) + Src->getParent() != MI.getParent() || Src->getNumDefs() != 1 || + !RISCVII::isFirstDefTiedToFirstUse(Src->getDesc()) || + !RISCVII::hasVLOp(Src->getDesc().TSFlags) || + !RISCVII::hasVecPolicyOp(Src->getDesc().TSFlags)) return false; - assert(Src->getNumDefs() == 1 && - RISCVII::isFirstDefTiedToFirstUse(Src->getDesc()) && - RISCVII::hasVLOp(Src->getDesc().TSFlags) && - RISCVII::hasVecPolicyOp(Src->getDesc().TSFlags)); - // Src needs to have the same passthru as VMV_V_V if (Src->getOperand(1).getReg() != RISCV::NoRegister && Src->getOperand(1).getReg() != Passthru.getReg()) @@ -429,21 +424,27 @@ bool RISCVVectorPeephole::foldVMV_V_V(MachineInstr &MI) { bool VLChanged = !MinVL->isIdenticalTo(SrcVL); bool RaisesFPExceptions = MI.getDesc().mayRaiseFPException() && !MI.getFlag(MachineInstr::MIFlag::NoFPExcept); - if (VLChanged && (Info->ActiveElementsAffectResult || RaisesFPExceptions)) + auto ActiveElementsAffectResult = lookupActiveElementsAffectsResult(*Src); + if (!ActiveElementsAffectResult) + return false; + if (VLChanged && (*ActiveElementsAffectResult || RaisesFPExceptions)) return false; if (!isSafeToMove(*Src, MI)) return false; - // Move Src down to MI, then replace all uses of MI with it. + // Move Src down to MI so it can access its passthru/VL, then replace all uses + // of MI with it. Src->moveBefore(&MI); - Src->getOperand(1).setReg(Passthru.getReg()); - // If Src is masked then its passthru needs to be in VRNoV0. - if (Passthru.getReg() != RISCV::NoRegister) - MRI->constrainRegClass(Passthru.getReg(), - TII->getRegClass(Src->getDesc(), 1, TRI, - *Src->getParent()->getParent())); + if (Src->getOperand(1).getReg() != Passthru.getReg()) { + Src->getOperand(1).setReg(Passthru.getReg()); + // If Src is masked then its passthru needs to be in VRNoV0. + if (Passthru.getReg() != RISCV::NoRegister) + MRI->constrainRegClass(Passthru.getReg(), + TII->getRegClass(Src->getDesc(), 1, TRI, + *Src->getParent()->getParent())); + } if (MinVL->isImm()) SrcVL.ChangeToImmediate(MinVL->getImm()); From a621896fa71fbe9639436e0dacde8f2289901d99 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Wed, 31 Jul 2024 15:27:58 +0800 Subject: [PATCH 3/6] Rebase, carry over the EEW check added in #101152 Since we don't have access to the MVTs, we can also do the same thing by checking the VLMAXs are the same (i.e. the sew/lmul ratio) --- llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp index f9b162e47f82d..ea4fcc2681939 100644 --- a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp +++ b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp @@ -373,6 +373,12 @@ lookupActiveElementsAffectsResult(const MachineInstr &MI) { return Info->ActiveElementsAffectResult; } +static unsigned getSEWLMULRatio(const MachineInstr &MI) { + RISCVII::VLMUL LMUL = RISCVII::getLMul(MI.getDesc().TSFlags); + unsigned Log2SEW = MI.getOperand(RISCVII::getSEWOpNum(MI.getDesc())).getImm(); + return RISCVVType::getSEWLMULRatio(1 << Log2SEW, LMUL); +} + /// If a PseudoVMV_V_V is the only user of its input, fold its passthru and VL /// into it. /// @@ -399,6 +405,10 @@ bool RISCVVectorPeephole::foldVMV_V_V(MachineInstr &MI) { !RISCVII::hasVecPolicyOp(Src->getDesc().TSFlags)) return false; + // Src needs to have the same VLMAX as MI + if (getSEWLMULRatio(MI) != getSEWLMULRatio(*Src)) + return false; + // Src needs to have the same passthru as VMV_V_V if (Src->getOperand(1).getReg() != RISCV::NoRegister && Src->getOperand(1).getReg() != Passthru.getReg()) From bcd80a77e8acd9ba76bdf19f9dd86ed9e661de27 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Mon, 5 Aug 2024 12:39:59 +0800 Subject: [PATCH 4/6] Update to use ActiveElementsAffectResults TSFlag --- llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp index ea4fcc2681939..023e2c7cf7501 100644 --- a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp +++ b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp @@ -362,17 +362,6 @@ static bool isSafeToMove(const MachineInstr &From, const MachineInstr &To) { return From.isSafeToMove(SawStore); } -static std::optional -lookupActiveElementsAffectsResult(const MachineInstr &MI) { - const RISCV::RISCVMaskedPseudoInfo *Info = - RISCV::lookupMaskedIntrinsicByUnmasked(MI.getOpcode()); - if (!Info) - Info = RISCV::getMaskedPseudoInfo(MI.getOpcode()); - if (!Info) - return std::nullopt; - return Info->ActiveElementsAffectResult; -} - static unsigned getSEWLMULRatio(const MachineInstr &MI) { RISCVII::VLMUL LMUL = RISCVII::getLMul(MI.getDesc().TSFlags); unsigned Log2SEW = MI.getOperand(RISCVII::getSEWOpNum(MI.getDesc())).getImm(); @@ -434,10 +423,10 @@ bool RISCVVectorPeephole::foldVMV_V_V(MachineInstr &MI) { bool VLChanged = !MinVL->isIdenticalTo(SrcVL); bool RaisesFPExceptions = MI.getDesc().mayRaiseFPException() && !MI.getFlag(MachineInstr::MIFlag::NoFPExcept); - auto ActiveElementsAffectResult = lookupActiveElementsAffectsResult(*Src); - if (!ActiveElementsAffectResult) - return false; - if (VLChanged && (*ActiveElementsAffectResult || RaisesFPExceptions)) + bool ActiveElementsAffectResult = RISCVII::activeElementsAffectResult( + TII->get(RISCV::getRVVMCOpcode(Src->getOpcode())).TSFlags); + + if (VLChanged && (ActiveElementsAffectResult || RaisesFPExceptions)) return false; if (!isSafeToMove(*Src, MI)) From 8bb02da412c4f59017b02134d0102ec78dfbfe17 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Fri, 16 Aug 2024 12:31:56 +0800 Subject: [PATCH 5/6] Remove redundant (and incorrect) RaisesFPExceptions check We check isSafeToMove on Src which already checks mayRaiseFPException (which in turn, also already checks for NoFPExcept --- llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp index 023e2c7cf7501..3b7f6664b619b 100644 --- a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp +++ b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp @@ -421,14 +421,13 @@ bool RISCVVectorPeephole::foldVMV_V_V(MachineInstr &MI) { return false; bool VLChanged = !MinVL->isIdenticalTo(SrcVL); - bool RaisesFPExceptions = MI.getDesc().mayRaiseFPException() && - !MI.getFlag(MachineInstr::MIFlag::NoFPExcept); bool ActiveElementsAffectResult = RISCVII::activeElementsAffectResult( TII->get(RISCV::getRVVMCOpcode(Src->getOpcode())).TSFlags); - if (VLChanged && (ActiveElementsAffectResult || RaisesFPExceptions)) + if (VLChanged && ActiveElementsAffectResult) return false; + // Check if any physical register uses would get clobbered (e.g fflags) if (!isSafeToMove(*Src, MI)) return false; From f41e0f2e0545bd161fe88aa5afeb146bdd4c0171 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Fri, 16 Aug 2024 13:22:00 +0800 Subject: [PATCH 6/6] Only move Src if needed Because of this, we don't always call isSafeToMove so add back in the mayRaiseFPException check --- llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp index 3b7f6664b619b..2abed1ac984e3 100644 --- a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp +++ b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp @@ -399,8 +399,9 @@ bool RISCVVectorPeephole::foldVMV_V_V(MachineInstr &MI) { return false; // Src needs to have the same passthru as VMV_V_V - if (Src->getOperand(1).getReg() != RISCV::NoRegister && - Src->getOperand(1).getReg() != Passthru.getReg()) + MachineOperand &SrcPassthru = Src->getOperand(1); + if (SrcPassthru.getReg() != RISCV::NoRegister && + SrcPassthru.getReg() != Passthru.getReg()) return false; // Because Src and MI have the same passthru, we can use either AVL as long as @@ -424,19 +425,19 @@ bool RISCVVectorPeephole::foldVMV_V_V(MachineInstr &MI) { bool ActiveElementsAffectResult = RISCVII::activeElementsAffectResult( TII->get(RISCV::getRVVMCOpcode(Src->getOpcode())).TSFlags); - if (VLChanged && ActiveElementsAffectResult) + if (VLChanged && (ActiveElementsAffectResult || Src->mayRaiseFPException())) return false; - // Check if any physical register uses would get clobbered (e.g fflags) - if (!isSafeToMove(*Src, MI)) - return false; - - // Move Src down to MI so it can access its passthru/VL, then replace all uses - // of MI with it. - Src->moveBefore(&MI); + // If Src ends up using MI's passthru/VL, move it so it can access it. + // TODO: We don't need to do this if they already dominate Src. + if (!SrcVL.isIdenticalTo(*MinVL) || !SrcPassthru.isIdenticalTo(Passthru)) { + if (!isSafeToMove(*Src, MI)) + return false; + Src->moveBefore(&MI); + } - if (Src->getOperand(1).getReg() != Passthru.getReg()) { - Src->getOperand(1).setReg(Passthru.getReg()); + if (SrcPassthru.getReg() != Passthru.getReg()) { + SrcPassthru.setReg(Passthru.getReg()); // If Src is masked then its passthru needs to be in VRNoV0. if (Passthru.getReg() != RISCV::NoRegister) MRI->constrainRegClass(Passthru.getReg(),