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] Remove SEW operand for load/store and SEW-aware pseudos #90396

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wangpc-pp
Copy link
Contributor

@wangpc-pp wangpc-pp commented Apr 28, 2024

We can remove the SEW operand and encode the SEW value in TSFlags.

There are some exceptions that we can't remove the SEW operand:

  1. Mask load/store. Their SEW are always 1.
  2. Indexed load/store. Data SEW and index SEW can be different.

The MatcherTable is reduced by about 2.6% (57884 bytes) and
RISCVGenInstrInfo.inc has some reductions too. Besides, about
0.53% of compile-time can be reduced (not a stable result).

But it will also add some complexities and make the RVV pseudos
inconsistent.

Differential Revision: https://reviews.llvm.org/D159368

Created using spr 1.3.6-beta.1
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 28, 2024

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

Author: Pengcheng Wang (wangpc-pp)

Changes

We can remove the SEW operand and encode the SEW value in TSFlags.

There are some exceptions that we can't remove the SEW operand:

  1. Mask load/store. Their SEW are always 1.
  2. Indexed load/store. Data SEW and index SEW can be different.

The MatcherTable is reduced by about 2.6% (22808 bytes) and
RISCVGenInstrInfo.inc has some reductions too. Besides, about
0.53% of compile-time can be reduced (not a stable result).

But it will also add some complexities and make the RVV pseudos
inconsistent.

Differential Revision: https://reviews.llvm.org/D159368


Patch is 264.76 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90396.diff

29 Files Affected:

  • (modified) llvm/include/llvm/TargetParser/RISCVTargetParser.h (+7)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h (+51-5)
  • (modified) llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp (+23-12)
  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (-1)
  • (modified) llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp (+8-12)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrFormats.td (+7)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+8-14)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td (+573-298)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td (+128-137)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td (+155-131)
  • (modified) llvm/lib/Target/RISCV/RISCVOptWInstrs.cpp (+2-2)
  • (modified) llvm/test/CodeGen/RISCV/rvv/addi-scalable-offset.mir (+2-2)
  • (modified) llvm/test/CodeGen/RISCV/rvv/copyprop.mir (+1-1)
  • (modified) llvm/test/CodeGen/RISCV/rvv/debug-info-rvv-dbg-value.mir (+2-2)
  • (modified) llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fmf.ll (+1-1)
  • (modified) llvm/test/CodeGen/RISCV/rvv/frameindex-addr.ll (+1-1)
  • (modified) llvm/test/CodeGen/RISCV/rvv/implicit-def-copy.ll (+1-1)
  • (modified) llvm/test/CodeGen/RISCV/rvv/pass-fast-math-flags-sdnode.ll (+1-1)
  • (modified) llvm/test/CodeGen/RISCV/rvv/reg-coalescing.mir (+6-6)
  • (modified) llvm/test/CodeGen/RISCV/rvv/rvv-peephole-vmerge-vops-mir.ll (+2-2)
  • (modified) llvm/test/CodeGen/RISCV/rvv/strided-vpload-vpstore-output.ll (+2-2)
  • (modified) llvm/test/CodeGen/RISCV/rvv/subregister-undef-early-clobber.mir (+80-80)
  • (modified) llvm/test/CodeGen/RISCV/rvv/tail-agnostic-impdef-copy.mir (+2-3)
  • (modified) llvm/test/CodeGen/RISCV/rvv/vleff-vlseg2ff-output.ll (+6-6)
  • (modified) llvm/test/CodeGen/RISCV/rvv/vmv-copy.mir (+42-42)
  • (modified) llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir (+34-22)
  • (modified) llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir (+18-17)
  • (modified) llvm/test/CodeGen/RISCV/rvv/wrong-stack-offset-for-rvv-object.mir (+2-2)
  • (modified) llvm/test/CodeGen/RISCV/rvv/zvlsseg-spill.mir (+2-2)
diff --git a/llvm/include/llvm/TargetParser/RISCVTargetParser.h b/llvm/include/llvm/TargetParser/RISCVTargetParser.h
index cdd19189f8dc7d..d80ad7b6e9a8c3 100644
--- a/llvm/include/llvm/TargetParser/RISCVTargetParser.h
+++ b/llvm/include/llvm/TargetParser/RISCVTargetParser.h
@@ -51,6 +51,13 @@ enum VLMUL : uint8_t {
   LMUL_F2
 };
 
