Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV] Move vector pseudo hasAllNBitUsers switch into RISCVInstrInfo. NFC #67593

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

lukel97
Copy link
Contributor

@lukel97 lukel97 commented Sep 27, 2023

The handling for vector pseudos in hasAllNBitUsers is duplicated across
RISCVISelDAGToDAG and RISCVOptWInstrs. This deduplicates it between the two,
with the common denominator between the two call sites being the opcode and
SEW: We need to handle extracting these separately since one operates at the
SelectionDAG level and the other at the MachineInstr level.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 27, 2023

@llvm/pr-subscribers-backend-risc-v

Changes

The handling for vector pseudos in hasAllNBitUsers is duplicated across
RISCVISelDAGToDAG and RISCVOptWInstrs. This deduplicates it between the two,
with the common denominator between the two call sites being the opcode and
SEW: We need to handle extracting these separately since one operates at the
SelectionDAG level and the other at the MachineInstr level.


Full diff: https://github.com/llvm/llvm-project/pull/67593.diff

4 Files Affected:

  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp (+112)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h (+5)
  • (modified) llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp (+25-145)
  • (modified) llvm/lib/Target/RISCV/RISCVOptWInstrs.cpp (+16-138)
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
index 0a42c6faee29008..95cea0c61acfd5d 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
@@ -130,6 +130,118 @@ parseFeatureBits(bool IsRV64, const FeatureBitset &FeatureBits) {
 
 } // namespace RISCVFeatures
 
