Author: Arthur Eubanks Date: 2022-07-15T11:01:01-07:00 New Revision: 21b7e5248ffc423cd36c9d4a020085e363451465
URL: https://github.com/llvm/llvm-project/commit/21b7e5248ffc423cd36c9d4a020085e363451465 DIFF: https://github.com/llvm/llvm-project/commit/21b7e5248ffc423cd36c9d4a020085e363451465.diff LOG: [DSE] Revisit pointers that may no longer escape after removing another store In dependent-capture, previously we'd see that %tmp4 is captured due to the first store. We'd cache this info in CapturedBeforeReturn and InvisibleToCallerAfterRet. Then the first store is then removed, causing the cached values to be wrong. We also need to revisit everything because normally we work backwards when removing stores at the end of the function, but in this case removing an earlier store causes a later store to be removable. No compile time impact: https://llvm-compile-time-tracker.com/compare.php?from=32f3633171aa9d7352e9507c12d219efb48899a0&to=f1dc620e55635a69250d011c1a9950c45a364264&stat=instructions Reviewed By: asbirlea Differential Revision: https://reviews.llvm.org/D123686 Added: Modified: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp llvm/test/Transforms/DeadStoreElimination/dependent-capture.ll Removed: ################################################################################ diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp index 4c42869dbd581..9095ca7e4e7c1 100644 --- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp +++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp @@ -776,6 +776,11 @@ struct DSEState { // fall back to CFG scan starting from all non-unreachable roots. bool AnyUnreachableExit; + // Whether or not we should iterate on removing dead stores at the end of the + // function due to removing a store causing a previously captured pointer to + // no longer be captured. + bool ShouldIterateEndOfFunctionDSE; + // Class contains self-reference, make sure it's not copied/moved. DSEState(const DSEState &) = delete; DSEState &operator=(const DSEState &) = delete; @@ -1598,6 +1603,14 @@ struct DSEState { if (MemoryAccess *MA = MSSA.getMemoryAccess(DeadInst)) { if (MemoryDef *MD = dyn_cast<MemoryDef>(MA)) { SkipStores.insert(MD); + if (auto *SI = dyn_cast<StoreInst>(MD->getMemoryInst())) { + if (SI->getValueOperand()->getType()->isPointerTy()) { + const Value *UO = getUnderlyingObject(SI->getValueOperand()); + if (CapturedBeforeReturn.erase(UO)) + ShouldIterateEndOfFunctionDSE = true; + InvisibleToCallerAfterRet.erase(UO); + } + } } Updater.removeMemoryAccess(MA); @@ -1671,33 +1684,36 @@ struct DSEState { LLVM_DEBUG( dbgs() << "Trying to eliminate MemoryDefs at the end of the function\n"); - for (MemoryDef *Def : llvm::reverse(MemDefs)) { - if (SkipStores.contains(Def)) - continue; + do { + ShouldIterateEndOfFunctionDSE = false; + for (MemoryDef *Def : llvm::reverse(MemDefs)) { + if (SkipStores.contains(Def)) + continue; - Instruction *DefI = Def->getMemoryInst(); - auto DefLoc = getLocForWrite(DefI); - if (!DefLoc || !isRemovable(DefI)) - continue; + Instruction *DefI = Def->getMemoryInst(); + auto DefLoc = getLocForWrite(DefI); + if (!DefLoc || !isRemovable(DefI)) + continue; - // NOTE: Currently eliminating writes at the end of a function is limited - // to MemoryDefs with a single underlying object, to save compile-time. In - // practice it appears the case with multiple underlying objects is very - // uncommon. If it turns out to be important, we can use - // getUnderlyingObjects here instead. - const Value *UO = getUnderlyingObject(DefLoc->Ptr); - if (!isInvisibleToCallerAfterRet(UO)) - continue; + // NOTE: Currently eliminating writes at the end of a function is + // limited to MemoryDefs with a single underlying object, to save + // compile-time. In practice it appears the case with multiple + // underlying objects is very uncommon. If it turns out to be important, + // we can use getUnderlyingObjects here instead. + const Value *UO = getUnderlyingObject(DefLoc->Ptr); + if (!isInvisibleToCallerAfterRet(UO)) + continue; - if (isWriteAtEndOfFunction(Def)) { - // See through pointer-to-pointer bitcasts - LLVM_DEBUG(dbgs() << " ... MemoryDef is not accessed until the end " - "of the function\n"); - deleteDeadInstruction(DefI); - ++NumFastStores; - MadeChange = true; + if (isWriteAtEndOfFunction(Def)) { + // See through pointer-to-pointer bitcasts + LLVM_DEBUG(dbgs() << " ... MemoryDef is not accessed until the end " + "of the function\n"); + deleteDeadInstruction(DefI); + ++NumFastStores; + MadeChange = true; + } } - } + } while (ShouldIterateEndOfFunctionDSE); return MadeChange; } diff --git a/llvm/test/Transforms/DeadStoreElimination/dependent-capture.ll b/llvm/test/Transforms/DeadStoreElimination/dependent-capture.ll index 4973fadd38c0f..a7d171e5ce882 100644 --- a/llvm/test/Transforms/DeadStoreElimination/dependent-capture.ll +++ b/llvm/test/Transforms/DeadStoreElimination/dependent-capture.ll @@ -5,7 +5,6 @@ define void @f() { ; CHECK-LABEL: @f( ; CHECK-NEXT: [[TMP1:%.*]] = call noalias ptr @_Znwm() ; CHECK-NEXT: [[TMP4:%.*]] = call noalias ptr @_Znwm() -; CHECK-NEXT: store i8 0, ptr [[TMP4]], align 1 ; CHECK-NEXT: ret void ; %tmp1 = call noalias ptr @_Znwm() _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
