[PATCH] D72148: [DSE] Support traversing MemoryPhis.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D72148#2239379 <https://reviews.llvm.org/D72148#2239379>, @ebrevnov wrote:

> @fhahn  is this patch supposed to support the following case?
>
> define i8* @foo(i1 %cond, i8* %p1, i8* %p2) {
> entry:
>
>   %0 = icmp ne i1 %cond, 0
>   br i1 %0, label %dead_store, label %merge
>
> dead_store:
>
>   store i8 1, i8* %p1
>   br label %merge
>
> merge:
>
>   %p3 = phi i8* [ %p1, %dead_store ], [ %p2, %entry ]
>   store i8 2, i8* %p3
>   ret i8* %p3
>
> }
>
> If not, is there any plans or prototype which can handle it? Thanks.

Currently we do not phi-translate the location of the 'killing def' and hence do not eliminate the store in this case. I haven't thought too much about it so far, but I think doing the phi translation of the 'killing loc' should not be too hard. It might be as simple as adjusting the `ToCheck` set to contain a pair `(Access to check, Phi-translated DefLoc)` and do phi translation when we queue the incoming defs for MemoryPhis (https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp#L2284)

I won't have time to look into this in the near future unfortunately, as I am mainly focused on getting things in good shape to enable DSE & MemorySSA by default. It might be good to file a bug report so this opportunity does not drop off our radar. If anyone is interested in giving implementing it a try, even better :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72148