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

[Nuno Lopes via Phabricator via llvm-commits]

nlopes added a comment.

I like the direction in general. I've reviewed this patch and it LGTM (as well as the overall plan).
There are still a few corner cases we need to fix regarding the meaning of size -1, but I guess it's an orthogonal fix. Right now I don't know exactly what -1 size is: does it mean potentially access the whole object, or access the object from the current offset and potentially until the end?



================
Comment at: include/llvm/Analysis/AliasSetTracker.h:74
       bool SizeChanged = false;
-      if (NewSize > Size) {
-        Size = NewSize;
-        SizeChanged = true;
+      if (NewSize != Size && Size.hasValue()) {
+        LocationSize OldSize = Size;
----------------
This is changing the semantics slightly. Before it would update Size if Newsize=-1. Is this intended?


================
Comment at: include/llvm/Analysis/AliasSetTracker.h:78
+        // we're getting. Don't treat it as an upper-bound.
+        Size = Size.isZero() ? NewSize : Size.combineWith(NewSize);
+        SizeChanged = OldSize != Size;
----------------
Access size can be zero. Is it safe to do the above?  You could use the map tombstone instead perhaps?


https://reviews.llvm.org/D44748