[PATCH] D72700: [DSE] Add first version of MemorySSA-backed DSE (Bottom up walk).

Wed Jan 15 05:54:19 PST 2020

fhahn added a comment.

In D72700#1821571 <https://reviews.llvm.org/D72700#1821571>, @Tyker wrote:

> I did a few experiment with bottom-up algorithm before the patch i showed on phabricator. my implementation of the bottom-up had similar average complie-time to the current pass on the test-suite but it was only barely removing more stores than the current pass.
>  i gave up on it because i didn't found a good way forward to make it deal with cases like to the following:
>
>      store i32 0, i32* %Ptr.  ; DEAD
>      br i1 %cond, label %a, label %b
>   a:
>     store i32 2, i32* %Ptr
>      br label %c
>   b: 
>     store i32 2, i32* %Ptr
>     br label %c
>
>
> which is IMO definitely something we want to do. by the way gcc does this optimization. https://godbolt.org/z/VFBf-c
>  maybe there is a bottom-up way to deal with this that i didn't thought about. any thought ?

Yes I think we should make sure to cover this case. One approach could be to optimize the defining accesses of the accesses we start our bottom-up walks with. After visiting all MemoryDefs, to detect cases like the one above, we would just have to look for MemoryDefs `D` with multiple MemoryDefs as users (which we could probably collect along the way). Then we would just have to check that `D` is not read in between. Does that sound sensible?

> would you be interested in a port of my top-down algorithm from D72182 <https://reviews.llvm.org/D72182> to work with this patch series's framework ?
>  i have gotten since i wrote it many idea of how to improve it mostly on the compile-time side.

Yes, but I think it would be good to settle the question bottom-up vs top-down.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72700