[PATCH] D93529: [AA] Store and return estimated PartialAlias offsets.

Wed Jan 27 11:30:37 PST 2021

dfukalov marked 3 inline comments as done.
dfukalov added a comment.

In D93529#2517400 <https://reviews.llvm.org/D93529#2517400>, @asbirlea wrote:

> ... Could you (re-)upload a concrete use case as dependent patch?

I've just created D95543 <https://reviews.llvm.org/D95543> with the test for upcoming changes in GVN. It is actually the same code as one used in unit test in this patch.

================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1207
+          // cache.
+          if ((Off + VRightSize->getValue()).ule(VLeftSize->getValue()))
+            AAQI.setClobberOffset({LeftPtr, RightPtr}, Off.getSExtValue());
----------------
nikic wrote:
> I'm not sure this code is the right place to perform these access size checks. Consider the case where BatchAA is queried for the same pointers with different location sizes in a row. The first query is such that they are PartialAlias and the LocationSize fits into the larger access. The second one is also PartialAlias, but does not fit. In this case, the getClobberOffset() API will still return a result though, and a caller may incorrectly assume that the access is nested based on this check, even though it isn't.
Thanks for good point! Added sizes to cache key, updated test to check that offset is returned only for exact the same set of two pointers and two sizes.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93529/new/

https://reviews.llvm.org/D93529