Skip to content

[DAGCombiner] Remove no longer tested VP_MUL handling.#201238

Merged
topperc merged 1 commit into
llvm:mainfrom
topperc:pr/visitmul
Jun 3, 2026
Merged

[DAGCombiner] Remove no longer tested VP_MUL handling.#201238
topperc merged 1 commit into
llvm:mainfrom
topperc:pr/visitmul

Conversation

@topperc
Copy link
Copy Markdown
Contributor

@topperc topperc commented Jun 2, 2026

We no longer use VP_MUL in SelectionDAG on RISC-V so this code isn't tested.

This effectively reverts db6de1a

We no longer use VP_MUL in SelectionDAG on RISC-V so this code
isn't tested.

This effectively reverts db6de1a
@llvmorg-github-actions llvmorg-github-actions Bot added the llvm:SelectionDAG SelectionDAGISel as well label Jun 2, 2026
@llvmorg-github-actions
Copy link
Copy Markdown

@llvm/pr-subscribers-llvm-selectiondag

Author: Craig Topper (topperc)

Changes

We no longer use VP_MUL in SelectionDAG on RISC-V so this code isn't tested.

This effectively reverts db6de1a


Full diff: https://github.com/llvm/llvm-project/pull/201238.diff

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+45-62)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 0c9820fb64de9..e26d4c749aea8 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -442,7 +442,7 @@ namespace {
     SDValue visitSUBE(SDNode *N);
     SDValue visitUSUBO_CARRY(SDNode *N);
     SDValue visitSSUBO_CARRY(SDNode *N);
-    template <class MatchContextClass> SDValue visitMUL(SDNode *N);
+    SDValue visitMUL(SDNode *N);
     SDValue visitMULFIX(SDNode *N);
     SDValue useDivRem(SDNode *N);
     SDValue visitSDIV(SDNode *N);
@@ -1958,7 +1958,7 @@ SDValue DAGCombiner::visit(SDNode *N) {
   case ISD::SMULFIXSAT:
   case ISD::UMULFIX:
   case ISD::UMULFIXSAT:         return visitMULFIX(N);
-  case ISD::MUL:                return visitMUL<EmptyMatchContext>(N);
+  case ISD::MUL:                return visitMUL(N);
   case ISD::SDIV:               return visitSDIV(N);
   case ISD::UDIV:               return visitUDIV(N);
   case ISD::SREM:
@@ -4813,14 +4813,12 @@ SDValue DAGCombiner::visitMULFIX(SDNode *N) {
   return SDValue();
 }
 
-template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
+SDValue DAGCombiner::visitMUL(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   EVT VT = N0.getValueType();
   unsigned BitWidth = VT.getScalarSizeInBits();
   SDLoc DL(N);
-  bool UseVP = std::is_same_v<MatchContextClass, VPMatchContext>;
-  MatchContextClass Matcher(DAG, TLI, N);
 
   // fold (mul x, undef) -> 0
   if (N0.isUndef() || N1.isUndef())
@@ -4833,7 +4831,7 @@ template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
   // canonicalize constant to RHS (vector doesn't have to splat)
   if (DAG.isConstantIntBuildVectorOrConstantInt(N0) &&
       !DAG.isConstantIntBuildVectorOrConstantInt(N1))
-    return Matcher.getNode(ISD::MUL, DL, VT, N1, N0);
+    return DAG.getNode(ISD::MUL, DL, VT, N1, N0);
 
   bool N1IsConst = false;
   bool N1IsOpaqueConst = false;
@@ -4841,10 +4839,8 @@ template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
 
   // fold vector ops
   if (VT.isVector()) {
-    // TODO: Change this to use SimplifyVBinOp when it supports VP op.
-    if (!UseVP)
-      if (SDValue FoldedVOp = SimplifyVBinOp(N, DL))
-        return FoldedVOp;
+    if (SDValue FoldedVOp = SimplifyVBinOp(N, DL))
+      return FoldedVOp;
 
     N1IsConst = ISD::isConstantSplatVector(N1.getNode(), ConstValue1);
     assert((!N1IsConst || ConstValue1.getBitWidth() == BitWidth) &&
@@ -4865,13 +4861,12 @@ template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
   if (N1IsConst && ConstValue1.isOne())
     return N0;
 
-  if (!UseVP)
-    if (SDValue NewSel = foldBinOpIntoSelect(N))
-      return NewSel;
+  if (SDValue NewSel = foldBinOpIntoSelect(N))
+    return NewSel;
 
   // fold (mul x, -1) -> 0-x
   if (N1IsConst && ConstValue1.isAllOnes())
-    return Matcher.getNode(ISD::SUB, DL, VT, DAG.getConstant(0, DL, VT), N0);
+    return DAG.getNegative(N0, DL, VT);
 
   // fold (mul x, (1 << c)) -> x << c
   if (isConstantOrConstantVector(N1, /*NoOpaques*/ true) &&
@@ -4886,7 +4881,7 @@ template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
       if (N->getFlags().hasNoSignedWrap() && N1IsConst &&
           ConstValue1.logBase2() < BitWidth - 1)
         Flags.setNoSignedWrap(true);
-      return Matcher.getNode(ISD::SHL, DL, VT, N0, Trunc, Flags);
+      return DAG.getNode(ISD::SHL, DL, VT, N0, Trunc, Flags);
     }
   }
 
@@ -4896,26 +4891,24 @@ template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
 
     // FIXME: If the input is something that is easily negated (e.g. a
     // single-use add), we should put the negate there.
-    return Matcher.getNode(
+    return DAG.getNode(
         ISD::SUB, DL, VT, DAG.getConstant(0, DL, VT),
-        Matcher.getNode(ISD::SHL, DL, VT, N0,
-                        DAG.getShiftAmountConstant(Log2Val, VT, DL)));
+        DAG.getNode(ISD::SHL, DL, VT, N0,
+                    DAG.getShiftAmountConstant(Log2Val, VT, DL)));
   }
 
   // Attempt to reuse an existing umul_lohi/smul_lohi node, but only if the
   // hi result is in use in case we hit this mid-legalization.
-  if (!UseVP) {
-    for (unsigned LoHiOpc : {ISD::UMUL_LOHI, ISD::SMUL_LOHI}) {
-      if (!LegalOperations || TLI.isOperationLegalOrCustom(LoHiOpc, VT)) {
-        SDVTList LoHiVT = DAG.getVTList(VT, VT);
-        // TODO: Can we match commutable operands with getNodeIfExists?
-        if (SDNode *LoHi = DAG.getNodeIfExists(LoHiOpc, LoHiVT, {N0, N1}))
-          if (LoHi->hasAnyUseOfValue(1))
-            return SDValue(LoHi, 0);
-        if (SDNode *LoHi = DAG.getNodeIfExists(LoHiOpc, LoHiVT, {N1, N0}))
-          if (LoHi->hasAnyUseOfValue(1))
-            return SDValue(LoHi, 0);
-      }
+  for (unsigned LoHiOpc : {ISD::UMUL_LOHI, ISD::SMUL_LOHI}) {
+    if (!LegalOperations || TLI.isOperationLegalOrCustom(LoHiOpc, VT)) {
+      SDVTList LoHiVT = DAG.getVTList(VT, VT);
+      // TODO: Can we match commutable operands with getNodeIfExists?
+      if (SDNode *LoHi = DAG.getNodeIfExists(LoHiOpc, LoHiVT, {N0, N1}))
+        if (LoHi->hasAnyUseOfValue(1))
+          return SDValue(LoHi, 0);
+      if (SDNode *LoHi = DAG.getNodeIfExists(LoHiOpc, LoHiVT, {N1, N0}))
+        if (LoHi->hasAnyUseOfValue(1))
+          return SDValue(LoHi, 0);
     }
   }
 
@@ -4934,8 +4927,7 @@ template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
   //           x * 0xf800 --> (x << 16) - (x << 11)
   //           x * -0x8800 --> -((x << 15) + (x << 11))
   //           x * -0xf800 --> -((x << 16) - (x << 11)) ; (x << 11) - (x << 16)
-  if (!UseVP && N1IsConst &&
-      TLI.decomposeMulByConstant(*DAG.getContext(), VT, N1)) {
+  if (N1IsConst && TLI.decomposeMulByConstant(*DAG.getContext(), VT, N1)) {
     // TODO: We could handle more general decomposition of any constant by
     //       having the target set a limit on number of ops and making a
     //       callback to determine that sequence (similar to sqrt expansion).
@@ -4971,7 +4963,7 @@ template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
   // (mul (shl X, c1), c2) -> (mul X, c2 << c1)
   {
     SDValue X, C1;
-    if (sd_context_match(N0, Matcher, m_Shl(m_Value(X), m_Value(C1))))
+    if (sd_match(N0, m_Shl(m_Value(X), m_Value(C1))))
       if (SDValue C3 = DAG.FoldConstantArithmetic(ISD::SHL, DL, VT, {N1, C1}))
         return DAG.getNode(ISD::MUL, DL, VT, X, C3);
   }
@@ -4980,29 +4972,27 @@ template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
   // use.
   {
     SDValue X, C, Y;
-    if (sd_context_match(
-            N, Matcher,
-            m_Mul(m_OneUse(m_Shl(m_Value(X), m_Value(C))), m_Value(Y))) &&
+    if (sd_match(N,
+                 m_Mul(m_OneUse(m_Shl(m_Value(X), m_Value(C))), m_Value(Y))) &&
         isConstantOrConstantVector(C)) {
-      SDValue Mul = Matcher.getNode(ISD::MUL, DL, VT, X, Y);
-      return Matcher.getNode(ISD::SHL, DL, VT, Mul, C);
+      SDValue Mul = DAG.getNode(ISD::MUL, DL, VT, X, Y);
+      return DAG.getNode(ISD::SHL, DL, VT, Mul, C);
     }
   }
 
   // fold (mul (add x, c1), c2) -> (add (mul x, c2), c1*c2)
-  if (sd_context_match(N0, Matcher, m_SpecificOpc(ISD::ADD)) &&
-      isConstantOrConstantVector(N1) &&
+  if (sd_match(N0, m_SpecificOpc(ISD::ADD)) && isConstantOrConstantVector(N1) &&
       isConstantOrConstantVector(N0.getOperand(1)) &&
       isMulAddWithConstProfitable(N, N0, N1))
-    return Matcher.getNode(
+    return DAG.getNode(
         ISD::ADD, DL, VT,
-        Matcher.getNode(ISD::MUL, SDLoc(N0), VT, N0.getOperand(0), N1),
-        Matcher.getNode(ISD::MUL, SDLoc(N1), VT, N0.getOperand(1), N1));
+        DAG.getNode(ISD::MUL, SDLoc(N0), VT, N0.getOperand(0), N1),
+        DAG.getNode(ISD::MUL, SDLoc(N1), VT, N0.getOperand(1), N1));
 
   // Fold (mul (vscale * C0), C1) to (vscale * (C0 * C1)).
   // avoid if ISD::MUL handling is poor and ISD::SHL isn't an option.
   ConstantSDNode *NC1 = isConstOrConstSplat(N1);
-  if (!UseVP && N0.getOpcode() == ISD::VSCALE && NC1) {
+  if (N0.getOpcode() == ISD::VSCALE && NC1) {
     const APInt &C0 = N0.getConstantOperandAPInt(0);
     const APInt &C1 = NC1->getAPIntValue();
     if (!C0.isPowerOf2() || C1.isPowerOf2() ||
@@ -5012,7 +5002,7 @@ template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
 
   // Fold (mul step_vector(C0), C1) to (step_vector(C0 * C1)).
   APInt MulVal;
-  if (!UseVP && N0.getOpcode() == ISD::STEP_VECTOR &&
+  if (N0.getOpcode() == ISD::STEP_VECTOR &&
       ISD::isConstantSplatVector(N1.getNode(), MulVal)) {
     const APInt &C0 = N0.getConstantOperandAPInt(0);
     APInt NewStep = C0 * MulVal;
@@ -5021,12 +5011,11 @@ template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
 
   // Fold Y = sra (X, size(X)-1); mul (or (Y, 1), X) -> (abs X)
   SDValue X;
-  if (!UseVP && (!LegalOperations || hasOperation(ISD::ABS, VT)) &&
-      sd_context_match(
-          N, Matcher,
-          m_Mul(m_Or(m_Sra(m_Value(X), m_SpecificInt(BitWidth - 1)), m_One()),
-                m_Deferred(X)))) {
-    return Matcher.getNode(ISD::ABS, DL, VT, X);
+  if ((!LegalOperations || hasOperation(ISD::ABS, VT)) &&
+      sd_match(N, m_Mul(m_Or(m_Sra(m_Value(X), m_SpecificInt(BitWidth - 1)),
+                             m_One()),
+                        m_Deferred(X)))) {
+    return DAG.getNode(ISD::ABS, DL, VT, X);
   }
 
   // Fold ((mul x, 0/undef) -> 0,
@@ -5060,17 +5049,13 @@ template <class MatchContextClass> SDValue DAGCombiner::visitMUL(SDNode *N) {
   }
 
   // reassociate mul
-  // TODO: Change reassociateOps to support vp ops.
-  if (!UseVP)
-    if (SDValue RMUL = reassociateOps(ISD::MUL, DL, N0, N1, N->getFlags()))
-      return RMUL;
+  if (SDValue RMUL = reassociateOps(ISD::MUL, DL, N0, N1, N->getFlags()))
+    return RMUL;
 
   // Fold mul(vecreduce(x), vecreduce(y)) -> vecreduce(mul(x, y))
-  // TODO: Change reassociateReduction to support vp ops.
-  if (!UseVP)
-    if (SDValue SD =
-            reassociateReduction(ISD::VECREDUCE_MUL, ISD::MUL, DL, VT, N0, N1))
-      return SD;
+  if (SDValue SD =
+          reassociateReduction(ISD::VECREDUCE_MUL, ISD::MUL, DL, VT, N0, N1))
+    return SD;
 
   // Simplify the operands using demanded-bits information.
   if (SimplifyDemandedBits(SDValue(N, 0)))
@@ -29672,8 +29657,6 @@ SDValue DAGCombiner::visitVPOp(SDNode *N) {
       return visitFMA<VPMatchContext>(N);
     case ISD::VP_SELECT:
       return visitVP_SELECT(N);
-    case ISD::VP_MUL:
-      return visitMUL<VPMatchContext>(N);
     case ISD::VP_SUB:
       return foldSubCtlzNot<VPMatchContext>(N, DAG);
     default:

Copy link
Copy Markdown
Contributor

@jacquesguan jacquesguan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@topperc topperc merged commit 866c39b into llvm:main Jun 3, 2026
12 checks passed
@topperc topperc deleted the pr/visitmul branch June 3, 2026 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants