Address review comments

Author: el-ev

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

@@ -15503,8 +15503,11 @@ static SDValue performXORCombine(SDNode *N, SelectionDAG &DAG,
   return combineSelectAndUseCommutative(N, DAG, /*AllOnes*/ false, Subtarget);
 }
 
+// Try to expand a multiply to a sequence of shifts and add/subs,
+// for a machine without native mul instruction.
 static SDValue expandMulToNAFSequence(SDNode *N, SelectionDAG &DAG,
-                                      const SDLoc &DL, uint64_t MulAmt) {
+                                      uint64_t MulAmt) {
+  SDLoc DL(N);
   EVT VT = N->getValueType(0);
   const uint64_t BitWidth = VT.getFixedSizeInBits();
 
@@ -15522,85 +15525,13 @@ static SDValue expandMulToNAFSequence(SDNode *N, SelectionDAG &DAG,
   SDValue N0 = N->getOperand(0);
 
   for (const auto &Op : Sequence) {
-    SDValue ShiftVal;
-    if (Op.second > 0)
-      ShiftVal =
-          DAG.getNode(ISD::SHL, DL, VT, N0, DAG.getConstant(Op.second, DL, VT));
-    else
-      ShiftVal = N0;
-
+    SDValue ShiftVal = DAG.getNode(
+        ISD::SHL, DL, VT, N0, DAG.getShiftAmountConstant(Op.second, VT, DL));
     ISD::NodeType AddSubOp = Op.first ? ISD::ADD : ISD::SUB;
     Result = DAG.getNode(AddSubOp, DL, VT, Result, ShiftVal);
   }
   return Result;
 }
-// Try to expand a multiply to a sequence of shifts and add/subs,
-// for a machine without native mul instruction.
-static SDValue expandMulToBasicOps(SDNode *N, SelectionDAG &DAG,
-                                   uint64_t MulAmt) {
-  EVT VT = N->getValueType(0);
-  const uint64_t BitWidth = VT.getFixedSizeInBits();
-  SDLoc DL(N);
-
-  if (MulAmt == 0)
-    return DAG.getConstant(0, DL, N->getValueType(0));
-
-  // Try to factorize into (2^N) * (2^M_1 +/- 1) * (2^M_2 +/- 1) * ...
-  uint64_t TrailingZeros = llvm::countr_zero(MulAmt);
-  uint64_t E = MulAmt >> TrailingZeros;
-
-  llvm::SmallVector<std::pair<bool, uint64_t>> Factors; // {is_2^M+1, M}
-
-  while (E > 1) {
-    bool Found = false;
-    for (int64_t I = BitWidth - 1; I >= 2; --I) {
-      uint64_t Factor = 1ULL << I;
-
-      if (E % (Factor + 1) == 0) {
-        Factors.push_back({true, I});
-        E /= Factor + 1;
-        Found = true;
-        break;
-      }
-      if (E % (Factor - 1) == 0) {
-        Factors.push_back({false, I});
-        E /= Factor - 1;
-        Found = true;
-        break;
-      }
-    }
-    if (!Found)
-      break;
-  }
-
-  SDValue Result;
-  SDValue N0 = N->getOperand(0);
-
-  bool UseFactorization = !Factors.empty() && (Factors.size() < 5);
-
-  if (UseFactorization) {
-    if (E == 1)
-      Result = N0;
-    else
-      Result = expandMulToNAFSequence(N, DAG, DL, E);
-
-    for (const auto &F : Factors) {
-      SDValue ShiftVal = DAG.getNode(ISD::SHL, DL, VT, Result,
-                                     DAG.getConstant(F.second, DL, VT));
-
-      ISD::NodeType AddSubOp = F.first ? ISD::ADD : ISD::SUB;
-      Result = DAG.getNode(AddSubOp, DL, N->getValueType(0), ShiftVal, Result);
-    }
-
-    if (TrailingZeros > 0)
-      Result = DAG.getNode(ISD::SHL, DL, VT, Result,
-                           DAG.getConstant(TrailingZeros, DL, VT));
-
-    return Result;
-  }
-
-  return expandMulToNAFSequence(N, DAG, DL, MulAmt);
-}
 
 // X * (2^N +/- 2^M) -> (add/sub (shl X, C1), (shl X, C2))
 static SDValue expandMulToAddOrSubOfShl(SDNode *N, SelectionDAG &DAG,
@@ -15640,17 +15571,17 @@ static SDValue expandMul(SDNode *N, SelectionDAG &DAG,
   if (VT != Subtarget.getXLenVT())
     return SDValue();
 
+  bool ShouldExpandMul =
+      (!DCI.isBeforeLegalize() && !DCI.isCalledByLegalizer()) ||
+      !Subtarget.hasStdExtZmmul();
+  if (!ShouldExpandMul)
+    return SDValue();
+
   ConstantSDNode *CNode = dyn_cast<ConstantSDNode>(N->getOperand(1));
   if (!CNode)
     return SDValue();
   uint64_t MulAmt = CNode->getZExtValue();
 
-  if (!Subtarget.hasStdExtM() && !Subtarget.hasStdExtZmmul())
-    return expandMulToBasicOps(N, DAG, MulAmt);
-
-  if (DCI.isBeforeLegalize() || DCI.isCalledByLegalizer())
-    return SDValue();
-
   const bool HasShlAdd = Subtarget.hasStdExtZba() ||
                          Subtarget.hasVendorXTHeadBa() ||
                          Subtarget.hasVendorXAndesPerf();
@@ -15792,6 +15723,9 @@ static SDValue expandMul(SDNode *N, SelectionDAG &DAG,
   if (SDValue V = expandMulToAddOrSubOfShl(N, DAG, MulAmt))
     return V;
 
+  if (!Subtarget.hasStdExtZmmul())
+    return expandMulToNAFSequence(N, DAG, MulAmt);
+
   return SDValue();
 }