[PATCH] D72146: [DSE] Add first version of MemorySSA-backed DSE.

Tyker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 4 02:41:07 PST 2020


Tyker added a comment.

In D72146#1803859 <https://reviews.llvm.org/D72146#1803859>, @fhahn wrote:

> Do you think it would make sense to use D72146 <https://reviews.llvm.org/D72146>  (this patch) as foundation and port the improvements from your patch? It would be great if you could have a look and let me know what you think!


I took a look at this patch and D72148 <https://reviews.llvm.org/D72148>(the patch adding MemPhi traversal). they have many good ideas i haven't thought about.
but i have one major concern.
how to you plan to dealing with store being killed by multiple stores which together post-dominate the "dead" store ?
like the store in the entry block of the following:

  define void @test5A(i32* %P) {
    store i32 1, i32* %P ; this is dead
    br i1 true, label %bb1, label %bb2
  bb1:
    store i32 0, i32* %P
    br label %bb3
  bb2:
    store i32 1, i32* %P
    br label %bb3
  bb3:
    ret void
  }

the design around `getNextMemoryDef` seems to me like it is too restrictive for the above example.

aside from that we went for different strategies for storing pass related data structures but i am not locked on my decision of having a `DSEContext` storing "everything".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72146





More information about the llvm-commits mailing list