[RISCV] Remove uses of RISCVII::hasMergeOp from RISCVDAGToDAGISel.cpp

[craig.topper]

Summary

This property was intended to indicate when RISCVAsmPrinter should
drop the tied source operand when converting to MCInst. Using it
in RISCVDAGToDAGISel distorts what it intended for.

This should remove some changes from D151850.

[frasercrmck]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

3195

Might this result in an not-used-in-release warning? I'm not sure what the current idiom is - [[maybe_unused]] or cast to void?

3250–3251

Comment needs updating

[reames]

Should we introduce a separate hasTiedDest utility? (With possibly a better chosen name?) I think having that might help clarify the difference between the check being performed here and the hasMergeOp routine. I have to admit I'm not completely following your semantic difference, so having a place to explain that in comments might help.

[craig.topper]

In D152039#4396229, @reames wrote:

Should we introduce a separate hasTiedDest utility? (With possibly a better chosen name?) I think having that might help clarify the difference between the check being performed here and the hasMergeOp routine. I have to admit I'm not completely following your semantic difference, so having a place to explain that in comments might help.

Maybe the name hasMergeOp is the problem because the documentation did say what it meant.

// Does this instruction have a merge operand that must be removed when        
// converting to MCInst. It will be the first explicit use operand. Used by    
// RVV Pseudos.                                                                
HasMergeOpShift = ForceTailAgnosticShift + 1,                                  
HasMergeOpMask = 1 << HasMergeOpShift,

I think I can remove it from RISCVMCInstLowering too so the distinction won't matter soon.

[craig.topper]

Address Fraser's review comments.

[craig.topper]

Add helper method as suggested by @reames

[asb]

LGTM.

[frasercrmck]

LGTM

llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h

static inline unsigned getSEWOpNum(const MCInstrDesc &Desc) {
   return Desc.getNumOperands() - Offset;
 }
 
 static inline unsigned getVecPolicyOpNum(const MCInstrDesc &Desc) {
   assert(hasVecPolicyOp(Desc.TSFlags));
   return Desc.getNumOperands() - 1;
 }
 
+// Is the first def operand tied to the first use operand. This is true for
+// vector pseudo instructions that have a merge operand for tail/mask
+// undisturbed. It's also true for vector FMA instructions where one of the
+// operands is also the destination register. This is different than
+// RISCVII::hasMergeOp which only indicates whether the tied operand from the
+// pseudoinstruction also exists on the MC layer instruction.
+static inline bool isFirstDefTiedToFirstUse(const MCInstrDesc &Desc) {
+  return Desc.getNumDefs() < Desc.getNumOperands() &&
+         Desc.getOperandConstraint(Desc.getNumDefs(), MCOI::TIED_TO) == 0;
+}
+
 // RISC-V Specific Machine Operand Flags
 enum {
   MO_None = 0,
   MO_CALL = 1,
   MO_PLT = 2,
   MO_LO = 3,
   MO_HI = 4,
   MO_PCREL_LO = 5,

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

if (RISCVII::hasVecPolicyOp(MaskedMCID.TSFlags)) {
     }
   }
 
   unsigned Opc = UseTUPseudo ? I->UnmaskedTUPseudo : I->UnmaskedPseudo;
 
   // Check that we're dropping the mask operand and any policy operand
   // when we transform to this unmasked pseudo. Additionally, if this
   // instruction is tail agnostic, the unmasked instruction should not have a
