[LV] Don't ignore invariant stores when costing #158682
Merged
+18
−18
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Invariant stores of reductions are handled early in the VPlan construction, and there is no reason to ignore them while costing.
|
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-llvm-transforms Author: Ramkumar Ramachandra (artagnon) ChangesInvariant stores of reductions are handled early in the VPlan construction, and there is no reason to ignore them while costing. Full diff: https://github.com/llvm/llvm-project/pull/158682.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 640a98c622f80..91266328ca93d 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -6367,19 +6367,8 @@ void LoopVectorizationCostModel::collectValuesToIgnore() {
LoopBlocksDFS DFS(TheLoop);
DFS.perform(LI);
- MapVector<Value *, SmallVector<Value *>> DeadInvariantStoreOps;
for (BasicBlock *BB : reverse(make_range(DFS.beginRPO(), DFS.endRPO())))
for (Instruction &I : reverse(*BB)) {
- // Find all stores to invariant variables. Since they are going to sink
- // outside the loop we do not need calculate cost for them.
- StoreInst *SI;
- if ((SI = dyn_cast<StoreInst>(&I)) &&
- Legal->isInvariantAddressOfReduction(SI->getPointerOperand())) {
- ValuesToIgnore.insert(&I);
- DeadInvariantStoreOps[SI->getPointerOperand()].push_back(
- SI->getValueOperand());
- }
-
if (VecValuesToIgnore.contains(&I) || ValuesToIgnore.contains(&I))
continue;
@@ -6426,9 +6415,6 @@ void LoopVectorizationCostModel::collectValuesToIgnore() {
append_range(DeadInterleavePointerOps, Op->operands());
}
- for (const auto &[_, Ops] : DeadInvariantStoreOps)
- llvm::append_range(DeadOps, drop_end(Ops));
-
// Mark ops that would be trivially dead and are only used by ignored
// instructions as free.
BasicBlock *Header = TheLoop->getHeader();
diff --git a/llvm/test/Transforms/LoopVectorize/X86/invariant-store-vectorization.ll b/llvm/test/Transforms/LoopVectorize/X86/invariant-store-vectorization.ll
index 199f1c15fbc3d..2f74049968544 100644
--- a/llvm/test/Transforms/LoopVectorize/X86/invariant-store-vectorization.ll
+++ b/llvm/test/Transforms/LoopVectorize/X86/invariant-store-vectorization.ll
@@ -398,16 +398,30 @@ for.end: ; preds = %for.body
define void @test_store_of_final_reduction_value(i64 %x, ptr %dst) {
; CHECK-LABEL: define void @test_store_of_final_reduction_value(
; CHECK-SAME: i64 [[X:%.*]], ptr [[DST:%.*]]) #[[ATTR0]] {
-; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: br i1 false, label %[[SCALAR_PH:.*]], label %[[VECTOR_PH:.*]]
+; CHECK: [[VECTOR_PH]]:
+; CHECK-NEXT: [[BROADCAST_SPLATINSERT:%.*]] = insertelement <2 x i64> poison, i64 [[X]], i64 0
+; CHECK-NEXT: [[BROADCAST_SPLAT:%.*]] = shufflevector <2 x i64> [[BROADCAST_SPLATINSERT]], <2 x i64> poison, <2 x i32> zeroinitializer
+; CHECK-NEXT: br label %[[VECTOR_BODY:.*]]
+; CHECK: [[VECTOR_BODY]]:
+; CHECK-NEXT: [[VEC_PHI:%.*]] = phi <2 x i64> [ <i64 0, i64 1>, %[[VECTOR_PH]] ], [ [[TMP0:%.*]], %[[VECTOR_BODY]] ]
+; CHECK-NEXT: [[TMP0]] = mul <2 x i64> [[VEC_PHI]], [[BROADCAST_SPLAT]]
+; CHECK-NEXT: br i1 true, label %[[MIDDLE_BLOCK:.*]], label %[[VECTOR_BODY]], !llvm.loop [[LOOP32:![0-9]+]]
+; CHECK: [[MIDDLE_BLOCK]]:
+; CHECK-NEXT: [[TMP1:%.*]] = call i64 @llvm.vector.reduce.mul.v2i64(<2 x i64> [[TMP0]])
+; CHECK-NEXT: store i64 [[TMP1]], ptr [[DST]], align 8
+; CHECK-NEXT: br label %[[EXIT:.*]]
+; CHECK: [[SCALAR_PH]]:
; CHECK-NEXT: br label %[[LOOP:.*]]
; CHECK: [[LOOP]]:
-; CHECK-NEXT: [[IV4:%.*]] = phi i64 [ 0, %[[ENTRY]] ], [ [[IV_NEXT:%.*]], %[[LOOP]] ]
-; CHECK-NEXT: [[RED:%.*]] = phi i64 [ 0, %[[ENTRY]] ], [ [[RED_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[IV4:%.*]] = phi i64 [ 0, %[[SCALAR_PH]] ], [ [[IV_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[RED:%.*]] = phi i64 [ 0, %[[SCALAR_PH]] ], [ [[RED_NEXT:%.*]], %[[LOOP]] ]
; CHECK-NEXT: [[RED_NEXT]] = mul i64 [[RED]], [[X]]
; CHECK-NEXT: store i64 [[RED_NEXT]], ptr [[DST]], align 8
; CHECK-NEXT: [[IV_NEXT]] = add i64 [[IV4]], 1
; CHECK-NEXT: [[EC:%.*]] = icmp eq i64 [[IV4]], 1
-; CHECK-NEXT: br i1 [[EC]], label %[[EXIT:.*]], label %[[LOOP]]
+; CHECK-NEXT: br i1 [[EC]], label %[[EXIT]], label %[[LOOP]], !llvm.loop [[LOOP33:![0-9]+]]
; CHECK: [[EXIT]]:
; CHECK-NEXT: ret void
;
|
fhahn
reviewed
Sep 15, 2025
Yes, I think so. |
fhahn
approved these changes
Sep 24, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/36235 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/137/builds/26087 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/175/builds/25932 Here is the relevant piece of the build log for the reference |
|
Looks like this needed a rebase before merging. Woul be good to update to latest main before merging and see if everything still passes |
Sorry, my bad! Currently building opt to fixup test. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/185/builds/25930 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/33/builds/23614 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Invariant stores of reductions are removed early in the VPlan construction, and there is no reason to ignore them while costing.