Skip to content

[X86] LowerStore - always split 512-bit concatenated stores #143426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 10, 2025

Conversation

RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented Jun 9, 2025

All BWI regressions have now been addressed, so remove the special case handling.

All BWI regressions have now been addressed, so remove the special case handling.
@llvmbot
Copy link
Member

llvmbot commented Jun 9, 2025

@llvm/pr-subscribers-backend-x86

Author: Simon Pilgrim (RKSimon)

Changes

All BWI regressions have now been addressed, so remove the special case handling.


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

1 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+5-7)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 9ea45513cc019..0cae8dbcc3bc7 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -25549,14 +25549,12 @@ static SDValue LowerStore(SDValue Op, const X86Subtarget &Subtarget,
   if (St->isTruncatingStore())
     return SDValue();
 
-  // If this is a 256-bit store of concatenated ops, we are better off splitting
-  // that store into two 128-bit stores. This avoids spurious use of 256-bit ops
-  // and each half can execute independently. Some cores would split the op into
-  // halves anyway, so the concat (vinsertf128) is purely an extra op.
+  // If this is a 256/512-bit store of concatenated ops, we are better off
+  // splitting that store into two half-size stores. This avoids spurious use of
+  // concatenated ops and each half can execute independently. Some cores would
+  // split the op into halves anyway, so the concat is purely an extra op.
   MVT StoreVT = StoredVal.getSimpleValueType();
-  if (StoreVT.is256BitVector() ||
-      ((StoreVT == MVT::v32i16 || StoreVT == MVT::v64i8) &&
-       !Subtarget.hasBWI())) {
+  if (StoreVT.is256BitVector() || StoreVT.is512BitVector()) {
     if (StoredVal.hasOneUse() && isFreeToSplitVector(StoredVal, DAG))
       return splitVectorStore(St, DAG);
     return SDValue();

@RKSimon RKSimon merged commit 530f5b7 into llvm:main Jun 10, 2025
9 checks passed
@RKSimon RKSimon deleted the x86-split-store-512 branch June 10, 2025 06:33
rorth pushed a commit to rorth/llvm-project that referenced this pull request Jun 11, 2025
)

All BWI regressions have now been addressed, so remove the special case
handling.
DhruvSrivastavaX pushed a commit to DhruvSrivastavaX/lldb-for-aix that referenced this pull request Jun 12, 2025
)

All BWI regressions have now been addressed, so remove the special case
handling.
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
)

All BWI regressions have now been addressed, so remove the special case
handling.
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