[RISCV] Check floating point vector instruction with SEW=64 is valid when vsetvl insertion

[kito-cheng]

Summary

Scalar move and splat instruction are only demand the SEW is greater than
its own needs, but floating point vector with SEW=64 is not alwaws valid even
SEW=64 is valid, because we have a special configuration: zve64f.

So we need to check floating point vector instruction with SEW=64 is
valid when compute demand of floating point scalar move and splat
instruction.

[craig.topper]

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

210

"must less" -> "must be less"

[reames]

I would suggest an initial patch which simply disabled the problematic logic for zve64f. Adding the additional inference state can be a separate follow on. In particularly, you currently don't have test coverage for the optimization bits.

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

1534

Instead of using a global, please just check the property of the subtarget as the call sites. Much easier to read.

[craig.topper]

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

332

Capitalize variable name

[kito-cheng]

Changes:

• Address comments
• Remove optimization part

[reames]

LGTM

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

 static uint16_t getRVVMCOpcode(uint16_t RVVPseudoOpcode) {
   const RISCVVPseudosTable::PseudoInfo *RVV =
       RISCVVPseudosTable::getPseudoInfo(RVVPseudoOpcode);
   if (!RVV)
     return 0;
   return RVV->BaseInstr;
 }
 
+static bool isFloatScalarMoveOrScalarSplatInstr(const MachineInstr &MI) {
+  switch (getRVVMCOpcode(MI.getOpcode())) {
+  default:
+    return false;
+  case RISCV::VFMV_S_F:
+  case RISCV::VFMV_V_F:
+    return true;
+  }
+}
+
 static bool isScalarMoveInstr(const MachineInstr &MI) {
   switch (getRVVMCOpcode(MI.getOpcode())) {
   default:
     return false;
   case RISCV::VMV_S_X:
   case RISCV::VFMV_S_F:
     return true;
   }

 struct DemandedFields {
   // Some unknown property of VL is used.  If demanded, must preserve entire
   // value.
   bool VLAny = false;
   // Only zero vs non-zero is used. If demanded, can change non-zero values.
   bool VLZeroness = false;
   // What properties of SEW we need to preserve.
   enum : uint8_t {
-    SEWEqual = 2,              // The exact value of SEW needs to be preserved.
-    SEWGreaterThanOrEqual = 1, // SEW can be changed as long as it's greater
+    SEWEqual = 3,              // The exact value of SEW needs to be preserved.
+    SEWGreaterThanOrEqual = 2, // SEW can be changed as long as it's greater
                                // than or equal to the original value.
-    SEWNone = 0                // We don't need to preserve SEW at all.
+    SEWGreaterThanOrEqualAndLessThan64 =
+        1,      // SEW can be changed as long as it's greater
+                // than or equal to the original value, but must
+                // less than 64
+    SEWNone = 0 // We don't need to preserve SEW at all.
   } SEW = SEWNone;
   bool LMUL = false;
   bool SEWLMULRatio = false;
   bool TailPolicy = false;
   bool MaskPolicy = false;
 
   // Return true if any part of VTYPE was used
   bool usedVTYPE() const {
void print(raw_ostream &OS) const {
     OS << "{";
     OS << "VLAny=" << VLAny << ", ";
     OS << "VLZeroness=" << VLZeroness << ", ";
     OS << "SEW=";
     switch (SEW) {
     case SEWEqual:
       OS << "SEWEqual";
       break;
+    case SEWGreaterThanOrEqualAndLessThan64:
+      OS << "SEWGreaterThanOrEqualAndLessThan64";
+      break;
     case SEWGreaterThanOrEqual:
       OS << "SEWGreaterThanOrEqual";
       break;
     case SEWNone:
       OS << "SEWNone";
       break;
     };
     OS << ", ";
static bool areCompatibleVTYPEs(uint64_t CurVType, uint64_t NewVType,
   if (Used.SEW == DemandedFields::SEWEqual &&
       RISCVVType::getSEW(CurVType) != RISCVVType::getSEW(NewVType))
     return false;
 
   if (Used.SEW == DemandedFields::SEWGreaterThanOrEqual &&
       RISCVVType::getSEW(NewVType) < RISCVVType::getSEW(CurVType))
     return false;
 
+  if (Used.SEW == DemandedFields::SEWGreaterThanOrEqualAndLessThan64 &&
+      (RISCVVType::getSEW(NewVType) < RISCVVType::getSEW(CurVType) ||
+       RISCVVType::getSEW(NewVType) >= 64))
+    return false;
+
   if (Used.LMUL &&
       RISCVVType::getVLMUL(CurVType) != RISCVVType::getVLMUL(NewVType))
     return false;
 
   if (Used.SEWLMULRatio) {
     auto Ratio1 = RISCVVType::getSEWLMULRatio(RISCVVType::getSEW(CurVType),
                                               RISCVVType::getVLMUL(CurVType));
     auto Ratio2 = RISCVVType::getSEWLMULRatio(RISCVVType::getSEW(NewVType),
                                               RISCVVType::getVLMUL(NewVType));
     if (Ratio1 != Ratio2)
       return false;
   }
 
   if (Used.TailPolicy && RISCVVType::isTailAgnostic(CurVType) !=
                              RISCVVType::isTailAgnostic(NewVType))
     return false;
   if (Used.MaskPolicy && RISCVVType::isMaskAgnostic(CurVType) !=
                              RISCVVType::isMaskAgnostic(NewVType))
     return false;
   return true;
 }
 
 /// Return the fields and properties demanded by the provided instruction.
 DemandedFields getDemanded(const MachineInstr &MI,
-                           const MachineRegisterInfo *MRI) {
+                           const MachineRegisterInfo *MRI,
+                           bool hasVInstructionsF64) {
   // Warning: This function has to work on both the lowered (i.e. post
   // emitVSETVLIs) and pre-lowering forms.  The main implication of this is
   // that it can't use the value of a SEW, VL, or Policy operand as they might
   // be stale after lowering.
 
   // Most instructions don't use any of these subfeilds.
   DemandedFields Res;
   // Start conservative if registers are used
if (isScalarMoveInstr(MI)) {
     Res.VLAny = false;
     // For vmv.s.x and vfmv.s.f, if the merge operand is *undefined*, we don't
     // need to preserve any other bits and are thus compatible with any larger,
     // etype and can disregard policy bits.  Warning: It's tempting to try doing
     // this for any tail agnostic operation, but we can't as TA requires
     // tail lanes to either be the original value or -1.  We are writing
     // unknown bits to the lanes here.
     if (hasUndefinedMergeOp(MI, *MRI)) {
+      if (isFloatScalarMoveOrScalarSplatInstr(MI) && !hasVInstructionsF64)
+        Res.SEW = DemandedFields::SEWGreaterThanOrEqualAndLessThan64;
+      else
         Res.SEW = DemandedFields::SEWGreaterThanOrEqual;
       Res.TailPolicy = false;
     }
   }
 
   return Res;
 }
 
 /// Defines the abstract state with which the forward dataflow models the

 
 class RISCVInsertVSETVLI : public MachineFunctionPass {
   const TargetInstrInfo *TII;
   MachineRegisterInfo *MRI;
 
   std::vector<BlockData> BlockInfo;
   std::queue<const MachineBasicBlock *> WorkList;
 
+  bool hasVInstructionsF64;
+
 public:
   static char ID;
 
   RISCVInsertVSETVLI() : MachineFunctionPass(ID) {
     initializeRISCVInsertVSETVLIPass(*PassRegistry::getPassRegistry());
   }
   bool runOnMachineFunction(MachineFunction &MF) override;
 

 bool RISCVInsertVSETVLI::needVSETVLI(const MachineInstr &MI,
                                      const VSETVLIInfo &Require,
                                      const VSETVLIInfo &CurInfo) const {
   assert(Require == computeInfoForInstr(MI, MI.getDesc().TSFlags, MRI));
 
   if (!CurInfo.isValid() || CurInfo.isUnknown() || CurInfo.hasSEWLMULRatioOnly())
     return true;
 
-  DemandedFields Used = getDemanded(MI, MRI);
+  DemandedFields Used = getDemanded(MI, MRI, hasVInstructionsF64);
 
   // A slidedown/slideup with an *undefined* merge op can freely clobber
   // elements not copied from the source vector (e.g. masked off, tail, or
   // slideup's prefix). Notes:
   // * We can't modify SEW here since the slide amount is in units of SEW.
   // * VL=1 is special only because we have existing support for zero vs
   //   non-zero VL.  We could generalize this if we had a VL > C predicate.
   // * The LMUL1 restriction is for machines whose latency may depend on VL.
bool RISCVInsertVSETVLI::needVSETVLI(const MachineInstr &MI,
   // immediate form of vmv.s.x, and thus frequently use vmv.v.i in it's place.
   // Since a splat is non-constant time in LMUL, we do need to be careful to not
   // increase the number of active vector registers (unlike for vmv.s.x.)
   if (isScalarSplatInstr(MI) && Require.hasAVLImm() && Require.getAVLImm() == 1 &&
       isLMUL1OrSmaller(CurInfo.getVLMUL()) && hasUndefinedMergeOp(MI, *MRI)) {
     Used.LMUL = false;
     Used.SEWLMULRatio = false;
     Used.VLAny = false;
+    if (isFloatScalarMoveOrScalarSplatInstr(MI) && !hasVInstructionsF64)
+      Used.SEW = DemandedFields::SEWGreaterThanOrEqualAndLessThan64;
+    else
       Used.SEW = DemandedFields::SEWGreaterThanOrEqual;
     Used.TailPolicy = false;
   }
 
   if (CurInfo.isCompatible(Used, Require, *MRI))
     return false;
 
   // We didn't find a compatible value. If our AVL is a virtual register,
   // it might be defined by a VSET(I)VLI. If it has the same VLMAX we need
void RISCVInsertVSETVLI::doLocalPostpass(MachineBasicBlock &MBB) {
   // must be considered demanded.
   DemandedFields Used;
   Used.demandVL();
   Used.demandVTYPE();
   SmallVector<MachineInstr*> ToDelete;
   for (MachineInstr &MI : make_range(MBB.rbegin(), MBB.rend())) {
 
     if (!isVectorConfigInstr(MI)) {
-      doUnion(Used, getDemanded(MI, MRI));
+      doUnion(Used, getDemanded(MI, MRI, hasVInstructionsF64));
       continue;
     }
 
     Register VRegDef = MI.getOperand(0).getReg();
     if (VRegDef != RISCV::X0 &&
         !(VRegDef.isVirtual() && MRI->use_nodbg_empty(VRegDef)))
       Used.demandVL();
 
if (NextMI) {
         Register NextVRefDef = NextMI->getOperand(0).getReg();
         if (NextVRefDef == RISCV::X0 ||
             (NextVRefDef.isVirtual() && MRI->use_nodbg_empty(NextVRefDef)))
           ToDelete.push_back(NextMI);
         // fallthrough
       }
     }
     NextMI = &MI;
-    Used = getDemanded(MI, MRI);
+    Used = getDemanded(MI, MRI, hasVInstructionsF64);
   }
 
   for (auto *MI : ToDelete)
     MI->eraseFromParent();
 }
 
 void RISCVInsertVSETVLI::insertReadVL(MachineBasicBlock &MBB) {
   for (auto I = MBB.begin(), E = MBB.end(); I != E;) {
bool RISCVInsertVSETVLI::runOnMachineFunction(MachineFunction &MF) {
   const RISCVSubtarget &ST = MF.getSubtarget<RISCVSubtarget>();
   if (!ST.hasVInstructions())
     return false;
 
   LLVM_DEBUG(dbgs() << "Entering InsertVSETVLI for " << MF.getName() << "\n");
 
   TII = ST.getInstrInfo();
   MRI = &MF.getRegInfo();
+  hasVInstructionsF64 = ST.hasVInstructionsF64();
 
   assert(BlockInfo.empty() && "Expect empty block infos");
   BlockInfo.resize(MF.getNumBlockIDs());
 
   bool HaveVectorOp = false;
 
   // Phase 1 - determine how VL/VTYPE are affected by the each block.
   for (const MachineBasicBlock &MBB : MF) {

llvm/test/CodeGen/RISCV/rvv/vsetvli-valid-elen-fp.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple riscv64 -mattr=+64bit,+m,+d,+zve64f,+zvfh \
 ; RUN:   -target-abi=lp64d < %s | FileCheck %s --check-prefix=CHECK-NO-FELEN64
 ; RUN: llc -mtriple riscv64 -mattr=+64bit,+m,+d,+zve64d,+zvfh \
 ; RUN:   -target-abi=lp64d < %s | FileCheck %s --check-prefix=CHECK-FELEN64
 
 define void @foo(half %y, ptr %i64p) {
 ; CHECK-NO-FELEN64-LABEL: foo:
 ; CHECK-NO-FELEN64:       # %bb.0: # %entry
 ; CHECK-NO-FELEN64-NEXT:    vsetivli zero, 1, e64, m1, ta, ma
 ; CHECK-NO-FELEN64-NEXT:    vle64.v v8, (a0)
+; CHECK-NO-FELEN64-NEXT:    vsetivli zero, 1, e16, m1, ta, ma
 ; CHECK-NO-FELEN64-NEXT:    vfmv.s.f v9, fa0
 ; CHECK-NO-FELEN64-NEXT:    #APP
 ; CHECK-NO-FELEN64-NEXT:    # use v8 v9
 ; CHECK-NO-FELEN64-NEXT:    #NO_APP
 ; CHECK-NO-FELEN64-NEXT:    ret
 ;
 ; CHECK-FELEN64-LABEL: foo:
 ; CHECK-FELEN64:       # %bb.0: # %entry