[RISCV] Allow compatible VTYPE in AVL Reg Forward cases

[reames]

Summary

During insertion of VSETVLI, we have two related bits of code which decide whether we can reuse a previous vsetvli result. As was pointed out in the original review, these cases can allow any prior state for which we know that VL is the same for any value of AVL.

This was originally separated out of a desire for separate tests and review. As it turns out, finding a test case for this has been quite challenging. Most of the cases I tried, we manage to already get through other chains of logic. We do have one correct test change, but that only exercises one of the two changes.

[frasercrmck]

LGTM

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

723

VLMAXE -> VLMAX

1243

Maybe make this vlmax upper case for consistency while you're here?

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

public:
 
   unsigned getSEWLMULRatio() const {
     assert(isValid() && !isUnknown() &&
            "Can't use VTYPE for uninitialized or unknown");
     return getSEWLMULRatio(SEW, VLMul);
   }
 
   // Check if the VTYPE for these two VSETVLIInfos produce the same VLMAX.
+  // Note that having the same VLMAX ensures that both share the same
+  // function from AVL to VL; that is, they must produce the same VL value
+  // for any given AVL value.
   bool hasSameVLMAX(const VSETVLIInfo &Other) const {
     assert(isValid() && Other.isValid() &&
            "Can't compare invalid VSETVLIInfos");
     assert(!isUnknown() && !Other.isUnknown() &&
            "Can't compare VTYPE in unknown state");
     return getSEWLMULRatio() == Other.getSEWLMULRatio();
   }
 

 }
 
 bool RISCVInsertVSETVLI::needVSETVLI(const VSETVLIInfo &Require,
                                      const VSETVLIInfo &CurInfo) {
   if (CurInfo.isCompatible(Require))
     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 VTYPE we need
+  // it might be defined by a VSET(I)VLI. If it has the same VLMAX we need
   // and the last VL/VTYPE we observed is the same, we don't need a
   // VSETVLI here.
   if (!CurInfo.isUnknown() && Require.hasAVLReg() &&
       Require.getAVLReg().isVirtual() && !CurInfo.hasSEWLMULRatioOnly() &&
       CurInfo.hasCompatibleVTYPE(Require)) {
     if (MachineInstr *DefMI = MRI->getVRegDef(Require.getAVLReg())) {
       if (isVectorConfigInstr(*DefMI)) {
         VSETVLIInfo DefInfo = getInfoForVSETVLI(*DefMI);
-        if (DefInfo.hasSameAVL(CurInfo) && DefInfo.hasSameVTYPE(CurInfo))
+        if (DefInfo.hasSameAVL(CurInfo) && DefInfo.hasSameVLMAX(CurInfo))
           return false;
       }
     }
   }
 
   return true;
 }
 
if (RISCVII::hasSEWOp(TSFlags)) {
                 }
                 CurInfo = computeInfoForInstr(MI, TSFlags, MRI);
                 continue;
               }
             }
           }
         }
 
-        // If AVL is defined by a vsetvli with the same vtype, we can
+        // If AVL is defined by a vsetvli with the same VLMAX, we can
         // replace the AVL operand with the AVL of the defining vsetvli.
         // We avoid general register AVLs to avoid extending live ranges
         // without being sure we can kill the original source reg entirely.
         // TODO: We can ignore policy bits here, we only need VL to be the same.
         if (Require.hasAVLReg() && Require.getAVLReg().isVirtual()) {
           if (MachineInstr *DefMI = MRI->getVRegDef(Require.getAVLReg())) {
             if (isVectorConfigInstr(*DefMI)) {
               VSETVLIInfo DefInfo = getInfoForVSETVLI(*DefMI);
-              if (DefInfo.hasSameVTYPE(Require) &&
+              if (DefInfo.hasSameVLMAX(Require) &&
                   (DefInfo.hasAVLImm() || DefInfo.getAVLReg() == RISCV::X0)) {
                 MachineOperand &VLOp = MI.getOperand(getVLOpNum(MI));
                 if (DefInfo.hasAVLImm())
                   VLOp.ChangeToImmediate(DefInfo.getAVLImm());
                 else
                   VLOp.ChangeToRegister(DefInfo.getAVLReg(), /*IsDef*/ false);
                 CurInfo = computeInfoForInstr(MI, TSFlags, MRI);
                 continue;

llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.ll

 ; Demonstrates a case where mutation in phase3 is problematic.  We mutate the
 ; vsetvli without considering that it changes the compatibility result of the
 ; vadd in the second block.
 define <vscale x 4 x i32> @cross_block_mutate(<vscale x 4 x i32> %a, <vscale x 4 x i32> %b,
 ; CHECK-LABEL: cross_block_mutate:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetivli a0, 6, e32, m2, tu, mu
 ; CHECK-NEXT:    vmv.s.x v8, a0
-; CHECK-NEXT:    vsetvli zero, a0, e32, m2, tu, mu
 ; CHECK-NEXT:    vadd.vv v8, v8, v10, v0.t
 ; CHECK-NEXT:    ret
                                          <vscale x 4 x i1> %mask) {
 entry:
   %vl = tail call i64 @llvm.riscv.vsetvli(i64 6, i64 1, i64 0)
   %vl.trunc = trunc i64 %vl to i32
   %a.mod = insertelement <vscale x 4 x i32> %a, i32 %vl.trunc, i32 0
   br label %fallthrough