From 3b0878a370055f2a8a6f3c0f05f00d33589c20cf Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Thu, 13 Aug 2020 11:56:40 +0100 Subject: [PATCH] [DSE,MSSA] Fix crash when using tryToMergePartialOverlappingStores. We are re-using tryToMergePartialOverlappingStores, which requires earlier to domiante Later. In the long run, tryToMergeParialOverlappingStores should be re-written using MemorySSA. Fixes PR46513. --- .../Scalar/DeadStoreElimination.cpp | 33 +++++++++++-------- .../MSSA/multiblock-overlap.ll | 30 +++++++++++++++++ 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp index 048b67a9f3b7..261fb3878ac1 100644 --- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp +++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp @@ -2249,22 +2249,27 @@ bool eliminateDeadStoresMemorySSA(Function &F, AliasAnalysis &AA, if (EnablePartialStoreMerging && OR == OW_PartialEarlierWithFullLater) { auto *Earlier = dyn_cast(NI); auto *Later = dyn_cast(SI); - if (Constant *Merged = tryToMergePartialOverlappingStores( - Earlier, Later, InstWriteOffset, DepWriteOffset, DL, &AA, - &DT)) { + // We are re-using tryToMergePartialOverlappingStores, which requires + // Earlier to domiante Later. + // TODO: implement tryToMergeParialOverlappingStores using MemorySSA. + if (Earlier && Later && DT.dominates(Earlier, Later)) { + if (Constant *Merged = tryToMergePartialOverlappingStores( + Earlier, Later, InstWriteOffset, DepWriteOffset, DL, &AA, + &DT)) { - // Update stored value of earlier store to merged constant. - Earlier->setOperand(0, Merged); - ++NumModifiedStores; - MadeChange = true; + // Update stored value of earlier store to merged constant. + Earlier->setOperand(0, Merged); + ++NumModifiedStores; + MadeChange = true; - // Remove later store and remove any outstanding overlap intervals - // for the updated store. - State.deleteDeadInstruction(Later); - auto I = State.IOLs.find(Earlier->getParent()); - if (I != State.IOLs.end()) - I->second.erase(Earlier); - break; + // Remove later store and remove any outstanding overlap intervals + // for the updated store. + State.deleteDeadInstruction(Later); + auto I = State.IOLs.find(Earlier->getParent()); + if (I != State.IOLs.end()) + I->second.erase(Earlier); + break; + } } } diff --git a/llvm/test/Transforms/DeadStoreElimination/MSSA/multiblock-overlap.ll b/llvm/test/Transforms/DeadStoreElimination/MSSA/multiblock-overlap.ll index 5c26566e25a5..579ec8e268fc 100644 --- a/llvm/test/Transforms/DeadStoreElimination/MSSA/multiblock-overlap.ll +++ b/llvm/test/Transforms/DeadStoreElimination/MSSA/multiblock-overlap.ll @@ -110,3 +110,33 @@ bb9: ; preds = %bb8, %bb7 store double 6.0, double* %tmp3, align 8 ret void } + +; Test case from PR46513. Make sure we do not crash. +; TODO: we should be able to shorten store i32 844283136, i32* %cast.i32 to a +; store of i16. +define void @overlap_no_dominance([4 x i8]* %arg, i1 %c) { +; CHECK-LABEL: @overlap_no_dominance( +; CHECK-NEXT: bb: +; CHECK-NEXT: br i1 [[C:%.*]], label [[BB13:%.*]], label [[BB9:%.*]] +; CHECK: bb9: +; CHECK-NEXT: [[CAST_I32:%.*]] = bitcast [4 x i8]* [[ARG:%.*]] to i32* +; CHECK-NEXT: store i32 844283136, i32* [[CAST_I32]], align 4 +; CHECK-NEXT: br label [[BB13]] +; CHECK: bb13: +; CHECK-NEXT: [[CAST_I16:%.*]] = bitcast [4 x i8]* [[ARG]] to i16* +; CHECK-NEXT: store i16 0, i16* [[CAST_I16]], align 4 +; CHECK-NEXT: ret void +; +bb: + br i1 %c, label %bb13, label %bb9 + +bb9: ; preds = %bb + %cast.i32 = bitcast [4 x i8]* %arg to i32* + store i32 844283136, i32* %cast.i32, align 4 + br label %bb13 + +bb13: ; preds = %bb9, %bb + %cast.i16 = bitcast [4 x i8]* %arg to i16* + store i16 0, i16* %cast.i16, align 4 + ret void +}