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

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

Hal,

I have mixed feelings here because I understand Chris's point, but there are differences in this case which make this patch worth pushing forward IMO.

MemorySSA in the current format will not solve this. It can be (and should be) extended to handle cases with *some* similarity (because we have other use cases for it). Trying to explain below.

I'm not that familiar with DSE but if I understand correctly, the sequence is:

- Keep a list of instructions, known to be allocas (or alike?)
- Call for all callsites *after* those allocas getModRefInfo
- If CS can read from any of those allocas, all stores above the CS are live (and it's iterating BBs in reverse so marking all preceding stores live as soon as one call is found to Ref)

So the patch adds info about the list of allocas inside the MemoryLocation they modify and saves some of the work getModRefInfo does.

MemorySSA cannot currently provide info about isClobberedBy, it provides uses which are not yet optimized (this is a useful and expensive extension, and we should have a walker for it)
But even if we had this in MemorySSA now, the info cached in this case does not necessarily overlap with storing "isLocal" and "isNonEscaping" for a MemoryLocation, plus it is more expensive.
We're looking here for ModRef between two particular instances  that may have many other memory-accessing instructions inbetween, with whom they MayAlias. A getModRefInfo call should be cheaper than a MemorySSA query here.
Perhaps I could argue that rewriting checks entirely (to avoid this particular ModRef call) would make MemorySSA more viable, but that's a whole other discussion and it would involve re-writing the pass.

The cost is clearly the extra memory added to MemoryLocation (Chris's objection for BBs) which is a core data structure. Unlike BBs we create MemoryLocations often on the spot and drop them right away.
This may make things better (as far as memory cost throughout the compilation) or worse (as far as allocations).
IMO, as far as caching invalidation, it is clearly better than BBs, since MemoryLocations are not passed between Analyses or Transforms. Hence the KnownFlags set by a pass will be used only in that pass, and whatever caching the pass does is dropped for others. 
It could be argued that this could be another extension to add to MemorySSA so we do pass cached info forward, but I think that's beyond the scope of MemorySSA right now.

Hope this makes sense, and please feel free to pull in Chris and the others to give feedback.


Repository:
  rL LLVM

https://reviews.llvm.org/D53203