[RISCV][DAGCombiner] Restrict VL->VW extension combine to single node

Previously, combineOp_VLToVWOp_VL maintained a private worklist and
recursively folded multiple nodes. This could cause inconsistent DAG
state and missed updates for some users.

This patch simplifies the combine to only attempt folding the input node,
leaving replacement and user updates to the outer DAGCombiner worklist.

The DEBUG-only option `-ext-max-web-size` is now obsolete and have been
removed.

A new test (combine-vl-vw-macc.ll) is added to verify that consecutive
vwmacc-like operations are generated correctly.

Author: OMG-link

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

@@ -57,12 +57,6 @@ using namespace llvm;
 
 STATISTIC(NumTailCalls, "Number of tail calls");
 
-static cl::opt<unsigned> ExtensionMaxWebSize(
-    DEBUG_TYPE "-ext-max-web-size", cl::Hidden,
-    cl::desc("Give the maximum size (in number of nodes) of the web of "
-             "instructions that we will consider for VW expansion"),
-    cl::init(18));
-
 static cl::opt<bool>
     AllowSplatInVW_W(DEBUG_TYPE "-form-vw-w-with-splat", cl::Hidden,
                      cl::desc("Allow the formation of VW_W operations (e.g., "
@@ -18201,109 +18195,30 @@ static SDValue combineOp_VLToVWOp_VL(SDNode *N,
   if (!NodeExtensionHelper::isSupportedRoot(N, Subtarget))
     return SDValue();
 
-  SmallVector<SDNode *> Worklist;
-  SmallPtrSet<SDNode *, 8> Inserted;
-  SmallPtrSet<SDNode *, 8> ExtensionsToRemove;
-  Worklist.push_back(N);
-  Inserted.insert(N);
-  SmallVector<CombineResult> CombinesToApply;
-
-  while (!Worklist.empty()) {
-    SDNode *Root = Worklist.pop_back_val();
-
-    NodeExtensionHelper LHS(Root, 0, DAG, Subtarget);
-    NodeExtensionHelper RHS(Root, 1, DAG, Subtarget);
-    auto AppendUsersIfNeeded =
-        [&Worklist, &Subtarget, &Inserted,
-         &ExtensionsToRemove](const NodeExtensionHelper &Op) {
-          if (Op.needToPromoteOtherUsers()) {
-            // Remember that we're supposed to remove this extension.
-            ExtensionsToRemove.insert(Op.OrigOperand.getNode());
-            for (SDUse &Use : Op.OrigOperand->uses()) {
-              SDNode *TheUser = Use.getUser();
-              if (!NodeExtensionHelper::isSupportedRoot(TheUser, Subtarget))
-                return false;
-              // We only support the first 2 operands of FMA.
-              if (Use.getOperandNo() >= 2)
-                return false;
-              if (Inserted.insert(TheUser).second)
-                Worklist.push_back(TheUser);
-            }
-          }
-          return true;
-        };
+  NodeExtensionHelper LHS(N, 0, DAG, Subtarget);
+  NodeExtensionHelper RHS(N, 1, DAG, Subtarget);
 
-    // Control the compile time by limiting the number of node we look at in
-    // total.
-    if (Inserted.size() > ExtensionMaxWebSize)
-      return SDValue();
+  SmallVector<NodeExtensionHelper::CombineToTry> FoldingStrategies =
+      NodeExtensionHelper::getSupportedFoldings(N);
 
-    SmallVector<NodeExtensionHelper::CombineToTry> FoldingStrategies =
-        NodeExtensionHelper::getSupportedFoldings(Root);
-
-    assert(!FoldingStrategies.empty() && "Nothing to be folded");
-    bool Matched = false;
-    for (int Attempt = 0;
-         (Attempt != 1 + NodeExtensionHelper::isCommutative(Root)) && !Matched;
-         ++Attempt) {
-
-      for (NodeExtensionHelper::CombineToTry FoldingStrategy :
-           FoldingStrategies) {
-        std::optional<CombineResult> Res =
-            FoldingStrategy(Root, LHS, RHS, DAG, Subtarget);
-        if (Res) {
-          // If this strategy wouldn't remove an extension we're supposed to
-          // remove, reject it.
-          if (!Res->LHSExt.has_value() &&
-              ExtensionsToRemove.contains(LHS.OrigOperand.getNode()))
-            continue;
-          if (!Res->RHSExt.has_value() &&
-              ExtensionsToRemove.contains(RHS.OrigOperand.getNode()))
-            continue;
+  if (FoldingStrategies.empty())
+    return SDValue();
 
-          Matched = true;
-          CombinesToApply.push_back(*Res);
-          // All the inputs that are extended need to be folded, otherwise
-          // we would be leaving the old input (since it is may still be used),
-          // and the new one.
-          if (Res->LHSExt.has_value())
-            if (!AppendUsersIfNeeded(LHS))
-              return SDValue();
-          if (Res->RHSExt.has_value())
-            if (!AppendUsersIfNeeded(RHS))
-              return SDValue();
-          break;
-        }
+  bool IsComm = NodeExtensionHelper::isCommutative(N);
+  for (int Attempt = 0; Attempt != 1 + IsComm; ++Attempt) {
+    for (NodeExtensionHelper::CombineToTry FoldingStrategy :
+         FoldingStrategies) {
+      std::optional<CombineResult> Res =
+          FoldingStrategy(N, LHS, RHS, DAG, Subtarget);
+      if (Res) {
+        SDValue NewValue = Res->materialize(DAG, Subtarget);
+        return NewValue;
       }
-      std::swap(LHS, RHS);
-    }
-    // Right now we do an all or nothing approach.
-    if (!Matched)
-      return SDValue();
-  }
-  // Store the value for the replacement of the input node separately.
-  SDValue InputRootReplacement;
-  // We do the RAUW after we materialize all the combines, because some replaced
-  // nodes may be feeding some of the yet-to-be-replaced nodes. Put differently,
-  // some of these nodes may appear in the NodeExtensionHelpers of some of the
-  // yet-to-be-visited CombinesToApply roots.
-  SmallVector<std::pair<SDValue, SDValue>> ValuesToReplace;
-  ValuesToReplace.reserve(CombinesToApply.size());
-  for (CombineResult Res : CombinesToApply) {
-    SDValue NewValue = Res.materialize(DAG, Subtarget);
-    if (!InputRootReplacement) {
-      assert(Res.Root == N &&
-             "First element is expected to be the current node");
-      InputRootReplacement = NewValue;
-    } else {
-      ValuesToReplace.emplace_back(SDValue(Res.Root, 0), NewValue);
     }
+    std::swap(LHS, RHS);
   }
-  for (std::pair<SDValue, SDValue> OldNewValues : ValuesToReplace) {
-    DAG.ReplaceAllUsesOfValueWith(OldNewValues.first, OldNewValues.second);
-    DCI.AddToWorklist(OldNewValues.second.getNode());
-  }
-  return InputRootReplacement;
+
+  return SDValue();
 }
 
 // Fold (vwadd(u).wv y, (vmerge cond, x, 0)) -> vwadd(u).wv y, x, y, cond

llvm/test/CodeGen/RISCV/rvv/combine-vl-vw-macc.ll

@@ -0,0 +1,54 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -mtriple=riscv32 -mattr=+v -verify-machineinstrs < %s | FileCheck %s --check-prefixes=CHECK,RV32
+; RUN: llc -mtriple=riscv64 -mattr=+v -verify-machineinstrs < %s | FileCheck %s --check-prefixes=CHECK,RV64
+
+define void @matmul_min(<32 x i8>* %vptr, i8* %scalars, <32 x i16>* %acc0_ptr, <32 x i16>* %acc1_ptr) {
+; CHECK-LABEL: matmul_min:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    li a4, 64
+; CHECK-NEXT:    li a5, 32
+; CHECK-NEXT:    vsetvli zero, a5, e8, m2, ta, ma
+; CHECK-NEXT:    vle8.v v16, (a0)
+; CHECK-NEXT:    lb a0, 0(a1)
+; CHECK-NEXT:    lb a1, 1(a1)
+; CHECK-NEXT:    vsetvli zero, a4, e8, m4, ta, ma
+; CHECK-NEXT:    vle8.v v8, (a2)
+; CHECK-NEXT:    vle8.v v12, (a3)
+; CHECK-NEXT:    vsetvli zero, a5, e8, m2, ta, ma
+; CHECK-NEXT:    vwmacc.vx v8, a0, v16
+; CHECK-NEXT:    vwmacc.vx v12, a1, v16
+; CHECK-NEXT:    vsetvli zero, a4, e8, m4, ta, ma
+; CHECK-NEXT:    vse8.v v8, (a2)
+; CHECK-NEXT:    vse8.v v12, (a3)
+; CHECK-NEXT:    ret
+entry:
+  %acc0 = load <32 x i16>, <32 x i16>* %acc0_ptr, align 1
+  %acc1 = load <32 x i16>, <32 x i16>* %acc1_ptr, align 1
+
+  %v8 = load <32 x i8>, <32 x i8>* %vptr, align 1
+  %v16 = sext <32 x i8> %v8 to <32 x i16>
+
+  %s0_ptr = getelementptr i8, i8* %scalars, i32 0
+  %s0_i8 = load i8, i8* %s0_ptr, align 1
+  %s0_i16 = sext i8 %s0_i8 to i16
+  %tmp0 = insertelement <32 x i16> undef, i16 %s0_i16, i32 0
+  %splat0 = shufflevector <32 x i16> %tmp0, <32 x i16> undef, <32 x i32> zeroinitializer
+  %mul0 = mul <32 x i16> %splat0, %v16
+  %add0 = add <32 x i16> %mul0, %acc0
+
+  %s1_ptr = getelementptr i8, i8* %scalars, i32 1
+  %s1_i8 = load i8, i8* %s1_ptr, align 1
+  %s1_i16 = sext i8 %s1_i8 to i16
+  %tmp1 = insertelement <32 x i16> undef, i16 %s1_i16, i32 0
+  %splat1 = shufflevector <32 x i16> %tmp1, <32 x i16> undef, <32 x i32> zeroinitializer
+  %mul1 = mul <32 x i16> %splat1, %v16
+  %add1 = add <32 x i16> %mul1, %acc1
+
+  store <32 x i16> %add0, <32 x i16>* %acc0_ptr, align 1
+  store <32 x i16> %add1, <32 x i16>* %acc1_ptr, align 1
+
+  ret void
+}
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; RV32: {{.*}}
+; RV64: {{.*}}