[PATCH] D73763: [DSE] Lift post-dominance restriction.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1682
       for (Use &U : Acc->uses())
         WorkList.insert(cast<MemoryAccess>(U.getUser()));
     };
----------------
george.burgess.iv wrote:
> tangential to this patch: should we be ignoring `U`s which are `MemoryDefs` whose only Use of `Acc` is as their `getOptimized()` operand? Without that, it's not immediately obvious to me how we aren't overly conservative in situations like:
> 
> ```
> {
>   // 1 = MemoryDef(LOE) -- this is DomAccess
>   *a = 1;
>   // 2 = MemoryDef(1)
>   *b = 2;
>   // 3 = MemoryDef(2) -- this is the obvious kill
>   *a = 3;
>   // 4 = MemoryDef(3) (optimized to 2)
>   *c = 4;
>   // MemoryUse(4) -- AA says MayAlias for `(d, a)`, so `isReadClobber()` should be `true` if we visit this?
>   if (*d) {
>     // ...
>   }
> }
> ```
Oh, the optimized access is also added to the uses of `2 = MemoryDef` here? Yes that would be overly conservative. Is there a way to force MemorySSA to be optimized before we run DSE, for testing? I'll put up a separate fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73763