+bool RISCVII::vectorInstUsesNBitsOfScalarOp(uint16_t Opcode, unsigned Bits,
+                                            unsigned Log2SEW) {
+  // TODO: Handle Zvbb instructions
+  switch (Opcode) {
+  default:
+    return false;
+
+  // 11.6. Vector Single-Width Shift Instructions
+  case RISCV::VSLL_VX:
+  case RISCV::VSRL_VX:
+  case RISCV::VSRA_VX:
+  // 12.4. Vector Single-Width Scaling Shift Instructions
+  case RISCV::VSSRL_VX:
+  case RISCV::VSSRA_VX:
+    // Only the low lg2(SEW) bits of the shift-amount value are used.
+    return Log2SEW <= Bits;
+
+  // 11.7 Vector Narrowing Integer Right Shift Instructions
+  case RISCV::VNSRL_WX:
+  case RISCV::VNSRA_WX:
+  // 12.5. Vector Narrowing Fixed-Point Clip Instructions
+  case RISCV::VNCLIPU_WX:
+  case RISCV::VNCLIP_WX:
+    // Only the low lg2(2*SEW) bits of the shift-amount value are used.
+    return (Log2SEW + 1) <= Bits;
+
+  // 11.1. Vector Single-Width Integer Add and Subtract
+  case RISCV::VADD_VX:
+  case RISCV::VSUB_VX:
+  case RISCV::VRSUB_VX:
+  // 11.2. Vector Widening Integer Add/Subtract
+  case RISCV::VWADDU_VX:
+  case RISCV::VWSUBU_VX:
+  case RISCV::VWADD_VX:
+  case RISCV::VWSUB_VX:
+  case RISCV::VWADDU_WX:
+  case RISCV::VWSUBU_WX:
+  case RISCV::VWADD_WX:
+  case RISCV::VWSUB_WX:
+  // 11.4. Vector Integer Add-with-Carry / Subtract-with-Borrow Instructions
+  case RISCV::VADC_VXM:
+  case RISCV::VADC_VIM:
+  case RISCV::VMADC_VXM:
+  case RISCV::VMADC_VIM:
+  case RISCV::VMADC_VX:
+  case RISCV::VSBC_VXM:
+  case RISCV::VMSBC_VXM:
+  case RISCV::VMSBC_VX:
+  // 11.5 Vector Bitwise Logical Instructions
+  case RISCV::VAND_VX:
+  case RISCV::VOR_VX:
+  case RISCV::VXOR_VX:
+  // 11.8. Vector Integer Compare Instructions
+  case RISCV::VMSEQ_VX:
+  case RISCV::VMSNE_VX:
+  case RISCV::VMSLTU_VX:
+  case RISCV::VMSLT_VX:
+  case RISCV::VMSLEU_VX:
+  case RISCV::VMSLE_VX:
+  case RISCV::VMSGTU_VX:
+  case RISCV::VMSGT_VX:
+  // 11.9. Vector Integer Min/Max Instructions
+  case RISCV::VMINU_VX:
+  case RISCV::VMIN_VX:
+  case RISCV::VMAXU_VX:
+  case RISCV::VMAX_VX:
+  // 11.10. Vector Single-Width Integer Multiply Instructions
+  case RISCV::VMUL_VX:
+  case RISCV::VMULH_VX:
+  case RISCV::VMULHU_VX:
+  case RISCV::VMULHSU_VX:
+  // 11.11. Vector Integer Divide Instructions
+  case RISCV::VDIVU_VX:
+  case RISCV::VDIV_VX:
+  case RISCV::VREMU_VX:
+  case RISCV::VREM_VX:
+  // 11.12. Vector Widening Integer Multiply Instructions
+  case RISCV::VWMUL_VX:
+  case RISCV::VWMULU_VX:
+  case RISCV::VWMULSU_VX:
+  // 11.13. Vector Single-Width Integer Multiply-Add Instructions
+  case RISCV::VMACC_VX:
+  case RISCV::VNMSAC_VX:
+  case RISCV::VMADD_VX:
+  case RISCV::VNMSUB_VX:
+  // 11.14. Vector Widening Integer Multiply-Add Instructions
+  case RISCV::VWMACCU_VX:
+  case RISCV::VWMACC_VX:
+  case RISCV::VWMACCSU_VX:
+  case RISCV::VWMACCUS_VX:
+  // 11.15. Vector Integer Merge Instructions
+  case RISCV::VMERGE_VXM:
+  // 11.16. Vector Integer Move Instructions
+  case RISCV::VMV_V_X:
+  // 12.1. Vector Single-Width Saturating Add and Subtract
+  case RISCV::VSADDU_VX:
+  case RISCV::VSADD_VX:
+  case RISCV::VSSUBU_VX:
+  case RISCV::VSSUB_VX:
+  // 12.2. Vector Single-Width Averaging Add and Subtract
+  case RISCV::VAADDU_VX:
+  case RISCV::VAADD_VX:
+  case RISCV::VASUBU_VX:
+  case RISCV::VASUB_VX:
+  // 12.3. Vector Single-Width Fractional Multiply with Rounding and Saturation
+  case RISCV::VSMUL_VX:
+  // 16.1. Integer Scalar Move Instructions
+  case RISCV::VMV_S_X:
+    return (1 << Log2SEW) <= Bits;
+  }
+}
+
 // Encode VTYPE into the binary format used by the the VSETVLI instruction which
 // is used by our MC layer representation.
 //
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
index 20ff26a39dc3b30..222d4e9eef674ec 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
@@ -241,6 +241,11 @@ static inline bool isFirstDefTiedToFirstUse(const MCInstrDesc &Desc) {
          Desc.getOperandConstraint(Desc.getNumDefs(), MCOI::TIED_TO) == 0;
 }
 
