[PATCH] D92723: [BasicAA] Migrate "same base pointer" logic to decomposed GEPs

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 5 14:48:44 PST 2020


reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

Ok, you and I are clearly thinking about the same problems.  :)  I have a patch which I hadn't yet posted to handle this exact same case.  I just needed to get prerequisites out of the way in terms of D92698 <https://reviews.llvm.org/D92698> and D92694 <https://reviews.llvm.org/D92694>.  I'm happy to defer to you and act as knowledgeable reviewer though!



================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1286
+      // of values across loop iterations.
+      if (Var0.Scale == -Var1.Scale && Var0.ZExtBits == Var1.ZExtBits &&
+          Var0.SExtBits == Var1.SExtBits && VisitedPhiBBs.empty() &&
----------------
You're missing the bug fix for the reasoning under recursive phis that I just landed.in bfda6941.  Please add the appropriate VisitedPhiBBs.empty check to your conditions.


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1289
+          isKnownNonEqual(Var0.V, Var1.V, DL)) {
+        // If the indexes are not equal, the actual offset will have at least
+        // Scale or -Scale added to it.
----------------
You're missing a necessary check here.  You need to ensure that scale > size and scale mod size == 0.  Otherwise, you can have a case such as scale == 2, size == 8, and have the (unaligned) accesses alias even if the indices differ.  


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1292
+        APInt Scale = Var0.Scale.abs();
+        APInt OffsetLo = DecompGEP1.Offset - Scale;
+        APInt OffsetHi = DecompGEP1.Offset + Scale;
----------------
I'm not following your logic here, and I think you may have it backwards.

If we know that the offsets are equal, scales are equal, and indices aren't, then we have no alias.  If we have different offsets, then we need to reason about the offset access sequences.  You might be trying to handle the offset pattern case, but if so, I'm not following?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92723



More information about the llvm-commits mailing list