[llvm] [SelectionDAG][x86] Ensure vector reduction optimization (PR #144231)

via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 17 11:18:15 PDT 2025


Suhajda =?utf-8?q?Tamás?= <sutajo at gmail.com>,
Suhajda =?utf-8?q?Tamás?= <sutajo at gmail.com>,
Suhajda =?utf-8?q?Tamás?= <sutajo at gmail.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/144231 at github.com>


github-actions[bot] wrote:

<!--LLVM CODE FORMAT COMMENT: {clang-format}-->


:warning: C/C++ code formatter, clang-format found issues in your code. :warning:

<details>
<summary>
You can test this locally with the following command:
</summary>

``````````bash
git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- llvm/lib/Target/X86/X86ISelLowering.cpp llvm/lib/Target/X86/X86TargetTransformInfo.cpp llvm/lib/Target/X86/X86TargetTransformInfo.h
``````````

</details>

<details>
<summary>
View the diff from clang-format here.
</summary>

``````````diff
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 1e98a0547..b143cb6ad 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -1436,11 +1436,9 @@ X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
   }
 
   // Vector min/max reductions
-  if (Subtarget.hasSSE41())
-  {
+  if (Subtarget.hasSSE41()) {
     for (MVT VT : MVT::vector_valuetypes()) {
-      if (VT.getScalarType() == MVT::i8 || VT.getScalarType() == MVT::i16)
-      {
+      if (VT.getScalarType() == MVT::i8 || VT.getScalarType() == MVT::i16) {
         setOperationAction(ISD::VECREDUCE_UMIN, VT, Custom);
         setOperationAction(ISD::VECREDUCE_UMAX, VT, Custom);
         setOperationAction(ISD::VECREDUCE_SMIN, VT, Custom);
@@ -25411,9 +25409,9 @@ static SDValue LowerEXTEND_VECTOR_INREG(SDValue Op,
 // Create a min/max v8i16/v16i8 horizontal reduction with PHMINPOSUW.
 static SDValue createMinMaxReduction(SDValue Src, EVT TargetVT, SDLoc DL,
                                      ISD::NodeType BinOp, SelectionDAG &DAG,
-                                     const X86Subtarget &Subtarget)
-{
-  assert(Subtarget.hasSSE41() && "The caller must check if SSE4.1 is available");
+                                     const X86Subtarget &Subtarget) {
+  assert(Subtarget.hasSSE41() &&
+         "The caller must check if SSE4.1 is available");
 
   EVT SrcVT = Src.getValueType();
   EVT SrcSVT = SrcVT.getScalarType();
@@ -25469,31 +25467,29 @@ static SDValue createMinMaxReduction(SDValue Src, EVT TargetVT, SDLoc DL,
 }
 
 static SDValue LowerVECTOR_REDUCE_MINMAX(SDValue Op,
-    const X86Subtarget& Subtarget,
-    SelectionDAG& DAG)
-{
+                                         const X86Subtarget &Subtarget,
+                                         SelectionDAG &DAG) {
   ISD::NodeType BinOp;
-  switch (Op.getOpcode())
-  {
-    default: 
-      assert(false && "Expected min/max reduction");
-      break;
-    case ISD::VECREDUCE_UMIN:
-      BinOp = ISD::UMIN;
-      break;
-    case ISD::VECREDUCE_UMAX:
-      BinOp = ISD::UMAX;
-      break;
-    case ISD::VECREDUCE_SMIN:
-      BinOp = ISD::SMIN;
-      break;
-    case ISD::VECREDUCE_SMAX:
-      BinOp = ISD::SMAX;
-      break;
+  switch (Op.getOpcode()) {
+  default:
+    assert(false && "Expected min/max reduction");
+    break;
+  case ISD::VECREDUCE_UMIN:
+    BinOp = ISD::UMIN;
+    break;
+  case ISD::VECREDUCE_UMAX:
+    BinOp = ISD::UMAX;
+    break;
+  case ISD::VECREDUCE_SMIN:
+    BinOp = ISD::SMIN;
+    break;
+  case ISD::VECREDUCE_SMAX:
+    BinOp = ISD::SMAX;
+    break;
   }
 
   return createMinMaxReduction(Op->getOperand(0), Op.getValueType(), SDLoc(Op),
-      BinOp, DAG, Subtarget);
+                               BinOp, DAG, Subtarget);
 }
 
 static SDValue LowerSIGN_EXTEND(SDValue Op, const X86Subtarget &Subtarget,
@@ -46218,8 +46214,8 @@ static SDValue combineMinMaxReduction(SDNode *Extract, SelectionDAG &DAG,
   if (!Src)
     return SDValue();
 
-  return createMinMaxReduction(Src, ExtractVT, SDLoc(Extract),
-      BinOp, DAG, Subtarget);
+  return createMinMaxReduction(Src, ExtractVT, SDLoc(Extract), BinOp, DAG,
+                               Subtarget);
 }
 
 // Attempt to replace an all_of/any_of/parity style horizontal reduction with a MOVMSK.
@@ -47052,8 +47048,8 @@ static SDValue combineArithReduction(SDNode *ExtElt, SelectionDAG &DAG,
 /// scalars back, while for x64 we should use 64-bit extracts and shifts.
 static SDValue combineExtractVectorElt(SDNode *N, SelectionDAG &DAG,
                                        TargetLowering::DAGCombinerInfo &DCI,
-                                       const X86Subtarget &Subtarget, 
-                                       bool& TransformedBinOpReduction) {
+                                       const X86Subtarget &Subtarget,
+                                       bool &TransformedBinOpReduction) {
   if (SDValue NewOp = combineExtractWithShuffle(N, DAG, DCI, Subtarget))
     return NewOp;
 
@@ -47237,26 +47233,27 @@ static SDValue combineExtractVectorElt(SDNode *N, SelectionDAG &DAG,
   return SDValue();
 }
 
-static SDValue combineExtractVectorEltAndOperand(SDNode* N, SelectionDAG& DAG,
-    TargetLowering::DAGCombinerInfo& DCI,
-    const X86Subtarget& Subtarget)
-{
+static SDValue
+combineExtractVectorEltAndOperand(SDNode *N, SelectionDAG &DAG,
+                                  TargetLowering::DAGCombinerInfo &DCI,
+                                  const X86Subtarget &Subtarget) {
   bool TransformedBinOpReduction = false;
-  auto Op = combineExtractVectorElt(N, DAG, DCI, Subtarget, TransformedBinOpReduction);
+  auto Op = combineExtractVectorElt(N, DAG, DCI, Subtarget,
+                                    TransformedBinOpReduction);
 
-  if (TransformedBinOpReduction)
-  {
+  if (TransformedBinOpReduction) {
     // In case we simplified N = extract_vector_element(V, 0) with Op and V
     // resulted from a reduction, then we need to replace all uses of V with
     // scalar_to_vector(Op) to make sure that we eliminated the binop + shuffle
-    // pyramid. This is safe to do, because the elements of V are undefined except 
-    // for the zeroth element and Op does not depend on V.
+    // pyramid. This is safe to do, because the elements of V are undefined
+    // except for the zeroth element and Op does not depend on V.
 
     auto OldV = N->getOperand(0);
-    assert(!Op.getNode()->hasPredecessor(OldV.getNode()) && 
-        "Op must not depend on the converted reduction");
+    assert(!Op.getNode()->hasPredecessor(OldV.getNode()) &&
+           "Op must not depend on the converted reduction");
 
-    auto NewV = DAG.getNode(ISD::SCALAR_TO_VECTOR, SDLoc(N), OldV->getValueType(0), Op);
+    auto NewV =
+        DAG.getNode(ISD::SCALAR_TO_VECTOR, SDLoc(N), OldV->getValueType(0), Op);
 
     auto NV = DCI.CombineTo(N, Op);
     DCI.CombineTo(OldV.getNode(), NewV);
diff --git a/llvm/lib/Target/X86/X86TargetTransformInfo.cpp b/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
index 3c479fc72..e753946d0 100644
--- a/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
+++ b/llvm/lib/Target/X86/X86TargetTransformInfo.cpp
@@ -6586,10 +6586,9 @@ bool llvm::X86TTIImpl::shouldExpandReduction(const IntrinsicInst *II) const {
   case Intrinsic::vector_reduce_smax:
     auto *VType = cast<FixedVectorType>(II->getOperand(0)->getType());
     auto SType = VType->getScalarType();
-    bool CanUsePHMINPOSUW = 
-        ST->hasSSE41() && II->getType() == SType &&
-        (VType->getPrimitiveSizeInBits() % 128) == 0 &&
-        (SType->isIntegerTy(8) || SType->isIntegerTy(16));
+    bool CanUsePHMINPOSUW = ST->hasSSE41() && II->getType() == SType &&
+                            (VType->getPrimitiveSizeInBits() % 128) == 0 &&
+                            (SType->isIntegerTy(8) || SType->isIntegerTy(16));
     return !CanUsePHMINPOSUW;
   }
 }

``````````

</details>


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


More information about the llvm-commits mailing list