+// Returns true if the .vx vector instruction \p Opcode only uses the lower \p
+// Bits for a given SEW.
+bool vectorInstUsesNBitsOfScalarOp(uint16_t Opcode, unsigned Bits,
+                                   unsigned Log2SEW);
+
 // RISC-V Specific Machine Operand Flags
 enum {
   MO_None = 0,
diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
index 283ab1feda7eca5..b5f91c6bf70b061 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -2753,148 +2753,6 @@ bool RISCVDAGToDAGISel::selectSHXADD_UWOp(SDValue N, unsigned ShAmt,
   return false;
 }
 
-static bool vectorPseudoHasAllNBitUsers(SDNode *User, unsigned UserOpNo,
-                                        unsigned Bits,
-                                        const TargetInstrInfo *TII) {
-  const RISCVVPseudosTable::PseudoInfo *PseudoInfo =
-      RISCVVPseudosTable::getPseudoInfo(User->getMachineOpcode());
-
-  if (!PseudoInfo)
-    return false;
-
-  const MCInstrDesc &MCID = TII->get(User->getMachineOpcode());
-  const uint64_t TSFlags = MCID.TSFlags;
-  if (!RISCVII::hasSEWOp(TSFlags))
-    return false;
-  assert(RISCVII::hasVLOp(TSFlags));
-
-  bool HasGlueOp = User->getGluedNode() != nullptr;
-  unsigned ChainOpIdx = User->getNumOperands() - HasGlueOp - 1;
-  bool HasChainOp = User->getOperand(ChainOpIdx).getValueType() == MVT::Other;
-  bool HasVecPolicyOp = RISCVII::hasVecPolicyOp(TSFlags);
-  unsigned VLIdx =
-      User->getNumOperands() - HasVecPolicyOp - HasChainOp - HasGlueOp - 2;
-  const unsigned Log2SEW = User->getConstantOperandVal(VLIdx + 1);
-
-  if (UserOpNo == VLIdx)
-    return false;
-
-  // TODO: Handle Zvbb instructions
-  switch (PseudoInfo->BaseInstr) {
-  default:
-    return false;
-
-  // 11.6. Vector Single-Width Shift Instructions
-  case RISCV::VSLL_VX:
-  case RISCV::VSRL_VX:
-  case RISCV::VSRA_VX:
-  // 12.4. Vector Single-Width Scaling Shift Instructions
-  case RISCV::VSSRL_VX:
-  case RISCV::VSSRA_VX:
-    // Only the low lg2(SEW) bits of the shift-amount value are used.
-    if (Bits < Log2SEW)
-      return false;
-    break;
-
-  // 11.7 Vector Narrowing Integer Right Shift Instructions
-  case RISCV::VNSRL_WX:
-  case RISCV::VNSRA_WX:
-  // 12.5. Vector Narrowing Fixed-Point Clip Instructions
-  case RISCV::VNCLIPU_WX:
-  case RISCV::VNCLIP_WX:
-    // Only the low lg2(2*SEW) bits of the shift-amount value are used.
-    if (Bits < Log2SEW + 1)
-      return false;
-    break;
-
-  // 11.1. Vector Single-Width Integer Add and Subtract
-  case RISCV::VADD_VX:
-  case RISCV::VSUB_VX:
-  case RISCV::VRSUB_VX:
-  // 11.2. Vector Widening Integer Add/Subtract
-  case RISCV::VWADDU_VX:
-  case RISCV::VWSUBU_VX:
-  case RISCV::VWADD_VX:
-  case RISCV::VWSUB_VX:
-  case RISCV::VWADDU_WX:
-  case RISCV::VWSUBU_WX:
-  case RISCV::VWADD_WX:
-  case RISCV::VWSUB_WX:
-  // 11.4. Vector Integer Add-with-Carry / Subtract-with-Borrow Instructions
-  case RISCV::VADC_VXM:
-  case RISCV::VADC_VIM:
-  case RISCV::VMADC_VXM:
-  case RISCV::VMADC_VIM:
-  case RISCV::VMADC_VX:
-  case RISCV::VSBC_VXM:
-  case RISCV::VMSBC_VXM:
-  case RISCV::VMSBC_VX:
-  // 11.5 Vector Bitwise Logical Instructions
-  case RISCV::VAND_VX:
-  case RISCV::VOR_VX:
-  case RISCV::VXOR_VX:
-  // 11.8. Vector Integer Compare Instructions
-  case RISCV::VMSEQ_VX:
-  case RISCV::VMSNE_VX:
-  case RISCV::VMSLTU_VX:
-  case RISCV::VMSLT_VX:
-  case RISCV::VMSLEU_VX:
-  case RISCV::VMSLE_VX:
-  case RISCV::VMSGTU_VX:
-  case RISCV::VMSGT_VX:
-  // 11.9. Vector Integer Min/Max Instructions
-  case RISCV::VMINU_VX:
-  case RISCV::VMIN_VX:
-  case RISCV::VMAXU_VX:
-  case RISCV::VMAX_VX:
-  // 11.10. Vector Single-Width Integer Multiply Instructions
-  case RISCV::VMUL_VX:
-  case RISCV::VMULH_VX:
-  case RISCV::VMULHU_VX:
-  case RISCV::VMULHSU_VX:
-  // 11.11. Vector Integer Divide Instructions
-  case RISCV::VDIVU_VX:
-  case RISCV::VDIV_VX:
-  case RISCV::VREMU_VX:
-  case RISCV::VREM_VX:
-  // 11.12. Vector Widening Integer Multiply Instructions
-  case RISCV::VWMUL_VX:
-  case RISCV::VWMULU_VX:
-  case RISCV::VWMULSU_VX:
-  // 11.13. Vector Single-Width Integer Multiply-Add Instructions
-  case RISCV::VMACC_VX:
-  case RISCV::VNMSAC_VX:
-  case RISCV::VMADD_VX:
-  case RISCV::VNMSUB_VX:
-  // 11.14. Vector Widening Integer Multiply-Add Instructions
-  case RISCV::VWMACCU_VX:
-  case RISCV::VWMACC_VX:
-  case RISCV::VWMACCSU_VX:
-  case RISCV::VWMACCUS_VX:
-  // 11.15. Vector Integer Merge Instructions
-  case RISCV::VMERGE_VXM:
-  // 11.16. Vector Integer Move Instructions
-  case RISCV::VMV_V_X:
-  // 12.1. Vector Single-Width Saturating Add and Subtract
-  case RISCV::VSADDU_VX:
-  case RISCV::VSADD_VX:
-  case RISCV::VSSUBU_VX:
-  case RISCV::VSSUB_VX:
-  // 12.2. Vector Single-Width Averaging Add and Subtract
-  case RISCV::VAADDU_VX:
-  case RISCV::VAADD_VX:
-  case RISCV::VASUBU_VX:
-  case RISCV::VASUB_VX:
-  // 12.3. Vector Single-Width Fractional Multiply with Rounding and Saturation
-  case RISCV::VSMUL_VX:
-  // 16.1. Integer Scalar Move Instructions
-  case RISCV::VMV_S_X:
-    if (Bits < (1 << Log2SEW))
-      return false;
-  }
-  return true;
-}
-
 // Return true if all users of this SDNode* only consume the lower \p Bits.
 // This can be used to form W instructions for add/sub/mul/shl even when the
 // root isn't a sext_inreg. This can allow the ADDW/SUBW/MULW/SLLIW to CSE if
@@ -2925,10 +2783,32 @@ bool RISCVDAGToDAGISel::hasAllNBitUsers(SDNode *Node, unsigned Bits,
 
     // TODO: Add more opcodes?
     switch (User->getMachineOpcode()) {
-    default:
-      if (vectorPseudoHasAllNBitUsers(User, UI.getOperandNo(), Bits, TII))
-        break;
+    default: {
+      if (const RISCVVPseudosTable::PseudoInfo *PseudoInfo =
+              RISCVVPseudosTable::getPseudoInfo(User->getMachineOpcode())) {
+
+        const MCInstrDesc &MCID = TII->get(User->getMachineOpcode());
+        if (!RISCVII::hasSEWOp(MCID.TSFlags))
+          return false;
+        assert(RISCVII::hasVLOp(MCID.TSFlags));
+
+        bool HasGlueOp = User->getGluedNode() != nullptr;
+        unsigned ChainOpIdx = User->getNumOperands() - HasGlueOp - 1;
+        bool HasChainOp =
+            User->getOperand(ChainOpIdx).getValueType() == MVT::Other;
+        bool HasVecPolicyOp = RISCVII::hasVecPolicyOp(MCID.TSFlags);
+        unsigned VLIdx = User->getNumOperands() - HasVecPolicyOp - HasChainOp -
+                         HasGlueOp - 2;
+        const unsigned Log2SEW = User->getConstantOperandVal(VLIdx + 1);
+
+        if (UI.getOperandNo() == VLIdx)
+          return false;
+        if (RISCVII::vectorInstUsesNBitsOfScalarOp(PseudoInfo->BaseInstr, Bits,
+                                                   Log2SEW))
+          break;
+      }
       return false;
+    }
     case RISCV::ADDW:
     case RISCV::ADDIW:
     case RISCV::SUBW:
diff --git a/llvm/lib/Target/RISCV/RISCVOptWInstrs.cpp b/llvm/lib/Target/RISCV/RISCVOptWInstrs.cpp
index 0cbdfa84640bf91..56aa5589bc19a29 100644
--- a/llvm/lib/Target/RISCV/RISCVOptWInstrs.cpp
+++ b/llvm/lib/Target/RISCV/RISCVOptWInstrs.cpp
@@ -78,141 +78,6 @@ FunctionPass *llvm::createRISCVOptWInstrsPass() {
   return new RISCVOptWInstrs();
 }
 
-static bool vectorPseudoHasAllNBitUsers(const MachineOperand &UserOp,
-                                        unsigned Bits) {
-  const MachineInstr &MI = *UserOp.getParent();
-  const RISCVVPseudosTable::PseudoInfo *PseudoInfo =
-      RISCVVPseudosTable::getPseudoInfo(MI.getOpcode());
-
-  if (!PseudoInfo)
-    return false;
-
-  const MCInstrDesc &MCID = MI.getDesc();
-  const uint64_t TSFlags = MI.getDesc().TSFlags;
-  if (!RISCVII::hasSEWOp(TSFlags))
-    return false;
-  assert(RISCVII::hasVLOp(TSFlags));
-  const unsigned Log2SEW = MI.getOperand(RISCVII::getSEWOpNum(MCID)).getImm();
-
-  if (UserOp.getOperandNo() == RISCVII::getVLOpNum(MCID))
-    return false;
-
-  // TODO: Handle Zvbb instructions
-  switch (PseudoInfo->BaseInstr) {
-  default:
-    return false;
-
-  // 11.6. Vector Single-Width Shift Instructions
-  case RISCV::VSLL_VX:
-  case RISCV::VSRL_VX:
-  case RISCV::VSRA_VX:
-  // 12.4. Vector Single-Width Scaling Shift Instructions
-  case RISCV::VSSRL_VX:
-  case RISCV::VSSRA_VX:
-    // Only the low lg2(SEW) bits of the shift-amount value are used.
-    if (Bits < Log2SEW)
-      return false;
-    break;
-
-  // 11.7 Vector Narrowing Integer Right Shift Instructions
-  case RISCV::VNSRL_WX:
-  case RISCV::VNSRA_WX:
-  // 12.5. Vector Narrowing Fixed-Point Clip Instructions
-  case RISCV::VNCLIPU_WX:
-  case RISCV::VNCLIP_WX:
-    // Only the low lg2(2*SEW) bits of the shift-amount value are used.
-    if (Bits < Log2SEW + 1)
-      return false;
-    break;
-
-  // 11.1. Vector Single-Width Integer Add and Subtract
-  case RISCV::VADD_VX:
-  case RISCV::VSUB_VX:
-  case RISCV::VRSUB_VX:
-  // 11.2. Vector Widening Integer Add/Subtract
-  case RISCV::VWADDU_VX:
-  case RISCV::VWSUBU_VX:
-  case RISCV::VWADD_VX:
-  case RISCV::VWSUB_VX:
-  case RISCV::VWADDU_WX:
-  case RISCV::VWSUBU_WX:
-  case RISCV::VWADD_WX:
-  case RISCV::VWSUB_WX:
-  // 11.4. Vector Integer Add-with-Carry / Subtract-with-Borrow Instructions
-  case RISCV::VADC_VXM:
-  case RISCV::VADC_VIM:
-  case RISCV::VMADC_VXM:
-  case RISCV::VMADC_VIM:
-  case RISCV::VMADC_VX:
-  case RISCV::VSBC_VXM:
-  case RISCV::VMSBC_VXM:
-  case RISCV::VMSBC_VX:
-  // 11.5 Vector Bitwise Logical Instructions
-  case RISCV::VAND_VX:
-  case RISCV::VOR_VX:
-  case RISCV::VXOR_VX:
-  // 11.8. Vector Integer Compare Instructions
-  case RISCV::VMSEQ_VX:
-  case RISCV::VMSNE_VX:
-  case RISCV::VMSLTU_VX:
-  case RISCV::VMSLT_VX:
-  case RISCV::VMSLEU_VX:
-  case RISCV::VMSLE_VX:
-  case RISCV::VMSGTU_VX:
-  case RISCV::VMSGT_VX:
-  // 11.9. Vector Integer Min/Max Instructions
-  case RISCV::VMINU_VX:
-  case RISCV::VMIN_VX:
-  case RISCV::VMAXU_VX:
-  case RISCV::VMAX_VX:
-  // 11.10. Vector Single-Width Integer Multiply Instructions
-  case RISCV::VMUL_VX:
-  case RISCV::VMULH_VX:
-  case RISCV::VMULHU_VX:
-  case RISCV::VMULHSU_VX:
-  // 11.11. Vector Integer Divide Instructions
-  case RISCV::VDIVU_VX:
-  case RISCV::VDIV_VX:
-  case RISCV::VREMU_VX:
-  case RISCV::VREM_VX:
-  // 11.12. Vector Widening Integer Multiply Instructions
-  case RISCV::VWMUL_VX:
-  case RISCV::VWMULU_VX:
-  case RISCV::VWMULSU_VX:
-  // 11.13. Vector Single-Width Integer Multiply-Add Instructions
-  case RISCV::VMACC_VX:
-  case RISCV::VNMSAC_VX:
-  case RISCV::VMADD_VX:
-  case RISCV::VNMSUB_VX:
-  // 11.14. Vector Widening Integer Multiply-Add Instructions
-  case RISCV::VWMACCU_VX:
-  case RISCV::VWMACC_VX:
-  case RISCV::VWMACCSU_VX:
-  case RISCV::VWMACCUS_VX:
-  // 11.15. Vector Integer Merge Instructions
-  case RISCV::VMERGE_VXM:
-  // 11.16. Vector Integer Move Instructions
-  case RISCV::VMV_V_X:
-  // 12.1. Vector Single-Width Saturating Add and Subtract
-  case RISCV::VSADDU_VX:
-  case RISCV::VSADD_VX:
-  case RISCV::VSSUBU_VX:
-  case RISCV::VSSUB_VX:
-  // 12.2. Vector Single-Width Averaging Add and Subtract
-  case RISCV::VAADDU_VX:
-  case RISCV::VAADD_VX:
-  case RISCV::VASUBU_VX:
-  case RISCV::VASUB_VX:
-  // 12.3. Vector Single-Width Fractional Multiply with Rounding and Saturation
-  case RISCV::VSMUL_VX:
-  // 16.1. Integer Scalar Move Instructions
-  case RISCV::VMV_S_X:
-    if (Bits < (1 << Log2SEW))
-      return false;
-  }
-  return true;
-}
-
 // Checks if all users only demand the lower \p OrigBits of the original
 // instruction's result.
 // TODO: handle multiple interdependent transformations
@@ -242,10 +107,23 @@ static bool hasAllNBitUsers(const MachineInstr &OrigMI,
       unsigned OpIdx = UserOp.getOperandNo();
 
       switch (UserMI->getOpcode()) {
-      default:
-        if (vectorPseudoHasAllNBitUsers(UserOp, Bits))
-          break;
+      default: {
+        if (const RISCVVPseudosTable::PseudoInfo *PseudoInfo =
+                RISCVVPseudosTable::getPseudoInfo(UserMI->getOpcode())) {
+          const MCInstrDesc &MCID = UserMI->getDesc();
+          if (!RISCVII::hasSEWOp(MCID.TSFlags))
+            return false;
+          assert(RISCVII::hasVLOp(MCID.TSFlags));
+          const unsigned Log2SEW =
+              UserMI->getOperand(RISCVII::getSEWOpNum(MCID)).getImm();
+          if (UserOp.getOperandNo() == RISCVII::getVLOpNum(MCID))
+            return false;
+          if (RISCVII::vectorInstUsesNBitsOfScalarOp(PseudoInfo->BaseInstr,
+                                                     Bits, Log2SEW))
+            break;
+        }
         return false;
+      }
 
       case RISCV::ADDIW:
       case RISCV::ADDW:

@@ -130,6 +130,118 @@ parseFeatureBits(bool IsRV64, const FeatureBitset &FeatureBits) {

} // namespace RISCVFeatures

bool RISCVII::vectorInstUsesNBitsOfScalarOp(uint16_t Opcode, unsigned Bits,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This interface makes more sense if it take Opcode and SEW and returns the number of bits consumed, and the comparison is done by the caller. This is overly specific to the current caller.

@@ -2753,148 +2753,6 @@ bool RISCVDAGToDAGISel::selectSHXADD_UWOp(SDValue N, unsigned ShAmt,
return false;
}

static bool vectorPseudoHasAllNBitUsers(SDNode *User, unsigned UserOpNo,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Killing off this helper function hurts readability. Please undo.

@@ -78,141 +78,6 @@ FunctionPass *llvm::createRISCVOptWInstrsPass() {
return new RISCVOptWInstrs();
}

static bool vectorPseudoHasAllNBitUsers(const MachineOperand &UserOp,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@topperc
Copy link
Collaborator

topperc commented Sep 27, 2023

Should we put this in RISCVInstrInfo.cpp instead of RISCVBaseInfo.cpp? No reason currently to increase the size of MC layer tools for something they don't use.

@lukel97 lukel97 changed the title [RISCV] Move vector pseudo hasAllNBitUsers switch into RISCVBaseInfo.{h,cpp}. NFC [RISCV] Move vector pseudo hasAllNBitUsers switch into RISCVInstrInfo. NFC Sep 27, 2023
default:
auto NumDemandedBits =
RISCV::getVectorLowDemandedScalarBits(PseudoInfo->BaseInstr, Log2SEW);
if (!NumDemandedBits || Bits < NumDemandedBits)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return (NumDemandedBits && Bits >= NumDemandedBits;

default:
auto NumDemandedBits =
RISCV::getVectorLowDemandedScalarBits(PseudoInfo->BaseInstr, Log2SEW);
if (!NumDemandedBits || Bits < NumDemandedBits)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

case RISCV::VSMUL_VX:
// 16.1. Integer Scalar Move Instructions
case RISCV::VMV_S_X:
return 1 << Log2SEW;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure you incorporate Craig's fix the signed warning. 1U << Log2SEW

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering why I wasn't getting any warning, but then I realised I was compiling with clang. I switched to gcc-12 and started getting it. Looks like clang 14-17 all miss this, this seems related: #22597

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't get the warning anymore because it's no longer a comparison. Do we still want to keep the unsigned suffix to be explicit?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

…{h,cpp}. NFC

The handling for vector pseudos in hasAllNBitUsers is duplicated across
RISCVISelDAGToDAG and RISCVOptWInstrs. This deduplicates it between the two,
with the common denominator between the two call sites being the opcode and
SEW: We need to handle extracting these separately since one operates at the
SelectionDAG level and the other at the MachineInstr level.
@lukel97 lukel97 force-pushed the vector-pseudos-hasAllNBitsUsers-refactor branch from 050cfe3 to 8583f6b Compare September 28, 2023 11:34
Copy link
Collaborator

@preames preames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lukel97 lukel97 merged commit e577e70 into llvm:main Oct 3, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants