diff options
author | Marianne Mailhot-Sarrasin <marianne.ms.qc@gmail.com> | 2021-12-22 16:11:23 -0500 |
---|---|---|
committer | Marianne Mailhot-Sarrasin <marianne.ms.qc@gmail.com> | 2021-12-22 16:11:23 -0500 |
commit | 90d1786ba0c233456b7785fe4b93eca835d56028 (patch) | |
tree | 1bf771c7bbf8a94ea182290ac722986e454a3f11 | |
parent | 32e8b30d6e67352040ffef54fb0e4b3a69242a4a (diff) | |
download | llvm-90d1786ba0c233456b7785fe4b93eca835d56028.zip llvm-90d1786ba0c233456b7785fe4b93eca835d56028.tar.gz llvm-90d1786ba0c233456b7785fe4b93eca835d56028.tar.bz2 |
[DSE] Fix invalid removal of store instruction
Fix handling of alloc-like instructions in isGuaranteedLoopInvariant(). It was not valid when the 'KillingDef' was outside of the loop, while the 'CurrentDef' was inside the loop. In that case, the 'KillingDef' only overwrites the definition from the last iteration of the loop, and not the ones of all iterations. Therefor it does not make the 'CurrentDef' to be dead, and must not remove it.
Fixing issue : https://github.com/llvm/llvm-project/issues/52774
Reviewed by: Florian Hahn
Differential revision: https://reviews.llvm.org/D115965
-rw-r--r-- | llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp | 13 | ||||
-rw-r--r-- | llvm/test/Transforms/DeadStoreElimination/store-after-loop.ll | 3 |
2 files changed, 5 insertions, 11 deletions
diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp index ee6c800..4c17e2c 100644 --- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp +++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp @@ -1209,17 +1209,10 @@ struct DSEState { /// loop. In particular, this guarantees that it only references a single /// MemoryLocation during execution of the containing function. bool isGuaranteedLoopInvariant(const Value *Ptr) { - auto IsGuaranteedLoopInvariantBase = [this](const Value *Ptr) { + auto IsGuaranteedLoopInvariantBase = [](const Value *Ptr) { Ptr = Ptr->stripPointerCasts(); - if (auto *I = dyn_cast<Instruction>(Ptr)) { - if (isa<AllocaInst>(Ptr)) - return true; - - if (isAllocLikeFn(I, &TLI)) - return true; - - return false; - } + if (auto *I = dyn_cast<Instruction>(Ptr)) + return I->getParent()->isEntryBlock(); return true; }; diff --git a/llvm/test/Transforms/DeadStoreElimination/store-after-loop.ll b/llvm/test/Transforms/DeadStoreElimination/store-after-loop.ll index 27c7c13..1e98fd7 100644 --- a/llvm/test/Transforms/DeadStoreElimination/store-after-loop.ll +++ b/llvm/test/Transforms/DeadStoreElimination/store-after-loop.ll @@ -7,7 +7,6 @@ target datalayout = "E-m:e-p:32:32-i64:32-f64:32:64-a:0:32-n32-S32" ; There is no dead store in this test. Make sure no store is deleted by DSE. ; Test case related to bug report PR52774. -; NOTE: Showing actual (broken) DSE behavior. define %struct.ilist* @test() { ; CHECK-LABEL: @test( @@ -19,6 +18,8 @@ define %struct.ilist* @test() { ; CHECK-NEXT: [[LIST_NEW_ILIST_PTR]] = bitcast i8* [[LIST_NEW_I8_PTR]] to %struct.ilist* ; CHECK-NEXT: [[GEP_NEW_VALUE:%.*]] = getelementptr inbounds [[STRUCT_ILIST:%.*]], %struct.ilist* [[LIST_NEW_ILIST_PTR]], i32 0, i32 0 ; CHECK-NEXT: store i32 42, i32* [[GEP_NEW_VALUE]], align 8 +; CHECK-NEXT: [[GEP_NEW_NEXT:%.*]] = getelementptr inbounds [[STRUCT_ILIST]], %struct.ilist* [[LIST_NEW_ILIST_PTR]], i32 0, i32 1 +; CHECK-NEXT: store %struct.ilist* [[LIST_NEXT]], %struct.ilist** [[GEP_NEW_NEXT]], align 4 ; CHECK-NEXT: [[IV_NEXT]] = add nuw nsw i32 [[IV]], 1 ; CHECK-NEXT: [[COND:%.*]] = icmp eq i32 [[IV_NEXT]], 10 ; CHECK-NEXT: br i1 [[COND]], label [[EXIT:%.*]], label [[LOOP]] |