[PATCH] D53203: Allow MemoryLocation to carry pre-existing knowledge to AA to elide expensive repeated checks

Daniel Sanders via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 2 09:08:59 PDT 2018


dsanders added a comment.

In https://reviews.llvm.org/D53203#1285430, @hfinkel wrote:

> > I'm not very familiar with the MemorySSA pass but based on a fairly quick skim of it ...
> >  It looks like the MemoryDef/MemoryUse objects are attached to the BB rather than the instructions.
>
> I'll write more later, but quickly, this is incorrect. MemorySSA has per-instruction granularity.


Thanks. I got that from some comments about attaching MemoryUse/MemoryDef to basic blocks (not the ones about phi's) but I'm having trouble finding the same comments this morning. Is it storing the additional data in the BB objects or something like that? That could explain how I reached the wrong conclusion.

In that case it will come down to how conservative it is about attaching MemoryUse. Looking at buildMemorySSA() again, createNewAccess() is calling getModRefInfo(const Instruction *, Optional<MemoryLocation>) and is always providing None as the second argument. Following that code path, this is causing it to report the behaviour using getModRefBehavior(ImmutableCallSite) which is good news performance-wise since this path doesn't have the expensive is-this-a-non-escaping-local check. It's also good news for DSE when doesNotAccessMemory() is true since that avoids the need for the expensive call for all allocas on the basis that none of them can be accessed if it doesn't access memory. However, it's not enough for DSE when doesNotAccessMemory() is false since DSE wants to know if specific allocas are accessed by the call site so we'd still have to iterate over all the possibly-dead allocas checking each one with getModRefInfo(ImmutableCallSite *, MemoryLocation) which (without something like this patch) would repeatedly perform the expensive check to see if the MemoryLocation is a non-escaping local.


Repository:
  rL LLVM

https://reviews.llvm.org/D53203





More information about the llvm-commits mailing list