[PATCH] D98288: [DSE] Translate killing locations through phis.

[Daniil Seredkin via Phabricator via llvm-commits]

vdsered added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1999
+            if (Addr.NeedsPHITranslationFromBlock(PhiBlock) &&
+                Predecessors.count(IncomingBlock)) {
+              if (Addr.IsPotentiallyPHITranslatable() &&
----------------
vdsered wrote:
> Why do we need to check if `IncomingBlock` is in a set of predecessors of `PhiBlock`? Isn't it always true? I thought there could probably be a reason like `PhiBlock` points to itself, but it seems that I'm wrong because we shouldn't get here in that case as we forbid it by the if-statement
> ```
> if (State.PostOrderNumbers[IncomingBlock] >
>               State.PostOrderNumbers[PhiBlock])
> ```
> don't we?
*A little correction*
I meant that there could be a reason to check this if PhiBlock == IncomingBlock


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98288