[PATCH] D79391: [DSE] Remove noop stores in MSSA.
Alina Sbirlea via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 8 11:14:50 PDT 2020
asbirlea added inline comments.
================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1815
+ if (auto *Load = dyn_cast<LoadInst>(SI->getOperand(0))) {
+ if (Load->getPointerOperand() == SI->getOperand(1)) {
+ State.deleteDeadInstruction(SI);
----------------
fhahn wrote:
> I think the check is not strict enough at the moment. I think we have to ensure that there are no memory-writes between the load and the store that may alias them.
>
> In the simple example below, IIUC the patch would currently incorrectly eliminate ` store i32 %a, i32* %Q`.
>
> ```
> define void @test(i32* %Q) {
> %a = load i32, i32* %Q
> store i32 10, i32* %Q
> store i32 %a, i32* %Q
> ret void
> }```
>
> In terms of MemorySSA, I think we are looking for loads that use the same MemoryDef as `KillingDef`. One simple way to check this would be getting the memoryDef of KillingDef and check if it dominates the load I think.
>
+1.
Please add a testcase similar to the example fhahn@ gave.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79391/new/
https://reviews.llvm.org/D79391
More information about the llvm-commits
mailing list