[llvm] [RISCV] Move vmv.v.v peephole from SelectionDAG to RISCVVectorPeephole (PR #100367)

Luke Lau via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 15 22:22:40 PDT 2024


https://github.com/lukel97 updated https://github.com/llvm/llvm-project/pull/100367

>From 6208fcd1e5bef0066c449bac4e21039b3b335891 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
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 aed10c2de43720..63ecb654578682 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<RegisterSDNode>(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<RegisterSDNode>(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<const SDNode *, 4> LoopWorklist;
     SmallPtrSet<const SDNode *, 16> 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 979677ee92332d..fbb238cd3eeac7 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<unsigned> 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<Register> 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 <luke at igalia.com>
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 fbb238cd3eeac7..f9b162e47f82de 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<bool>
+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 <luke at igalia.com>
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 f9b162e47f82de..ea4fcc2681939d 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 <luke at igalia.com>
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 ea4fcc2681939d..023e2c7cf7501e 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<bool>
-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 <luke at igalia.com>
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 023e2c7cf7501e..3b7f6664b619b0 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 <luke at igalia.com>
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 3b7f6664b619b0..2abed1ac984e35 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(),



More information about the llvm-commits mailing list