-  // merge op.
-  uint64_t TSFlags = TII.get(Opc).TSFlags;
-  assert((UseTUPseudo == RISCVII::hasMergeOp(TSFlags)) &&
-         RISCVII::hasDummyMaskOp(TSFlags) &&
+  // tied destination.
+#ifndef NDEBUG
+  const MCInstrDesc &MCID = TII.get(Opc);
+  uint64_t TSFlags = MCID.TSFlags;
+  bool HasTiedDest = RISCVII::isFirstDefTiedToFirstUse(MCID);
+  assert(UseTUPseudo == HasTiedDest && RISCVII::hasDummyMaskOp(TSFlags) &&
          "Unexpected pseudo to transform to");
-  (void)TSFlags;
+#endif
 
   SmallVector<SDValue, 8> Ops;
   // Skip the merge operand at index 0 if !UseTUPseudo.
   for (unsigned I = !UseTUPseudo, E = N->getNumOperands(); I != E; I++) {
     // Skip the mask, the policy, and the Glue.
     SDValue Op = N->getOperand(I);
     if (I == MaskOpIdx || I == TailPolicyOpIdx ||
         Op.getValueType() == MVT::Glue)
bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N, bool IsTA) {
 
   // Need N is the exactly one using True.
   if (!True.hasOneUse())
     return false;
 
   if (!True.isMachineOpcode())
     return false;
 
   unsigned TrueOpc = True.getMachineOpcode();
-
-  // Skip if True has merge operand.
-  uint64_t TrueTSFlags = TII->get(TrueOpc).TSFlags;
-  bool HasMergeOp = RISCVII::hasMergeOp(TrueTSFlags);
+  const MCInstrDesc &TrueMCID = TII->get(TrueOpc);
+  uint64_t TrueTSFlags = TrueMCID.TSFlags;
+  bool HasTiedDest = RISCVII::isFirstDefTiedToFirstUse(TrueMCID);
 
   bool IsMasked = false;
   const RISCV::RISCVMaskedPseudoInfo *Info =
       RISCV::lookupMaskedIntrinsicByUnmaskedTA(TrueOpc);
-  if (!Info && HasMergeOp) {
+  if (!Info && HasTiedDest) {
     Info = RISCV::getMaskedPseudoInfo(TrueOpc);
     IsMasked = true;
   }
 
   if (!Info)
     return false;
 
-  if (HasMergeOp) {
+  if (HasTiedDest) {
     // The vmerge instruction must be TU.
     // FIXME: This could be relaxed, but we need to handle the policy for the
     // resulting op correctly.
     if (IsTA)
       return false;
     SDValue MergeOpTrue = True->getOperand(0);
     // Both the vmerge instruction and the True instruction must have the same
     // merge operand.
     if (False != MergeOpTrue)
       return false;
   }
 
   if (IsMasked) {
-    assert(HasMergeOp && "Expected merge op");
+    assert(HasTiedDest && "Expected tied dest");
     // The vmerge instruction must be TU.
     if (IsTA)
       return false;
     // The vmerge instruction must have an all 1s mask since we're going to keep
     // the mask from the True instruction.
     // FIXME: Support mask agnostic True instruction which would have an
     // undef merge operand.
     if (!usesAllOnesMask(N, /* MaskOpIdx */ 3))
bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N, bool IsTA) {
   // Allow the peephole for non-exception True with VLMAX vector length, since
   // all the values after VL of N are dependent on Merge. VLMAX should be
   // lowered to (XLenVT -1).
   if (TrueVL != VL && !(IsNoFPExcept(True) && isAllOnesConstant(TrueVL)))
     return false;
 
   SDLoc DL(N);
   unsigned MaskedOpc = Info->MaskedPseudo;
-  assert(RISCVII::hasVecPolicyOp(TII->get(MaskedOpc).TSFlags) &&
+#ifndef NDEBUG
+  const MCInstrDesc &MaskedMCID = TII->get(MaskedOpc);
+  assert(RISCVII::hasVecPolicyOp(MaskedMCID.TSFlags) &&
          "Expected instructions with mask have policy operand.");
-  assert(RISCVII::hasMergeOp(TII->get(MaskedOpc).TSFlags) &&
-         "Expected instructions with mask have merge operand.");
+  assert(MaskedMCID.getOperandConstraint(MaskedMCID.getNumDefs(),
+                                         MCOI::TIED_TO) == 0 &&
+         "Expected instructions with mask have a tied dest.");
+#endif
 
   SmallVector<SDValue, 8> Ops;
   if (IsMasked) {
     Ops.append(True->op_begin(), True->op_begin() + TrueVLIndex);
     Ops.append({VL, /* SEW */ True.getOperand(TrueVLIndex + 1)});
     Ops.push_back(
         CurDAG->getTargetConstant(Policy, DL, Subtarget->getXLenVT()));
     Ops.append(True->op_begin() + TrueVLIndex + 3, True->op_end());
   } else {
-    if (!HasMergeOp)
+    if (!HasTiedDest)
       Ops.push_back(False);
     Ops.append(True->op_begin(), True->op_begin() + TrueVLIndex);
     Ops.append({Mask, VL, /* SEW */ True.getOperand(TrueVLIndex + 1)});
     Ops.push_back(
         CurDAG->getTargetConstant(Policy, DL, Subtarget->getXLenVT()));
 
     // Result node should have chain operand of True.
     if (HasChainOp)