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

[Philip Reames via llvm-commits]

================
@@ -930,24 +952,46 @@ bool RISCVVLOptimizer::isCandidate(const MachineInstr &MI) const {
   return true;
 }
 
-bool RISCVVLOptimizer::checkUsers(const MachineOperand *&CommonVL,
-                                  MachineInstr &MI) {
+std::optional<const MachineOperand>
+RISCVVLOptimizer::checkUsers(MachineInstr &MI) {
   // FIXME: Avoid visiting each user for each time we visit something on the
   // worklist, combined with an extra visit from the outer loop. Restructure
   // along lines of an instcombine style worklist which integrates the outer
   // pass.
   bool CanReduceVL = true;
+  const MachineOperand *CommonVL = nullptr;
+  const MachineOperand One = MachineOperand::CreateImm(1);
   for (auto &UserOp : MRI->use_operands(MI.getOperand(0).getReg())) {
     const MachineInstr &UserMI = *UserOp.getParent();
     LLVM_DEBUG(dbgs() << "  Checking user: " << UserMI << "\n");
 
     // 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.
-    // TODO: Treat it like a scalar register instead of bailing out.
+    // 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 is may be non-zero. The most
+    // we can reduce it to is one.
     if (isVectorOpUsedAsScalarOp(UserOp)) {
-      CanReduceVL = false;
-      break;
+      [[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");
+      const MCInstrDesc &Desc = UserMI.getDesc();
+      // VMV_X_S and VFMV_F_S do not have a VL opt which would cause an assert
+      // assert failure if we called getVLOpNum. Therefore, we will set the
+      // CommonVL in that case as 1, even if it could have been set to 0.
+      if (!RISCVII::hasVLOp(Desc.TSFlags) || !RISCVII::hasSEWOp(Desc.TSFlags)) {
----------------
preames wrote:

This is not correct.  Consider the case where there is another use of the instruction *which was visited first*.  This blindly writes over the common VL.  This would allow reducing VL more than allowed by the *other user*.  

You are also missing the SEW correctness check I pointed out earlier. 

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