[PATCH] D44748: Track whether the size of a MemoryLocation is precise

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added inline comments.


================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:1115
   if (!Pair.second) {
-    if (CacheInfo->Size < Loc.Size) {
-      // The query's Size is greater than the cached one. Throw out the
-      // cached data and proceed with the query at the greater size.
-      CacheInfo->Pair = BBSkipFirstBlockPair();
-      CacheInfo->Size = Loc.Size;
-      for (auto &Entry : CacheInfo->NonLocalDeps)
-        if (Instruction *Inst = Entry.getResult().getInst())
-          RemoveFromReverseMap(ReverseNonLocalPtrDeps, Inst, CacheKey);
-      CacheInfo->NonLocalDeps.clear();
-    } else if (CacheInfo->Size > Loc.Size) {
-      // This query's Size is less than the cached one. Conservatively restart
-      // the query using the greater size.
-      return getNonLocalPointerDepFromBB(
-          QueryInst, Pointer, Loc.getWithNewSize(CacheInfo->Size), isLoad,
-          StartBB, Result, Visited, SkipFirstBlock);
+    if (!CacheInfo->Size.equalsIgnoringPrecision(Loc.Size)) {
+      // For our purposes, unknown size > all others.
----------------
nlopes wrote:
> Why is it ok to ignore precision here?
> If we did an alias query before with precise size, and the new one is with upper-bound, the results don't seem reusable.
> (I have no clue how this function works; so I don't know if the above case can happen)
I was trying to keep this as close to NFC as possible, but yeah, good point.

I tried bootstrapping clang with assertions about the size either being unknown or precise, and no assertions fired, so it doesn't appear that known-but-imprecise values can reach here (or if they do, they do so very rarely).

Switched to always throwing out the cached data if the precision differs (we can do better if we know the value is now precise, too). Happy to swap to what you suggested if you feel that's better, though. Like you, I'm not super familiar with memdep...


https://reviews.llvm.org/D44748