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

[Patrick Walton via Phabricator via llvm-commits]

pcwalton added a comment.

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. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136218