[PATCH] D136218: [DSE] Sink a memory access if it is only alive in one successor.

[luxufan via Phabricator via llvm-commits]

StephenFan added a comment.

In D136218#3881225 <https://reviews.llvm.org/D136218#3881225>, @pcwalton wrote:

> This is great! A couple of things I noticed on my test case:
>
> 1. This doesn't handle the case in which you might sink a store into a basic block that reads it (in my test case, it's a throwing instruction), because the check for readers will kick in and return `None` from `getDomMemoryDef()` before ever getting to check whether it can sink the store. I guess to fix this we could track reading instructions, instead of bailing out immediately, and see if they are all inside or dominated by `SuccToSinkTo`. If so, we should be able to safely sink the store, I think?
>
> 2. This doesn't handle the slow CFG search that kicks in if there are unreachable instructions. We could presumably detect sink targets there too.
>
> Both of these things could be followups of course. :)

Thanks a lot for testing it and making suggestions. I will try to solve these cases these days.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136218