[PATCH] D91383: [BasicAA] Make alias GEP positive offset handling symmetric
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 13 00:35:38 PST 2020
nikic added inline comments.
================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1452
+ (-GEP1BaseOffset).uge(V2Size.getValue()))
+ return NoAlias;
----------------
asbirlea wrote:
> jdoerfert wrote:
> > nikic wrote:
> > > jdoerfert wrote:
> > > > Copy paste error?
> > > Um, where? I'm not seeing it ^
> > I think I was simply confused. Maybe today is not the day to finish this review (for me). I'll give it another shot tomorrow ;)
> I believe this is correct :-).
I think @jdoerfert was right, and there is a mistake here: This should be comparing against `V1Size` rather than `V2Size`.
```
// Non-negative case:
0 ... V2Size ... GEP1BaseOffset ... GEP1BaseOffset+V1Size
// Non-positive case:
GEP1BaseOffset ... GEP1BaseOffset+V1Size ... 0 ... V2Size
```
We need to check that V1Size fits between GEP1BaseOffset and 0, the V2Size access is above zero and not relevant.
I'll have to add a test where both access sizes are different.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91383/new/
https://reviews.llvm.org/D91383
More information about the llvm-commits
mailing list