Skip to content

Conversation

@RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented Feb 12, 2025

Its not currently used, but is likely to just introduce additional shuffles, resulting in higher Port5 pressure etc. in future patches.

…s across binops to fold the load.

This is likely to just introduce additional shuffles, resulting in higher Port5 pressure etc.
@llvmbot
Copy link
Member

llvmbot commented Feb 12, 2025

@llvm/pr-subscribers-backend-x86

Author: Simon Pilgrim (RKSimon)

Changes

Its not currently used, but is likely to just introduce additional shuffles, resulting in higher Port5 pressure etc. in future patches.


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

1 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+3-6)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 134a5617ee768..77c426f214675 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -41579,8 +41579,7 @@ static SDValue canonicalizeShuffleWithOp(SDValue N, SelectionDAG &DAG,
   EVT ShuffleVT = N.getValueType();
   unsigned Opc = N.getOpcode();
 
-  auto IsMergeableWithShuffle = [Opc, &DAG](SDValue Op, bool FoldShuf = true,
-                                            bool FoldLoad = false) {
+  auto IsMergeableWithShuffle = [Opc, &DAG](SDValue Op, bool FoldShuf = true) {
     // AllZeros/AllOnes constants are freely shuffled and will peek through
     // bitcasts. Other constant build vectors do not peek through bitcasts. Only
     // merge with target shuffles if it has one use so shuffle combining is
@@ -41593,7 +41592,6 @@ static SDValue canonicalizeShuffleWithOp(SDValue N, SelectionDAG &DAG,
            (Op.getOpcode() == Opc && Op->hasOneUse()) ||
            (Op.getOpcode() == ISD::INSERT_SUBVECTOR && Op->hasOneUse()) ||
            (FoldShuf && isTargetShuffle(Op.getOpcode()) && Op->hasOneUse()) ||
-           (FoldLoad && isShuffleFoldableLoad(Op)) ||
            DAG.isSplatValue(Op, /*AllowUndefs*/ false);
   };
   auto IsSafeToMoveShuffle = [ShuffleVT](SDValue Op, unsigned BinOp) {
@@ -41629,9 +41627,8 @@ static SDValue canonicalizeShuffleWithOp(SDValue N, SelectionDAG &DAG,
         SDValue Op00 = peekThroughOneUseBitcasts(N0.getOperand(0));
         SDValue Op01 = peekThroughOneUseBitcasts(N0.getOperand(1));
         bool FoldShuf = Opc != X86ISD::VPERMI;
-        bool FoldLoad = Opc != X86ISD::PSHUFB;
-        if (IsMergeableWithShuffle(Op00, FoldShuf, FoldLoad) ||
-            IsMergeableWithShuffle(Op01, FoldShuf, FoldLoad)) {
+        if (IsMergeableWithShuffle(Op00, FoldShuf) ||
+            IsMergeableWithShuffle(Op01, FoldShuf)) {
           SDValue LHS, RHS;
           Op00 = DAG.getBitcast(ShuffleVT, Op00);
           Op01 = DAG.getBitcast(ShuffleVT, Op01);

@RKSimon RKSimon merged commit 085bdb1 into llvm:main Feb 12, 2025
8 of 10 checks passed
@RKSimon RKSimon deleted the x86-no-merge-shuffle-loads branch February 12, 2025 12:13
flovent pushed a commit to flovent/llvm-project that referenced this pull request Feb 13, 2025
…s across binops to fold the load. (llvm#126894)

Its not currently used, but is likely to just introduce additional shuffles, resulting in higher Port5 pressure etc. in future patches.
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Feb 14, 2025
…s across binops to fold the load. (llvm#126894)

Its not currently used, but is likely to just introduce additional shuffles, resulting in higher Port5 pressure etc. in future patches.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Feb 24, 2025
…s across binops to fold the load. (llvm#126894)

Its not currently used, but is likely to just introduce additional shuffles, resulting in higher Port5 pressure etc. in future patches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants