[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 May 30 00:02:21 PDT 2018
george.burgess.iv added inline comments.
================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1054
DL.getTypeStoreSize(cast<SequentialType>(Ty)->getElementType());
- if (V1Size != ElementSize || V2Size != ElementSize)
+ if (V1Size.getValue() != ElementSize || V2Size.getValue() != ElementSize)
return MayAlias;
----------------
george.burgess.iv wrote:
> reames wrote:
> > Don't you need to check that we have a value here?
> There's a check near the top of the function (line 1002) for `V1Size == MemoryLocation::UnknownSize || V2Size == MemoryLocation::UnknownSize`. It's annoying that it's so far away, though. :/
>
> Happy to unwrap `V1Size` and `V2Size` into `unsigned`s directly after the check if you think that'd be clearer.
> Happy to unwrap V1Size and V2Size into unsigneds directly after the check if you think that'd be clearer.
(I've done this across BasicAA in hopes of making things a bit clearer. Please let me know if you disagree)
================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1188
+ return GEPBaseOffset >=
+ ObjectBaseOffset + (int64_t)ObjectAccessSize.getValue();
}
----------------
Not a BasicAA expert, but since I see "negative offset" and an int64_t cast here, I wanted to highlight that this LocationSize change eats the sign bit of `ObjectAccessSize`. I assume that `ObjectAccessSize` should always be positive and the `int64_t` cast exists just to keep the >= comparison signed, but...
https://reviews.llvm.org/D44748
More information about the llvm-commits
mailing list