[PATCH] D45581: Add a LocationSize type to represent the size of MemoryLocations

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv abandoned this revision.
george.burgess.iv added a comment.

Thanks for the clarification, and sorry about the misunderstanding. I submitted the bit-for-bit NFC patch as r333314, and will rebase the original review soon. :)



================
Comment at: include/llvm/Analysis/AliasSetTracker.h:55
     AliasSet *AS = nullptr;
-    uint64_t Size = 0;
+    LocationSize Size = LocationSize::mapEmpty();
     AAMDNodes AAInfo;
----------------
reames wrote:
> To preserve the old behavior, doesn't this need to be 0?
If the goal is to preserve bit-for-bit behavior, yes. I failed to mention that I ran tests/built meaningful things with LLVM with `assert(isSizeSet())` before all Size accesses, and saw 0 issues. So, I assumed the intent was for size to always be set before anything attempted to access it. Hence, barring a few pathological cases with size nearing UINT64_MAX, the initial value doesn't appear to matter.

I considered this to be more noise than a meaningful functionality change, but I also didn't realize your intent was to have me submit something without review. With that in mind, yes, this change doesn't make much sense in this patch. :)


https://reviews.llvm.org/D45581