[llvm] 1294407 - [RISCV] Partially move doPeepholeMaskedRVV into RISCVFoldMasks (#72441)

[via llvm-commits]

Author: Philip Reames
Date: 2023-11-27T08:33:03-08:00
New Revision: 129440728ce3e688969ed253a6709a5d50c5e850

URL: https://github.com/llvm/llvm-project/commit/129440728ce3e688969ed253a6709a5d50c5e850
DIFF: https://github.com/llvm/llvm-project/commit/129440728ce3e688969ed253a6709a5d50c5e850.diff

LOG: [RISCV] Partially move doPeepholeMaskedRVV into RISCVFoldMasks (#72441)

This change is motived by a point of confusion on
https://github.com/llvm/llvm-project/pull/71764. I hadn't fully
understood why doPeepholeMaskedRVV needed to be part of the same change.
As indicated in the fixme in this patch, the reason is that
performCombineVMergeAndVOps doesn't know how to deal with the true side
of the merge being a all-ones masked instruction.

This change removes one of two calls to the routine in
RISCVISELDAGToDAG, and adds a clarifying comment on the precondition for
the remaining call. The post-ISEL code is tested by the cases where we
can form a unmasked instruction after folding the vmerge back into true.

I don't really care if we actually land this patch, or leave it roled
into https://github.com/llvm/llvm-project/pull/71764. I'm posting it
mostly to clarify the confusion.

Added: 
    

Modified: 
    llvm/lib/Target/RISCV/RISCVFoldMasks.cpp
    llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/RISCV/RISCVFoldMasks.cpp b/llvm/lib/Target/RISCV/RISCVFoldMasks.cpp
index 63849238f9ec7c2..4586a4a0c72ed39 100644
--- a/llvm/lib/Target/RISCV/RISCVFoldMasks.cpp
+++ b/llvm/lib/Target/RISCV/RISCVFoldMasks.cpp
@@ -18,6 +18,7 @@
 
 #include "RISCV.h"
 #include "RISCVSubtarget.h"
+#include "RISCVISelDAGToDAG.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"
@@ -48,6 +49,7 @@ class RISCVFoldMasks : public MachineFunctionPass {
   StringRef getPassName() const override { return "RISC-V Fold Masks"; }
 
 private:
+  bool convertToUnmasked(MachineInstr &MI, MachineInstr *MaskDef);
   bool convertVMergeToVMv(MachineInstr &MI, MachineInstr *MaskDef);
 
   bool isAllOnesMask(MachineInstr *MaskDef);
@@ -132,6 +134,49 @@ bool RISCVFoldMasks::convertVMergeToVMv(MachineInstr &MI, MachineInstr *V0Def) {
   return true;
 }
 
+bool RISCVFoldMasks::convertToUnmasked(MachineInstr &MI,
+                                       MachineInstr *MaskDef) {
+  const RISCV::RISCVMaskedPseudoInfo *I =
+      RISCV::getMaskedPseudoInfo(MI.getOpcode());
+  if (!I)
+    return false;
+
+  if (!isAllOnesMask(MaskDef))
+    return false;
+
+  // There are two classes of pseudos in the table - compares and
+  // everything else.  See the comment on RISCVMaskedPseudo for details.
+  const unsigned Opc = I->UnmaskedPseudo;
+  const MCInstrDesc &MCID = TII->get(Opc);
+  const bool HasPolicyOp = RISCVII::hasVecPolicyOp(MCID.TSFlags);
+  const bool HasPassthru = RISCVII::isFirstDefTiedToFirstUse(MCID);
+#ifndef NDEBUG
+  const MCInstrDesc &MaskedMCID = TII->get(MI.getOpcode());
+  assert(RISCVII::hasVecPolicyOp(MaskedMCID.TSFlags) ==
+             RISCVII::hasVecPolicyOp(MCID.TSFlags) &&
+         "Masked and unmasked pseudos are inconsistent");
+  assert(HasPolicyOp == HasPassthru && "Unexpected pseudo structure");
+#endif
+
+  MI.setDesc(MCID);
+
+  // TODO: Increment all MaskOpIdxs in tablegen by num of explicit defs?
+  unsigned MaskOpIdx = I->MaskOpIdx + MI.getNumExplicitDefs();
+  MI.removeOperand(MaskOpIdx);
+
+  // The unmasked pseudo will no longer be constrained to the vrnov0 reg class,
+  // so try and relax it to vr.
+  MRI->recomputeRegClass(MI.getOperand(0).getReg());
+  unsigned PassthruOpIdx = MI.getNumExplicitDefs();
+  if (HasPassthru) {
+    if (MI.getOperand(PassthruOpIdx).getReg() != RISCV::NoRegister)
+      MRI->recomputeRegClass(MI.getOperand(PassthruOpIdx).getReg());
+  } else
+    MI.removeOperand(PassthruOpIdx);
+
+  return true;
+}
+
 bool RISCVFoldMasks::runOnMachineFunction(MachineFunction &MF) {
   if (skipFunction(MF.getFunction()))
     return false;
@@ -159,6 +204,7 @@ bool RISCVFoldMasks::runOnMachineFunction(MachineFunction &MF) {
     CurrentV0Def = nullptr;
     for (MachineInstr &MI : MBB) {
       unsigned BaseOpc = RISCV::getRVVMCOpcode(MI.getOpcode());
+      Changed |= convertToUnmasked(MI, CurrentV0Def);
       if (BaseOpc == RISCV::VMERGE_VVM)
         Changed |= convertVMergeToVMv(MI, CurrentV0Def);
 

diff  --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
index 605a54071ea282e..ae7f1743b523937 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -151,6 +151,10 @@ void RISCVDAGToDAGISel::PostprocessISelDAG() {
       continue;
 
     MadeChange |= doPeepholeSExtW(N);
+
+    // FIXME: This is here only because the VMerge transform doesn't
+    // know how to handle masked true inputs.  Once that has been moved
+    // to post-ISEL, this can be deleted as well.
     MadeChange |= doPeepholeMaskedRVV(cast<MachineSDNode>(N));
   }
 
@@ -3711,8 +3715,6 @@ bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N) {
   for (unsigned Idx = 1; Idx < True->getNumValues(); ++Idx)
     ReplaceUses(True.getValue(Idx), SDValue(Result, Idx));
 
-  // Try to transform Result to unmasked intrinsic.
-  doPeepholeMaskedRVV(Result);
   return true;
 }