[PATCH] D73763: [DSE] Lift post-dominance restriction.
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 1 12:22:33 PDT 2020
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
More information about the llvm-commits
mailing list