[PATCH] D98967: [Analysis]Add getPointersDiff function to improve compile time.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1132-1133
+
+  // Check that the address spaces match and that the pointers are valid.
+  if (!PtrA || !PtrB || (ASA != ASB) || PtrA->getType() != PtrB->getType())
+    return None;
----------------
Feel free to ignore this one:
for opaque ptr types forward-compatibility, it might be good to just take element type as an extra argument
and not look at the elt types of the pointers.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1158-1159
+
+    OffsetA = OffsetA.sextOrTrunc(IdxWidth);
+    OffsetB = OffsetB.sextOrTrunc(IdxWidth);
+
----------------
ABataev wrote:
> lebedev.ri wrote:
> > 
> It is used for `Size`
Err, i should have left this as a comment.
I think this should mention that while we might have traversed into another address space,
we still want to keep the original index width.
Either that, or we need to refresh index width.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1167-1172
+    // Check if they are based on the same pointer. That makes the offsets
+    // sufficient.
+    int Val = OffsetDelta.getSExtValue() / Size.getSExtValue();
+    if (Val * Size == OffsetDelta)
+      return Val;
+    return None;
----------------
ABataev wrote:
> lebedev.ri wrote:
> > But this checks that `OffsetDelta` is a multiple of original pointed-to type,
> > not what the comment says?
> > 
> > I suppose this overly restrictive.
> 1. Yes, the comment is wrong, will fix it.
> 2. Unfortunately not. Caused by a real example.
Err, i understand what this checks and that this is for sure currently needed in some cases,
i'm only saying that perhaps some code could deal with sub-element-misalignment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98967