[PATCH] D111727: [DSE] Eliminates redundant store of an exisiting value (PR16520)

[Dawid Jurczak via Phabricator via llvm-commits]

yurai007 added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1962
+      if (!MaybeUpperLoc || !DefInst->isIdenticalTo(UpperInst) ||
+          isReadClobber(*MaybeUpperLoc, DefInst))
+        continue;
----------------
nikic wrote:
> Actually, why do we need this isReadClobber() check? I don't think we care whether the instruction also reads the value, removing the redundant store is still fine. This would allow us to catch the `@pr50339` case as well.
It's redundant and can be removed. I thought it was needed to ensure correctness for cases with intermediate clobbering (like tests3/test4). I will get rid of check and adjust tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111727/new/

https://reviews.llvm.org/D111727