[X86] checkBitcastSrcVectorSize - early return when reach to MaxRecursionDepth. #130226

haonanya1 · 2025-03-07T03:23:47Z

No description provided.

github-actions · 2025-03-07T03:24:08Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-03-07T03:24:43Z

@llvm/pr-subscribers-backend-x86

Author: haonan (haonanya1)

Changes

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

1 Files Affected:

(modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+53-21)

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index deab638b7e546..1e58222f9bea1 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -63,6 +63,7 @@
 #include <bitset>
 #include <cctype>
 #include <numeric>
+#include <tuple>
 using namespace llvm;
 
 #define DEBUG_TYPE "x86-isel"
@@ -44745,31 +44746,59 @@ bool X86TargetLowering::isSplatValueForTargetNode(SDValue Op,
 
 // Helper to peek through bitops/trunc/setcc to determine size of source vector.
 // Allows combineBitcastvxi1 to determine what size vector generated a <X x i1>.
-static bool checkBitcastSrcVectorSize(SDValue Src, unsigned Size,
-                                      bool AllowTruncate) {
+static bool
+checkBitcastSrcVectorSize(SDValue Src, unsigned Size, bool AllowTruncate,
+                          std::map<std::tuple<SDValue, unsigned, bool>, bool>
+                              &BitcastSrcVectorSizeMap) {
+  auto Tp = std::make_tuple(Src, Size, AllowTruncate);
+  if (BitcastSrcVectorSizeMap.count(Tp))
+    return BitcastSrcVectorSizeMap[Tp];
   switch (Src.getOpcode()) {
   case ISD::TRUNCATE:
-    if (!AllowTruncate)
+    if (!AllowTruncate) {
+      BitcastSrcVectorSizeMap[Tp] = false;
       return false;
+    }
     [[fallthrough]];
-  case ISD::SETCC:
-    return Src.getOperand(0).getValueSizeInBits() == Size;
-  case ISD::FREEZE:
-    return checkBitcastSrcVectorSize(Src.getOperand(0), Size, AllowTruncate);
+  case ISD::SETCC: {
+    auto Ret = Src.getOperand(0).getValueSizeInBits() == Size;
+    BitcastSrcVectorSizeMap[Tp] = Ret;
+    return Ret;
+  }
+  case ISD::FREEZE: {
+    auto Ret = checkBitcastSrcVectorSize(Src.getOperand(0), Size, AllowTruncate,
+                                         BitcastSrcVectorSizeMap);
+    BitcastSrcVectorSizeMap[Tp] = Ret;
+    return Ret;
+  }
   case ISD::AND:
   case ISD::XOR:
-  case ISD::OR:
-    return checkBitcastSrcVectorSize(Src.getOperand(0), Size, AllowTruncate) &&
-           checkBitcastSrcVectorSize(Src.getOperand(1), Size, AllowTruncate);
+  case ISD::OR: {
+    auto Ret1 = checkBitcastSrcVectorSize(
+        Src.getOperand(0), Size, AllowTruncate, BitcastSrcVectorSizeMap);
+    auto Ret2 = checkBitcastSrcVectorSize(
+        Src.getOperand(1), Size, AllowTruncate, BitcastSrcVectorSizeMap);
+    BitcastSrcVectorSizeMap[Tp] = Ret1 && Ret2;
+    return Ret1 && Ret2;
+  }
   case ISD::SELECT:
-  case ISD::VSELECT:
-    return Src.getOperand(0).getScalarValueSizeInBits() == 1 &&
-           checkBitcastSrcVectorSize(Src.getOperand(1), Size, AllowTruncate) &&
-           checkBitcastSrcVectorSize(Src.getOperand(2), Size, AllowTruncate);
-  case ISD::BUILD_VECTOR:
-    return ISD::isBuildVectorAllZeros(Src.getNode()) ||
-           ISD::isBuildVectorAllOnes(Src.getNode());
+  case ISD::VSELECT: {
+    auto Ret1 = checkBitcastSrcVectorSize(
+        Src.getOperand(1), Size, AllowTruncate, BitcastSrcVectorSizeMap);
+    auto Ret2 = checkBitcastSrcVectorSize(
+        Src.getOperand(2), Size, AllowTruncate, BitcastSrcVectorSizeMap);
+    auto Ret3 = Src.getOperand(0).getScalarValueSizeInBits() == 1;
+    BitcastSrcVectorSizeMap[Tp] = Ret1 && Ret2 && Ret3;
+    return Ret1 && Ret2 && Ret3;
+  }
+  case ISD::BUILD_VECTOR: {
+    auto Ret = ISD::isBuildVectorAllZeros(Src.getNode()) ||
+               ISD::isBuildVectorAllOnes(Src.getNode());
+    BitcastSrcVectorSizeMap[Tp] = Ret;
+    return Ret;
   }
+  }
+  BitcastSrcVectorSizeMap[Tp] = false;
   return false;
 }
 
@@ -44925,6 +44954,7 @@ static SDValue combineBitcastvxi1(SelectionDAG &DAG, EVT VT, SDValue Src,
   // (v16i8 shuffle <0,2,4,6,8,10,12,14,u,u,...,u> (v16i8 bitcast t0), undef)
   MVT SExtVT;
   bool PropagateSExt = false;
+  std::map<std::tuple<SDValue, unsigned, bool>, bool> BitcastSrcVectorSizeMap;
   switch (SrcVT.getSimpleVT().SimpleTy) {
   default:
     return SDValue();
@@ -44936,7 +44966,8 @@ static SDValue combineBitcastvxi1(SelectionDAG &DAG, EVT VT, SDValue Src,
     // For cases such as (i4 bitcast (v4i1 setcc v4i64 v1, v2))
     // sign-extend to a 256-bit operation to avoid truncation.
     if (Subtarget.hasAVX() &&
-        checkBitcastSrcVectorSize(Src, 256, Subtarget.hasAVX2())) {
+        checkBitcastSrcVectorSize(Src, 256, Subtarget.hasAVX2(),
+                                  BitcastSrcVectorSizeMap)) {
       SExtVT = MVT::v4i64;
       PropagateSExt = true;
     }
@@ -44948,8 +44979,9 @@ static SDValue combineBitcastvxi1(SelectionDAG &DAG, EVT VT, SDValue Src,
     // If the setcc operand is 128-bit, prefer sign-extending to 128-bit over
     // 256-bit because the shuffle is cheaper than sign extending the result of
     // the compare.
-    if (Subtarget.hasAVX() && (checkBitcastSrcVectorSize(Src, 256, true) ||
-                               checkBitcastSrcVectorSize(Src, 512, true))) {
+    if (Subtarget.hasAVX() &&
+        (checkBitcastSrcVectorSize(Src, 256, true, BitcastSrcVectorSizeMap) ||
+         checkBitcastSrcVectorSize(Src, 512, true, BitcastSrcVectorSizeMap))) {
       SExtVT = MVT::v8i32;
       PropagateSExt = true;
     }
@@ -44974,7 +45006,7 @@ static SDValue combineBitcastvxi1(SelectionDAG &DAG, EVT VT, SDValue Src,
       break;
     }
     // Split if this is a <64 x i8> comparison result.
-    if (checkBitcastSrcVectorSize(Src, 512, false)) {
+    if (checkBitcastSrcVectorSize(Src, 512, false, BitcastSrcVectorSizeMap)) {
       SExtVT = MVT::v64i8;
       break;
     }

haonanya1 · 2025-03-07T03:25:40Z

@phoebewang, could you please review the pr? Thanks.

phoebewang · 2025-03-07T06:16:09Z

llvm/lib/Target/X86/X86ISelLowering.cpp

  switch (Src.getOpcode()) {
  case ISD::TRUNCATE:
-    if (!AllowTruncate)
+    if (!AllowTruncate) {
+      BitcastSrcVectorSizeMap[Tp] = false;


How about define a bool Result = false; to avoid repeat BitcastSrcVectorSizeMap[Tp] =?

Update pr to Early return when reach to MaxRecursionDepth. It's same as current other implementation, such as SimplifyMultipleUseDemandedBits, matchIndexRecursively and so on.

phoebewang · 2025-03-07T06:30:54Z

llvm/lib/Target/X86/X86ISelLowering.cpp

@@ -44925,6 +44954,7 @@ static SDValue combineBitcastvxi1(SelectionDAG &DAG, EVT VT, SDValue Src,
  // (v16i8 shuffle <0,2,4,6,8,10,12,14,u,u,...,u> (v16i8 bitcast t0), undef)
  MVT SExtVT;
  bool PropagateSExt = false;
+  std::map<std::tuple<SDValue, unsigned, bool>, bool> BitcastSrcVectorSizeMap;


Update pr to Early return when reach to MaxRecursionDepth. It's same as current other implementation, such as SimplifyMultipleUseDemandedBits, matchIndexRecursively and so on.

RKSimon

Would we be better off just adding a recursive depth limit (SelectionDAG::MaxRecursionDepth) like we do in other recursive calls?

phoebewang

LGTM.

RKSimon

LGTM (I've adjusted the patch title to explicitly mention checkBitcastSrcVectorSize)

github-actions · 2025-03-10T09:38:21Z

@haonanya1 Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

[X86] Fix duplicated compute in recursive search.

4574283

llvmbot added the backend:X86 label Mar 7, 2025

phoebewang requested review from RKSimon and phoebewang March 7, 2025 06:00

phoebewang reviewed Mar 7, 2025

View reviewed changes

RKSimon reviewed Mar 7, 2025

View reviewed changes

Add a recursive depth limit.

b5aeeee

haonanya1 changed the title ~~[X86] Fix duplicated compute in recursive search.~~ [X86] Early return when reach to MaxRecursionDepth. Mar 10, 2025

haonanya1 requested review from RKSimon and phoebewang March 10, 2025 03:00

phoebewang approved these changes Mar 10, 2025

View reviewed changes

RKSimon changed the title ~~[X86] Early return when reach to MaxRecursionDepth.~~ [X86] checkBitcastSrcVectorSize - early return when reach to MaxRecursionDepth. Mar 10, 2025

RKSimon approved these changes Mar 10, 2025

View reviewed changes

phoebewang merged commit 4a59d0c into llvm:main Mar 10, 2025
11 checks passed

haonanya1 deleted the fix-checkBitcastSrcVectorSize branch March 10, 2025 09:40

shiltian mentioned this pull request Mar 10, 2025

[AMDGPU] Fix test failures when expensive checks are enabled #130644

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[X86] checkBitcastSrcVectorSize - early return when reach to MaxRecursionDepth. #130226

[X86] checkBitcastSrcVectorSize - early return when reach to MaxRecursionDepth. #130226

Uh oh!

haonanya1 commented Mar 7, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Mar 7, 2025

Uh oh!

llvmbot commented Mar 7, 2025

Uh oh!

haonanya1 commented Mar 7, 2025

Uh oh!

phoebewang Mar 7, 2025

Uh oh!

haonanya1 Mar 10, 2025 •

edited

Loading

Uh oh!

phoebewang Mar 7, 2025

Uh oh!

haonanya1 Mar 10, 2025 •

edited

Loading

Uh oh!

RKSimon left a comment

Uh oh!

phoebewang left a comment

Uh oh!

RKSimon left a comment

Uh oh!

Uh oh!

github-actions bot commented Mar 10, 2025

Uh oh!

Uh oh!

[X86] checkBitcastSrcVectorSize - early return when reach to MaxRecursionDepth. #130226

[X86] checkBitcastSrcVectorSize - early return when reach to MaxRecursionDepth. #130226

Uh oh!

Conversation

haonanya1 commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 7, 2025

Uh oh!

llvmbot commented Mar 7, 2025

Uh oh!

haonanya1 commented Mar 7, 2025

Uh oh!

phoebewang Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

haonanya1 Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phoebewang Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

haonanya1 Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

phoebewang left a comment

Choose a reason for hiding this comment

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Mar 10, 2025

Uh oh!

Uh oh!

haonanya1 commented Mar 7, 2025 •

edited

Loading

haonanya1 Mar 10, 2025 •

edited

Loading

haonanya1 Mar 10, 2025 •

edited

Loading