[RISCV] Ensure the forwarded AVL register is alive

[loralb]

Summary

When the AVL value does not fit in 5 bits, the register in which this value is stored may be dead when we want to forward it. This patch ensure the kill flags on the register are cleared before forwarding.

[reames]

Oh, good catch!

We have the same bug in several places in this code, would you mind updating the fix to include all three? I tagged them inline.

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

1218

Here

1265

Here

[craig.topper]

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

1218

I'm pretty sure this one is always X0, since that's the only register hasNonZeroAVL/hasZeroAVL work for. But we should assert to be safe.

1265

This one is always X0. I don't think MRI knows about the uses of X0 to find kill flags to clear.

[reames]

Craig is correct on both points. Given that, LGTM to the original patch unmodified.

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

if (isScalarMoveInstr(MI)) {
       // TODO: We can probably relax this for immediates.
       if (((CurInfo.hasNonZeroAVL() && NewInfo.hasNonZeroAVL()) ||
            (CurInfo.hasZeroAVL() && NewInfo.hasZeroAVL())) &&
           NewInfo.hasSameVLMAX(CurInfo)) {
         MachineOperand &VLOp = MI.getOperand(getVLOpNum(MI));
         if (CurInfo.hasAVLImm())
           VLOp.ChangeToImmediate(CurInfo.getAVLImm());
         else
           VLOp.ChangeToRegister(CurInfo.getAVLReg(), /*IsDef*/ false);
         CurInfo = computeInfoForInstr(MI, TSFlags, MRI);
         continue;
       }
     }
 
     if (RISCVII::hasSEWOp(TSFlags)) {
       if (RISCVII::hasVLOp(TSFlags)) {
         const auto Require = computeInfoForInstr(MI, TSFlags, MRI);
         // If the AVL is the result of a previous vsetvli which has the
         // same AVL and VLMAX as our current state, we can reuse the AVL
         // from the current state for the new one.  This allows us to
         // generate 'vsetvli x0, x0, vtype" or possible skip the transition
         // entirely.
         if (!CurInfo.isUnknown() && Require.hasAVLReg() &&
             Require.getAVLReg().isVirtual()) {
           if (MachineInstr *DefMI = MRI->getVRegDef(Require.getAVLReg())) {
             if (isVectorConfigInstr(*DefMI)) {
               VSETVLIInfo DefInfo = getInfoForVSETVLI(*DefMI);
               if (DefInfo.hasSameAVL(CurInfo) &&
                   DefInfo.hasSameVLMAX(CurInfo)) {
                 MachineOperand &VLOp = MI.getOperand(getVLOpNum(MI));
                 if (CurInfo.hasAVLImm())
                   VLOp.ChangeToImmediate(CurInfo.getAVLImm());
-                else
+                else {
+                  MRI->clearKillFlags(CurInfo.getAVLReg());
                   VLOp.ChangeToRegister(CurInfo.getAVLReg(), /*IsDef*/ false);
+                }
                 CurInfo = computeInfoForInstr(MI, TSFlags, MRI);
                 continue;
               }
             }
           }
         }
 
         // If AVL is defined by a vsetvli with the same vtype, 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) &&
                   (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.ll

 }
 
 declare { <vscale x 1 x i64>, i64 } @llvm.riscv.vleff.nxv1i64.i64(
   <vscale x 1 x i64>, <vscale x 1 x i64>* nocapture, i64)
 
 declare <vscale x 1 x i1> @llvm.riscv.vmseq.nxv1i64.i64.i64(
   <vscale x 1 x i64>, i64, i64)
 
+; Ensure AVL register is alive when forwarding an AVL immediate that does not fit in 5 bits
+define <vscale x 2 x i32> @avl_forward5(<vscale x 2 x i32>* %addr) {
+; CHECK-LABEL: avl_forward5:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    li a1, 32
+; CHECK-NEXT:    vsetvli zero, a1, e8, m4, ta, mu
+; CHECK-NEXT:    vsetvli zero, a1, e32, m1, ta, mu
+; CHECK-NEXT:    vle32.v v8, (a0)
+; CHECK-NEXT:    ret
+  %gvl = tail call i64 @llvm.riscv.vsetvli.i64(i64 32, i64 0, i64 2)
+  %ret = tail call <vscale x 2 x i32> @llvm.riscv.vle.nxv2i32.i64(<vscale x 2 x i32> undef, <vscale x 2 x i32>* %addr, i64 %gvl)
+  ret <vscale x 2 x i32> %ret
+}
+
 declare <vscale x 1 x i64> @llvm.riscv.vadd.mask.nxv1i64.nxv1i64(
   <vscale x 1 x i64>,
   <vscale x 1 x i64>,
   <vscale x 1 x i64>,
   <vscale x 1 x i1>,
   i64,
   i64);