[PATCH] D90328: Eliminates dead store of an exisiting value

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:2371
+    for (MemoryDef *Def : reverse(MemDefs)) {
+      if (SkipStores.find(Def) != SkipStores.end() ||
+          !isRemovable(Def->getMemoryInst()))
----------------
SkipStores.contains(Def)


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:2372
+      if (SkipStores.find(Def) != SkipStores.end() ||
+          !isRemovable(Def->getMemoryInst()))
+        continue;
----------------
This isRemovable checks is performed not on the store we'll try to remove. I think this is mostly okay because isIdenticalTo makes sure that if DefI is removable, so is UseI. The only part that doesn't directly follow is the `CB->use_empty()` part. Could it be that we first have a libcall with an unused return, and then a libcall with a used return, but otherwise identical?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90328