[llvm] [RISCV][VLOPT] Add support for checkUsers when UserMI is a Single-Width Integer Reduction (PR #120345)

[Luke Lau via llvm-commits]

================
@@ -1028,79 +1055,113 @@ bool RISCVVLOptimizer::isCandidate(const MachineInstr &MI) const {
   return true;
 }
 
-bool RISCVVLOptimizer::checkUsers(const MachineOperand *&CommonVL,
-                                  MachineInstr &MI) {
+std::optional<MachineOperand>
+RISCVVLOptimizer::getVLForUser(MachineOperand &UserOp) {
+  const MachineInstr &UserMI = *UserOp.getParent();
+  const MCInstrDesc &Desc = UserMI.getDesc();
+
+  if (!RISCVII::hasVLOp(Desc.TSFlags) || !RISCVII::hasSEWOp(Desc.TSFlags)) {
+    LLVM_DEBUG(dbgs() << "    Abort due to lack of VL, assume that"
+                         " use VLMAX\n");
+    return std::nullopt;
+  }
+
+  // Instructions like reductions may use a vector register as a scalar
+  // register. In this case, we should treat it like a scalar register which
+  // does not impact the decision on whether to optimize VL. But if there is
+  // another user of MI and it may have VL=0, we need to be sure not to reduce
+  // the VL of MI to zero when the VLOp of UserOp may be non-zero. The most
+  // we can reduce it to is one.
+  if (isVectorOpUsedAsScalarOp(UserOp)) {
+    [[maybe_unused]] Register R = UserOp.getReg();
+    [[maybe_unused]] const TargetRegisterClass *RC = MRI->getRegClass(R);
+    assert(RISCV::VRRegClass.hasSubClassEq(RC) &&
+           "Expect LMUL 1 register class for vector as scalar operands!");
+    LLVM_DEBUG(dbgs() << "    Used this operand as a scalar operand\n");
+
+    unsigned VLOpNum = RISCVII::getVLOpNum(Desc);
+    const MachineOperand &VLOp = UserMI.getOperand(VLOpNum);
+    if (VLOp.isReg() || (VLOp.isImm() && VLOp.getImm() != 0))
+      return MachineOperand::CreateImm(1);
----------------
lukel97 wrote:

I think I'm missing something here. We have two claims in the comments. 

> we need to be sure not to reduce the VL of MI to zero when the VLOp of UserOp may be non-zero

This implies that we **may** reduce MI's VL to something smaller than a user's VL. 

But meanwhile in checkUsers:

>  Use the largest VL among all the users. If we cannot determine this statically, then we cannot optimize the VL.

This implies that we **never** reduce MI's VL to something smaller than a user's VL.

My understanding is that checkUsers is correct here, so we will never reduce MI's VL to zero if a user has a non-zero VL.

And if we remove the isVectorOpUsedAsScalarOp check, the MIR test case is still correct:

```diff
diff --git a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
index 0b437e63fa3d..860e56fba839 100644
--- a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
@@ -1066,26 +1066,6 @@ RISCVVLOptimizer::getMinimumVLForUser(MachineOperand &UserOp) {
     return std::nullopt;
   }
 
-  // Instructions like reductions may use a vector register as a scalar
-  // register. In this case, we should treat it like a scalar register which
-  // does not impact the decision on whether to optimize VL. But if there is
-  // another user of MI and it may have VL=0, we need to be sure not to reduce
-  // the VL of MI to zero when the VLOp of UserOp may be non-zero. The most
-  // we can reduce it to is one.
-  if (isVectorOpUsedAsScalarOp(UserOp)) {
-    [[maybe_unused]] Register R = UserOp.getReg();
-    [[maybe_unused]] const TargetRegisterClass *RC = MRI->getRegClass(R);
-    assert(RISCV::VRRegClass.hasSubClassEq(RC) &&
-           "Expect LMUL 1 register class for vector as scalar operands!");
-    LLVM_DEBUG(dbgs() << "    Used this operand as a scalar operand\n");
-
-    unsigned VLOpNum = RISCVII::getVLOpNum(Desc);
-    const MachineOperand &VLOp = UserMI.getOperand(VLOpNum);
-    return VLOp.isReg() || (VLOp.isImm() && VLOp.getImm() != 0)
-               ? MachineOperand::CreateImm(1)
-               : VLOp;
-  }
-
   unsigned VLOpNum = RISCVII::getVLOpNum(Desc);
   const MachineOperand &VLOp = UserMI.getOperand(VLOpNum);
   // Looking for an immediate or a register VL that isn't X0.
diff --git a/llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir b/llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir
index 9486fe92c041..c3e09d6b1438 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/vl-opt-op-info.mir
@@ -1172,13 +1172,13 @@ name: vred_vl0_and_vlreg
 body: |
   bb.0:
     ; CHECK-LABEL: name: vred_vl0_and_vlreg
     ; CHECK: %vl:gprnox0 = COPY $x1
-    ; CHECK-NEXT: %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
+    ; CHECK-NEXT: %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0 /* tu, mu */
     ; CHECK-NEXT: %y:vr = PseudoVREDSUM_VS_M1_E8 $noreg, $noreg, %x, %vl, 3 /* e8 */, 0 /* tu, mu */
     ; CHECK-NEXT: %z:vr = PseudoVADD_VV_M1 $noreg, %x, $noreg, 0, 3 /* e8 */, 0 /* tu, mu */
     %vl:gprnox0 = COPY $x1
     %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0
     %y:vr = PseudoVREDSUM_VS_M1_E8 $noreg, $noreg, %x, %vl, 3 /* e8 */, 0
     %z:vr = PseudoVADD_VV_M1 $noreg, %x, $noreg, 0, 3 /* e8 */, 0
 ...
 ---
```

checkUsers doesn't reduce the VL because VREDSUM's VL `%x` is not statically known to be greater than or equal to VADD's VL `0`.

https://github.com/llvm/llvm-project/pull/120345