[PATCH] D44748: Track whether the size of a MemoryLocation is precise
George Burgess IV via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 28 15:13:53 PDT 2018
george.burgess.iv marked an inline comment as done.
george.burgess.iv added a comment.
Thanks for the comments!
================
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;
----------------
nlopes wrote:
> This is changing the semantics slightly. Before it would update Size if Newsize=-1. Is this intended?
I don't see how it's a functionality change. If `!Size.hasValue()`, that's the equivalent to `Size == UINT64_MAX` in the old code, which would result in updates never happening. If `NewSize != Size`, we'll conditionally update to the greater size (with the upper-bound bit definitely set). Am I missing something?
In any case, I removed the `hasValue()` check here, since we should handle `!hasValue()` correctly in `Size.combineWith(NewSize)`.
================
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;
----------------
nlopes wrote:
> Access size can be zero. Is it safe to do the above? You could use the map tombstone instead perhaps?
Good catch. I used the empty key since that's what we appear to use for `AAInfo`
https://reviews.llvm.org/D44748
More information about the llvm-commits
mailing list