[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

In D87163#2275058 <https://reviews.llvm.org/D87163#2275058>, @asbirlea wrote:

> Yes, the load should have the Phi as the defining access. I'm still looking into where this information should come from, but it's not hitting the phi translation.
> Thank you for the revert, I'll update with the fix once I have it available.

Thanks! I also started looking into where things go wrong. It appears that this happens during MemorySSA use optimization. (https://github.com/llvm/llvm-project/blob/master/llvm/lib/Analysis/MemorySSA.cpp#L1447). It seems like `getClobberingMemoryAccess` does not account for the fact that the query location could map to multiple memory locations if it is loop dependent. Just querying AA for both pointers will return `noalias`, because they only alias for different concrete phi values. I am not sure if we have a safe way to detect that a location actually refers to a range of locations. I am currently experimenting with being more conservative in `getClobberingMemoryAccess` for locations for which we cannot prove that they are loop-invariant for possible cycle in the function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163