+enum VSEW : uint8_t {
+  SEW_8 = 0,
+  SEW_16,
+  SEW_32,
+  SEW_64,
+};
+
 enum {
   TAIL_UNDISTURBED_MASK_UNDISTURBED = 0,
   TAIL_AGNOSTIC = 1,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
index 08f056f78979af..cb5ab1e3a42911 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
@@ -18,10 +18,13 @@
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/CodeGen/MachineInstr.h"
+#include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/MC/MCInstrDesc.h"
 #include "llvm/TargetParser/RISCVISAInfo.h"
 #include "llvm/TargetParser/RISCVTargetParser.h"
 #include "llvm/TargetParser/SubtargetFeature.h"
+#include <cstdint>
 
 namespace llvm {
 
@@ -123,6 +126,12 @@ enum {
   // 3 -> widening case
   TargetOverlapConstraintTypeShift = UsesVXRMShift + 1,
   TargetOverlapConstraintTypeMask = 3ULL << TargetOverlapConstraintTypeShift,
+
+  HasImplictSEWShift = TargetOverlapConstraintTypeShift + 2,
+  HasImplictSEWMask = 1 << HasImplictSEWShift,
+
+  VSEWShift = HasImplictSEWShift + 1,
+  VSEWMask = 0b11 << VSEWShift,
 };
 
 // Helper functions to read TSFlags.
@@ -171,14 +180,29 @@ static inline bool hasRoundModeOp(uint64_t TSFlags) {
 /// \returns true if this instruction uses vxrm
 static inline bool usesVXRM(uint64_t TSFlags) { return TSFlags & UsesVXRMMask; }
 
+/// \returns true if this instruction has implict SEW value.
+static inline bool hasImplictSEW(uint64_t TSFlags) {
+  return TSFlags & HasImplictSEWMask;
+}
+
+/// \returns the VSEW for the instruction.
+static inline VSEW getVSEW(uint64_t TSFlags) {
+  return static_cast<VSEW>((TSFlags & VSEWMask) >> VSEWShift);
+}
+
+/// \returns true if there is a SEW value for the instruction.
+static inline bool hasSEW(uint64_t TSFlags) {
+  return hasSEWOp(TSFlags) || hasImplictSEW(TSFlags);
+}
+
 static inline unsigned getVLOpNum(const MCInstrDesc &Desc) {
   const uint64_t TSFlags = Desc.TSFlags;
-  // This method is only called if we expect to have a VL operand, and all
-  // instructions with VL also have SEW.
-  assert(hasSEWOp(TSFlags) && hasVLOp(TSFlags));
-  unsigned Offset = 2;
+  // This method is only called if we expect to have a VL operand.
+  assert(hasVLOp(TSFlags));
+  // Some instructions don't have SEW operand.
+  unsigned Offset = 1 + hasSEWOp(TSFlags);
   if (hasVecPolicyOp(TSFlags))
-    Offset = 3;
+    Offset = Offset + 1;
   return Desc.getNumOperands() - Offset;
 }
 
@@ -191,6 +215,28 @@ static inline unsigned getSEWOpNum(const MCInstrDesc &Desc) {
   return Desc.getNumOperands() - Offset;
 }
 
+static inline unsigned getLog2SEW(uint64_t TSFlags) {
+  return 3 + RISCVII::getVSEW(TSFlags);
+}
+
+static inline MachineOperand getSEWOp(const MachineInstr &MI) {
+  uint64_t TSFlags = MI.getDesc().TSFlags;
+  assert(hasSEW(TSFlags) && "The instruction doesn't have SEW value!");
+  if (hasSEWOp(TSFlags))
+    return MI.getOperand(getSEWOpNum(MI.getDesc()));
+
+  return MachineOperand::CreateImm(getLog2SEW(TSFlags));
+}
+
+static inline unsigned getLog2SEW(const MachineInstr &MI) {
+  uint64_t TSFlags = MI.getDesc().TSFlags;
+  assert(RISCVII::hasSEW(TSFlags) && "The instruction doesn't have SEW value!");
+  if (RISCVII::hasSEWOp(TSFlags))
+    return MI.getOperand(RISCVII::getSEWOpNum(MI.getDesc())).getImm();
+
+  return getLog2SEW(TSFlags);
+}
+
 static inline unsigned getVecPolicyOpNum(const MCInstrDesc &Desc) {
   assert(hasVecPolicyOp(Desc.TSFlags));
   return Desc.getNumOperands() - 1;
diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
index b0568297a470a7..d5db2717a721d3 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -317,8 +317,11 @@ void RISCVDAGToDAGISel::addVectorLoadStoreOperands(
   Operands.push_back(VL);
 
   MVT XLenVT = Subtarget->getXLenVT();
-  SDValue SEWOp = CurDAG->getTargetConstant(Log2SEW, DL, XLenVT);
-  Operands.push_back(SEWOp);
+  // Add SEW operand if it is indexed or mask load/store instruction.
+  if (Log2SEW == 0 || IndexVT) {
+    SDValue SEWOp = CurDAG->getTargetConstant(Log2SEW, DL, XLenVT);
+    Operands.push_back(SEWOp);
+  }
 
   // At the IR layer, all the masked load intrinsics have policy operands,
   // none of the others do.  All have passthru operands.  For our pseudos,
@@ -2226,7 +2229,6 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
       selectVLOp(Node->getOperand(2), VL);
 
     unsigned Log2SEW = Log2_32(VT.getScalarSizeInBits());
-    SDValue SEW = CurDAG->getTargetConstant(Log2SEW, DL, XLenVT);
 
     // If VL=1, then we don't need to do a strided load and can just do a
     // regular load.
@@ -2243,7 +2245,7 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
       Operands.push_back(CurDAG->getRegister(RISCV::X0, XLenVT));
     uint64_t Policy = RISCVII::MASK_AGNOSTIC | RISCVII::TAIL_AGNOSTIC;
     SDValue PolicyOp = CurDAG->getTargetConstant(Policy, DL, XLenVT);
-    Operands.append({VL, SEW, PolicyOp, Ld->getChain()});
+    Operands.append({VL, PolicyOp, Ld->getChain()});
 
     RISCVII::VLMUL LMUL = RISCVTargetLowering::getLMUL(VT);
     const RISCV::VLEPseudo *P = RISCV::getVLEPseudo(
@@ -2970,7 +2972,7 @@ static bool vectorPseudoHasAllNBitUsers(SDNode *User, unsigned UserOpNo,
 
   const MCInstrDesc &MCID = TII->get(User->getMachineOpcode());
   const uint64_t TSFlags = MCID.TSFlags;
-  if (!RISCVII::hasSEWOp(TSFlags))
+  if (!RISCVII::hasSEW(TSFlags))
     return false;
   assert(RISCVII::hasVLOp(TSFlags));
 
@@ -2980,7 +2982,9 @@ static bool vectorPseudoHasAllNBitUsers(SDNode *User, unsigned UserOpNo,
   bool HasVecPolicyOp = RISCVII::hasVecPolicyOp(TSFlags);
   unsigned VLIdx =
       User->getNumOperands() - HasVecPolicyOp - HasChainOp - HasGlueOp - 2;
-  const unsigned Log2SEW = User->getConstantOperandVal(VLIdx + 1);
+  const unsigned Log2SEW = RISCVII::hasSEWOp(TSFlags)
+                               ? User->getConstantOperandVal(VLIdx + 1)
+                               : RISCVII::getLog2SEW(TSFlags);
 
   if (UserOpNo == VLIdx)
     return false;
@@ -3696,12 +3700,18 @@ bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N) {
       return false;
   }
 
+  SDLoc DL(N);
+
   // The vector policy operand may be present for masked intrinsics
   bool HasVecPolicyOp = RISCVII::hasVecPolicyOp(TrueTSFlags);
-  unsigned TrueVLIndex =
-      True.getNumOperands() - HasVecPolicyOp - HasChainOp - HasGlueOp - 2;
+  bool HasSEWOp = RISCVII::hasSEWOp(TrueTSFlags);
+  unsigned TrueVLIndex = True.getNumOperands() - HasVecPolicyOp - HasChainOp -
+                         HasGlueOp - 1 - HasSEWOp;
   SDValue TrueVL = True.getOperand(TrueVLIndex);
-  SDValue SEW = True.getOperand(TrueVLIndex + 1);
+  SDValue SEW =
+      HasSEWOp ? True.getOperand(TrueVLIndex + 1)
+               : CurDAG->getTargetConstant(RISCVII::getLog2SEW(TrueTSFlags), DL,
+                                           Subtarget->getXLenVT());
 
   auto GetMinVL = [](SDValue LHS, SDValue RHS) {
     if (LHS == RHS)
@@ -3732,8 +3742,6 @@ bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N) {
         !True->getFlags().hasNoFPExcept())
       return false;
 
-  SDLoc DL(N);
-
   // From the preconditions we checked above, we know the mask and thus glue
   // for the result node will be taken from True.
   if (IsMasked) {
@@ -3799,7 +3807,10 @@ bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N) {
   if (HasRoundingMode)
     Ops.push_back(True->getOperand(TrueVLIndex - 1));
 
-  Ops.append({VL, SEW, PolicyOp});
+  Ops.push_back(VL);
+  if (RISCVII::hasSEWOp(TrueTSFlags))
+    Ops.push_back(SEW);
+  Ops.push_back(PolicyOp);
 
   // Result node should have chain operand of True.
   if (HasChainOp)
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 3ab9e7d69105ca..c317f63aadd621 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -17857,7 +17857,6 @@ static MachineBasicBlock *emitVFROUND_NOEXCEPT_MASK(MachineInstr &MI,
       .add(MI.getOperand(3))
       .add(MachineOperand::CreateImm(7)) // frm = DYN
       .add(MI.getOperand(4))
-      .add(MI.getOperand(5))
       .add(MI.getOperand(6))
       .add(MachineOperand::CreateReg(RISCV::FRM,
                                      /*IsDef*/ false,
diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index b5fd508fa77de2..ffb4bdd1cd392b 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -55,10 +55,6 @@ static unsigned getVLOpNum(const MachineInstr &MI) {
   return RISCVII::getVLOpNum(MI.getDesc());
 }
 
-static unsigned getSEWOpNum(const MachineInstr &MI) {
-  return RISCVII::getSEWOpNum(MI.getDesc());
-}
-
 static bool isVectorConfigInstr(const MachineInstr &MI) {
   return MI.getOpcode() == RISCV::PseudoVSETVLI ||
          MI.getOpcode() == RISCV::PseudoVSETVLIX0 ||
@@ -166,9 +162,9 @@ static bool isNonZeroLoadImmediate(const MachineInstr &MI) {
 /// Return true if this is an operation on mask registers.  Note that
 /// this includes both arithmetic/logical ops and load/store (vlm/vsm).
 static bool isMaskRegOp(const MachineInstr &MI) {
-  if (!RISCVII::hasSEWOp(MI.getDesc().TSFlags))
+  if (!RISCVII::hasSEW(MI.getDesc().TSFlags))
     return false;
-  const unsigned Log2SEW = MI.getOperand(getSEWOpNum(MI)).getImm();
+  const unsigned Log2SEW = RISCVII::getLog2SEW(MI);
   // A Log2SEW of 0 is an operation on mask registers only.
   return Log2SEW == 0;
 }
@@ -383,7 +379,7 @@ DemandedFields getDemanded(const MachineInstr &MI,
     Res.demandVTYPE();
   // Start conservative on the unlowered form too
   uint64_t TSFlags = MI.getDesc().TSFlags;
-  if (RISCVII::hasSEWOp(TSFlags)) {
+  if (RISCVII::hasSEW(TSFlags)) {
     Res.demandVTYPE();
     if (RISCVII::hasVLOp(TSFlags))
       Res.demandVL();
@@ -405,7 +401,7 @@ DemandedFields getDemanded(const MachineInstr &MI,
   }
 
   // Store instructions don't use the policy fields.
-  if (RISCVII::hasSEWOp(TSFlags) && MI.getNumExplicitDefs() == 0) {
+  if (RISCVII::hasSEW(TSFlags) && MI.getNumExplicitDefs() == 0) {
     Res.TailPolicy = false;
     Res.MaskPolicy = false;
   }
@@ -940,7 +936,7 @@ static VSETVLIInfo computeInfoForInstr(const MachineInstr &MI, uint64_t TSFlags,
 
   RISCVII::VLMUL VLMul = RISCVII::getLMul(TSFlags);
 
-  unsigned Log2SEW = MI.getOperand(getSEWOpNum(MI)).getImm();
+  unsigned Log2SEW = RISCVII::getLog2SEW(MI);
   // A Log2SEW of 0 is an operation on mask registers only.
   unsigned SEW = Log2SEW ? 1 << Log2SEW : 8;
   assert(RISCVVType::isValidSEW(SEW) && "Unexpected SEW");
@@ -1176,7 +1172,7 @@ static VSETVLIInfo adjustIncoming(VSETVLIInfo PrevInfo, VSETVLIInfo NewInfo,
 void RISCVInsertVSETVLI::transferBefore(VSETVLIInfo &Info,
                                         const MachineInstr &MI) const {
   uint64_t TSFlags = MI.getDesc().TSFlags;
-  if (!RISCVII::hasSEWOp(TSFlags))
+  if (!RISCVII::hasSEW(TSFlags))
     return;
 
   const VSETVLIInfo NewInfo = computeInfoForInstr(MI, TSFlags, *ST, MRI);
@@ -1256,7 +1252,7 @@ bool RISCVInsertVSETVLI::computeVLVTYPEChanges(const MachineBasicBlock &MBB,
   for (const MachineInstr &MI : MBB) {
     transferBefore(Info, MI);
 
-    if (isVectorConfigInstr(MI) || RISCVII::hasSEWOp(MI.getDesc().TSFlags))
+    if (isVectorConfigInstr(MI) || RISCVII::hasSEW(MI.getDesc().TSFlags))
       HadVectorOp = true;
 
     transferAfter(Info, MI);
@@ -1385,7 +1381,7 @@ void RISCVInsertVSETVLI::emitVSETVLIs(MachineBasicBlock &MBB) {
     }
 
     uint64_t TSFlags = MI.getDesc().TSFlags;
-    if (RISCVII::hasSEWOp(TSFlags)) {
+    if (RISCVII::hasSEW(TSFlags)) {
       if (PrevInfo != CurInfo) {
         // If this is the first implicit state change, and the state change
         // requested can be proven to produce the same register contents, we
diff --git a/llvm/lib/Target/RISCV/RISCVInstrFormats.td b/llvm/lib/Target/RISCV/RISCVInstrFormats.td
index a5c8524d05cbc5..01e514609eaf31 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrFormats.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrFormats.td
@@ -223,6 +223,13 @@ class RVInstCommon<dag outs, dag ins, string opcodestr, string argstr,
   // 3 -> widening case
   bits<2> TargetOverlapConstraintType = 0;
   let TSFlags{22-21} = TargetOverlapConstraintType;
+
+  bit HasImplictSEW = 0;
+  let TSFlags{23} = HasImplictSEW;
+
+  // The actual SEW value is 8 * (2 ^ VSEW).
+  bits<2> VSEW = 0;
+  let TSFlags{25-24} = VSEW;
 }
 
 class RVInst<dag outs, dag ins, string opcodestr, string argstr,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 8cb9a40a98bcd8..50c4db3346c0b8 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -287,7 +287,7 @@ static bool isConvertibleToVMV_V_V(const RISCVSubtarget &STI,
 
           // If the producing instruction does not depend on vsetvli, do not
           // convert COPY to vmv.v.v. For example, VL1R_V or PseudoVRELOAD.
-          if (!RISCVII::hasSEWOp(TSFlags) || !RISCVII::hasVLOp(TSFlags))
+          if (!RISCVII::hasSEW(TSFlags) || !RISCVII::hasVLOp(TSFlags))
             return false;
 
           // Found the definition.
@@ -410,9 +410,9 @@ void RISCVInstrInfo::copyPhysRegVector(
       MIB = MIB.addReg(ActualSrcReg, getKillRegState(KillSrc));
     if (UseVMV) {
       const MCInstrDesc &Desc = DefMBBI->getDesc();
-      MIB.add(DefMBBI->getOperand(RISCVII::getVLOpNum(Desc)));  // AVL
-      MIB.add(DefMBBI->getOperand(RISCVII::getSEWOpNum(Desc))); // SEW
-      MIB.addImm(0);                                            // tu, mu
+      MIB.add(DefMBBI->getOperand(RISCVII::getVLOpNum(Desc))); // AVL
+      MIB.add(RISCVII::getSEWOp(*DefMBBI));                    // SEW
+      MIB.addImm(0);                                           // tu, mu
       MIB.addReg(RISCV::VL, RegState::Implicit);
       MIB.addReg(RISCV::VTYPE, RegState::Implicit);
     }
@@ -1706,8 +1706,7 @@ bool RISCVInstrInfo::areRVVInstsReassociable(const MachineInstr &Root,
     return false;
 
   // SEW
-  if (RISCVII::hasSEWOp(TSFlags) &&
-      !checkImmOperand(RISCVII::getSEWOpNum(Desc)))
+  if (RISCVII::hasSEW(TSFlags) && !checkImmOperand(RISCVII::getSEWOpNum(Desc)))
     return false;
 
   // Mask
@@ -2463,10 +2462,6 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
         return false;
       }
     }
-    if (!RISCVII::hasSEWOp(TSFlags)) {
-      ErrInfo = "VL operand w/o SEW operand?";
-      return false;
-    }
   }
   if (RISCVII::hasSEWOp(TSFlags)) {
     unsigned OpIdx = RISCVII::getSEWOpNum(Desc);
@@ -3521,8 +3516,8 @@ MachineInstr *RISCVInstrInfo::convertToThreeAddress(MachineInstr &MI,
   case CASE_FP_WIDEOP_OPCODE_LMULS_MF4(FWADD_WV):
   case CASE_FP_WIDEOP_OPCODE_LMULS_MF4(FWSUB_WV): {
     assert(RISCVII::hasVecPolicyOp(MI.getDesc().TSFlags) &&
-           MI.getNumExplicitOperands() == 7 &&
-           "Expect 7 explicit operands rd, rs2, rs1, rm, vl, sew, policy");
+           MI.getNumExplicitOperands() == 6 &&
+           "Expect 6 explicit operands rd, rs2, rs1, rm, vl, policy");
     // If the tail policy is undisturbed we can't convert.
     if ((MI.getOperand(RISCVII::getVecPolicyOpNum(MI.getDesc())).getImm() &
          1) == 0)
@@ -3545,8 +3540,7 @@ MachineInstr *RISCVInstrInfo::convertToThreeAddress(MachineInstr &MI,
               .add(MI.getOperand(2))
               .add(MI.getOperand(3))
               .add(MI.getOperand(4))
-              .add(MI.getOperand(5))
-              .add(MI.getOperand(6));
+              .add(MI.getOperand(5));
     break;
   }
   case CASE_WIDEOP_OPCODE_LMULS(WADD_WV):
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
index fc60a9cc7cd30e..21086688f13c70 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
@@ -769,15 +769,20 @@ class GetVTypeScalarPredicates<VTypeInfo vti> {
 class VPseudoUSLoadNoMask<VReg RetClass,
                           int EEW> :
       Pseudo<(outs RetClass:$rd),
-             (ins RetClass:$dest, GPRMem:$rs1, AVL:$vl, ixlenimm:$sew,
-                  ixlenimm:$policy), []>,
+             !if(!eq(EEW, 1),
+                 (ins RetClass:$dest, GPRMem:$rs1, AVL:$vl, ixlenimm:$sew, ixlenimm:$policy),
+                 (ins RetClass:$dest, GPRMem:$rs1, AVL:$vl, ixlenimm:$policy)), []>,
       RISCVVPseudo,
       RISCVVLE</*Masked*/0, /*Strided*/0, /*FF*/0, !logtwo(EEW), VLMul> {
   let mayLoad = 1;
   let mayStore = 0;
   let hasSideEffects = 0;
   let HasVLOp = 1;
-  let HasSEWOp = 1;
+  defvar hasSEWOp = !eq(EEW, 1);
+  let HasSEWOp = hasSEWOp;
+  // For mask load, EEW = 1.
+  let HasImplictSEW = !not(hasSEWOp);
+  let VSEW = !if(hasSEWOp, 0, !logtwo(!div(EEW, 8)));
   let HasVecPolicyOp = 1;
   let Constraints = "$rd = $dest";
 }
@@ -787,7 +792,7 @@ class VPseudoUSLoadMask<VReg RetClass,
       Pseudo<(outs GetVRegNoV0<RetClass>.R:$rd),
              (ins GetVRegNoV0<RetClass>.R:$merge,
                   GPRMem:$rs1,
-                  VMaskOp:$vm, AVL:$vl, ixlenimm:$sew, ixlenimm:$policy), []>,
+                  VMaskOp:$vm, AVL:$vl, ixlenimm:$policy), []>,
       RISCVVPseudo,
       RISCVVLE</*Masked*/1, /*Strided*/0, /*FF*/0, !logtwo(EEW), VLMul> {
   let mayLoad = 1;
@@ -795,7 +800,8 @@ class VPseudoUSLoadMask<VReg RetClass,
   let hasSideEffects = 0;
   let Constraints = "$rd = $merge";
   let HasVLOp = 1;
-  let HasSEWOp = 1;
+  let HasImplictSEW = 1;
+  let VSEW = !logtwo(!div(EEW, 8));
   let HasVecPolicyOp = 1;
   let UsesMaskPolicy = 1;
 }
@@ -803,15 +809,15 @@ class VPseudoUSLoadMask<VReg RetClass,
 class VPseudoUSLoadFFNoMask<VReg RetClass,
                             int EEW> :
       Pseudo<(outs RetClass:$rd, GPR:$vl),
-             (ins RetClass:$dest, GPRMem:$rs1, AVL:$avl,
-                  ixlenimm:$sew, ixlenimm:$policy), []>,
+             (ins RetClass:$dest, GPRMem:$rs1, AVL:$avl, ixlenimm:$policy), []>,
       RISCVVPseudo,
       RISCVVLE</*Masked*/0, /*Strided*/0, /*FF*/1, !logtwo(EEW), VLMul> {
   let mayLoad = 1;
   let mayStore = 0;
   let hasSideEffects = 0;
   let HasVLOp = 1;
-  let HasSEWOp = 1;
+  let HasImplictSEW = 1;
+  let VSEW = !logtwo(!div(EEW, 8));
   let HasVecPolicyOp = 1;
   let Constraints = "$rd = $dest";
 }
@@ -821,7 +827,7 @@ class VPseudoUSLoadFFMask<VReg RetClass,
       Pseudo<(outs GetVRegNoV0<RetClass>.R:$rd, GPR:$vl),
              (ins GetVRegNoV0<RetClass>.R:$merge,
                   GPRMem:$rs1,
-                  VMaskOp:$vm, AVL:$avl, ixlenimm:$sew, ixlenimm:$policy), []>,
+                  VMaskOp:$vm, AVL:$avl, ixlenimm:$policy), []>,
       RISCVVPseudo,
       RISCVVLE</*Masked*/1, /*Strided*/0, /*FF*/1, !logtwo(EEW), VLMul> {
   let mayLoad = 1;
@@ -829,7 +835,8 @@ class VPseudoUSLoadFFMask<VReg RetClass,
   let hasSideEffects = 0;
   let Constraints = "$rd = $merge";
   let HasVLOp = 1;
-  let HasSEWOp = 1;
+  let HasImplictSEW = 1;
+  let VSEW = !logtwo(!div(EEW, 8));
   let HasVecPolicyOp = 1;
   let UsesMaskPolicy = 1;
 }
@@ -837,15 +844,15 @@ class VPseudoUSLoadFFMask<VReg RetClass,
 class VPseudoSLoadNoMask<VReg RetClass,
                          int EEW> :
       Pseudo<(outs RetClass:$rd),
-             (ins RetClass:$dest, GPRMem:$rs1, GPR:$rs2, AVL:$vl,
-                  ixlenimm:$sew, ixlenimm:$policy), []>,
+             (ins RetClass:$dest, GPRMem:$rs1, GPR:$rs2, AVL:$vl, ixlenimm:$policy), []>,
       RISCVVPseudo,
       RISCVVLE</*Masked*/0, /*Strided*/1, /*FF*/0, !logtwo(EEW), VLMul> {
   let mayLoad = 1;
   let mayStore = 0;
   let hasSideEffects = 0;
   let HasVLOp = 1;
-  let HasSEWOp = 1;
+  let HasImplictSEW = 1;
+  let VSEW = !logtwo(!div(EEW, 8));
   let HasVecPolicyOp = 1;
   let Constraints = "$rd = $dest";
 }
@@ -855,7 +862,7 @@ class VPseudoSLoadMask<VReg RetClass,
       Pseudo<(outs GetVRegNoV0<RetClass>.R:...
[truncated]

}

static inline unsigned getLog2SEW(const MachineInstr &MI) {
uint64_t TSFlags = MI.getDesc().TSFlags;
Copy link
Contributor

Choose a reason for hiding this comment

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

How about to implement getLog2SEW with getSEWOp?

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 thought about this before, but it seems to be silly because for the code path that RISCVII::hasSEWOp returns false, we will create an immediate operand and then extract the imm.

@yetingk
Copy link
Contributor

yetingk commented Apr 29, 2024

Mask load/store. Their SEW are always 1.

Could we make VSEW also support SEW=1 scenario?

@wangpc-pp
Copy link
Contributor Author

Mask load/store. Their SEW are always 1.

Could we make VSEW also support SEW=1 scenario?

Then we will need 3 bits to encode VSEW in TSFlags. Maybe we can do it in this way:

  • Remove HasImplictSEW field.
  • Extend VSEW to 3 bits:
    • 0 means explicit SEW in operands.
    • 1-5 mean SEW is 1, 8, 16, 32, 64.

Created using spr 1.3.6-beta.1
Comment on lines +772 to 775
!if(!eq(EEW, 1),
(ins RetClass:$dest, GPRMem:$rs1, AVL:$vl, ixlenimm:$sew, ixlenimm:$policy),
(ins RetClass:$dest, GPRMem:$rs1, AVL:$vl, ixlenimm:$policy)), []>,
RISCVVPseudo,
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to add a separate class for the vlm.v/vsm.v pseudos? Since not only do they not need an SEW but they also don't need a policy operand either, it's always TAMA IIUC? TA as per the spec, MA because there's no masked variant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I will have a try.

// This method is only called if we expect to have a VL operand.
assert(hasVLOp(TSFlags));
// Some instructions don't have SEW operand.
unsigned Offset = 1 + hasSEWOp(TSFlags);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think hasSEWOp returns bool. Is it correct to add unsigned + bool?

@@ -2980,7 +2982,9 @@ static bool vectorPseudoHasAllNBitUsers(SDNode *User, unsigned UserOpNo,
bool HasVecPolicyOp = RISCVII::hasVecPolicyOp(TSFlags);
unsigned VLIdx =
User->getNumOperands() - HasVecPolicyOp - HasChainOp - HasGlueOp - 2;
const unsigned Log2SEW = User->getConstantOperandVal(VLIdx + 1);
const unsigned Log2SEW = RISCVII::hasSEWOp(TSFlags)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the assignment here backwards? If hasSEWOp is true then should we get it from getLog2SEW(TSFlags)?

@@ -223,6 +223,13 @@ class RVInstCommon<dag outs, dag ins, string opcodestr, string argstr,
// 3 -> widening case
bits<2> TargetOverlapConstraintType = 0;
let TSFlags{22-21} = TargetOverlapConstraintType;

bit HasImplictSEW = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can you add a comment what it means to have an implicit SEW?

@@ -2463,10 +2462,6 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
return false;
}
}
if (!RISCVII::hasSEWOp(TSFlags)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we check hasSEWOp && !hasVLOperand that would throw an error SEW operand without VL operand.

defvar hasSEWOp = !eq(EEW, 1);
let HasSEWOp = hasSEWOp;
// For mask load, EEW = 1.
let HasImplictSEW = !not(hasSEWOp);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I'm not sure if you can do this but would it be simpler to do:

let HasSEWOp = !eq(EEW, 1);
// For mask load, EEW = 1.
let HasImplictSEW = !not(HasSEWOp);
let VSEW = !if(HasSEWOp, 0, !logtwo(!div(EEW, 8)));

If that is not valid tablegen, please feel free to ignore this comment.

RISCVVPseudo,
RISCVVSE</*Masked*/0, /*Strided*/0, !logtwo(EEW), VLMul> {
let mayLoad = 0;
let mayStore = 1;
let hasSideEffects = 0;
let HasVLOp = 1;
let HasSEWOp = 1;
// For mask store, EEW = 1.
Copy link
Contributor

@michaelmaitland michaelmaitland Apr 29, 2024

Choose a reason for hiding this comment

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

Can VPseudoUSStoreNoMask be a mask store? Those two things sound like they may conflict.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think "mask store" here means a store where the data value is a mask. Specifically the vsm instruction. The NoMask means that v0 isn't used to indicate which elements get stored, what may be referred to as a "masked store". vsm doesn't support masking off elements.

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

6 participants