Skip to content

[VectorCombine] scalarizeLoadExtract - don't create scalar loads if any extract is waiting to be erased #129375

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 3 commits into from
Mar 1, 2025

Conversation

RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented Mar 1, 2025

If any extract is waiting to be erased, then bail out as this will distort the cost calculation and possibly lead to infinite loops.

Fixes #129373

…ny extract is waiting to be erased

If any extract is waiting to be erased, then bail out as this will distort the cost calculation and possibly lead to infinite loops.

Fixes llvm#129373
@llvmbot
Copy link
Member

llvmbot commented Mar 1, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Simon Pilgrim (RKSimon)

Changes

If any extract is waiting to be erased, then bail out as this will distort the cost calculation and possibly lead to infinite loops.

Fixes #129373


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VectorCombine.cpp (+5)
  • (modified) llvm/test/Transforms/VectorCombine/X86/load-extractelement-scalarization.ll (+12)
diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index bb4e1c0f67e79..84cd08cd690d2 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -1579,6 +1579,11 @@ bool VectorCombine::scalarizeLoadExtract(Instruction &I) {
     if (!UI || UI->getParent() != LI->getParent())
       return false;
 
+    // If any extract is waiting to be erased, then bail out as this will
+    // distort the cost calculation and possibly lead to infinite loops.
+    if (isInstructionTriviallyDead(UI))
+      return false;
+
     // Check if any instruction between the load and the extract may modify
     // memory.
     if (LastCheckedInst->comesBefore(UI)) {
diff --git a/llvm/test/Transforms/VectorCombine/X86/load-extractelement-scalarization.ll b/llvm/test/Transforms/VectorCombine/X86/load-extractelement-scalarization.ll
index 0acfeccb92ef7..4e95950591cbc 100644
--- a/llvm/test/Transforms/VectorCombine/X86/load-extractelement-scalarization.ll
+++ b/llvm/test/Transforms/VectorCombine/X86/load-extractelement-scalarization.ll
@@ -24,3 +24,15 @@ define void @multiple_extract(ptr %p) {
   store i32 %e1, ptr %p1, align 4
   ret void
 }
+
+; infinite loop if we fold an extract that is waiting to be erased
+define void @unsued_extract(ptr %p) {
+; CHECK-LABEL: @unsued_extract(
+; CHECK-NEXT:    ret void
+;
+  %load = load <4 x float>, ptr %p, align 8
+  %shuffle0 = shufflevector <4 x float> zeroinitializer, <4 x float> %load, <4 x i32> <i32 0, i32 4, i32 1, i32 5>
+  %shuffle1 = shufflevector <4 x float> %shuffle0, <4 x float> zeroinitializer, <4 x i32> <i32 0, i32 4, i32 poison, i32 poison>
+  %extract = extractelement <4 x float> %load, i64 1
+  ret void
+}

@llvmbot
Copy link
Member

llvmbot commented Mar 1, 2025

@llvm/pr-subscribers-vectorizers

Author: Simon Pilgrim (RKSimon)

Changes

If any extract is waiting to be erased, then bail out as this will distort the cost calculation and possibly lead to infinite loops.

Fixes #129373


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VectorCombine.cpp (+5)
  • (modified) llvm/test/Transforms/VectorCombine/X86/load-extractelement-scalarization.ll (+12)
diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index bb4e1c0f67e79..84cd08cd690d2 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -1579,6 +1579,11 @@ bool VectorCombine::scalarizeLoadExtract(Instruction &I) {
     if (!UI || UI->getParent() != LI->getParent())
       return false;
 
+    // If any extract is waiting to be erased, then bail out as this will
+    // distort the cost calculation and possibly lead to infinite loops.
+    if (isInstructionTriviallyDead(UI))
+      return false;
+
     // Check if any instruction between the load and the extract may modify
     // memory.
     if (LastCheckedInst->comesBefore(UI)) {
diff --git a/llvm/test/Transforms/VectorCombine/X86/load-extractelement-scalarization.ll b/llvm/test/Transforms/VectorCombine/X86/load-extractelement-scalarization.ll
index 0acfeccb92ef7..4e95950591cbc 100644
--- a/llvm/test/Transforms/VectorCombine/X86/load-extractelement-scalarization.ll
+++ b/llvm/test/Transforms/VectorCombine/X86/load-extractelement-scalarization.ll
@@ -24,3 +24,15 @@ define void @multiple_extract(ptr %p) {
   store i32 %e1, ptr %p1, align 4
   ret void
 }
+
+; infinite loop if we fold an extract that is waiting to be erased
+define void @unsued_extract(ptr %p) {
+; CHECK-LABEL: @unsued_extract(
+; CHECK-NEXT:    ret void
+;
+  %load = load <4 x float>, ptr %p, align 8
+  %shuffle0 = shufflevector <4 x float> zeroinitializer, <4 x float> %load, <4 x i32> <i32 0, i32 4, i32 1, i32 5>
+  %shuffle1 = shufflevector <4 x float> %shuffle0, <4 x float> zeroinitializer, <4 x i32> <i32 0, i32 4, i32 poison, i32 poison>
+  %extract = extractelement <4 x float> %load, i64 1
+  ret void
+}

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

RKSimon added 2 commits March 1, 2025 12:36
…ructionTriviallyDead is only needed for more complex general cases
@RKSimon RKSimon merged commit 5ddf40f into llvm:main Mar 1, 2025
11 checks passed
@RKSimon RKSimon deleted the vectorcombine-extract-load-unused branch March 1, 2025 16:54
swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Mar 12, 2025
…ny extract is waiting to be erased (llvm#129375)

If any extract is waiting to be erased, then bail out as this will distort the cost calculation and possibly lead to infinite loops.

Fixes llvm#129373

(cherry picked from commit 5ddf40f)
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
…ny extract is waiting to be erased (llvm#129375)

If any extract is waiting to be erased, then bail out as this will distort the cost calculation and possibly lead to infinite loops.

Fixes llvm#129373
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.

[VectorCombine] scalarizeLoadExtract - infinite loop if we scalarize a dead extract waiting to be erased
3 participants