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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1129-1134
+  unsigned ASA = PtrA->getType()->getPointerAddressSpace();
+  unsigned ASB = PtrB->getType()->getPointerAddressSpace();
+
+  // Check that the address spaces match and that the pointers are valid.
+  if (!PtrA || !PtrB || (ASA != ASB) || PtrA->getType() != PtrB->getType())
+    return None;
----------------
lebedev.ri wrote:
> 
Ok, will do


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1158-1159
+
+    OffsetA = OffsetA.sextOrTrunc(IdxWidth);
+    OffsetB = OffsetB.sextOrTrunc(IdxWidth);
+
----------------
lebedev.ri wrote:
> 
It is used for `Size`


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1161-1165
+    //  OffsetDelta = OffsetB - OffsetA;
+    const SCEV *OffsetSCEVA = SE.getConstant(OffsetA);
+    const SCEV *OffsetSCEVB = SE.getConstant(OffsetB);
+    const SCEV *OffsetDeltaSCEV = SE.getMinusSCEV(OffsetSCEVB, OffsetSCEVA);
+    const APInt &OffsetDelta = cast<SCEVConstant>(OffsetDeltaSCEV)->getAPInt();
----------------
lebedev.ri wrote:
> Why not just do this in APInt directly?
Will try


================
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;
----------------
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.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1180-1185
+  if (Diff) {
+    int Val = Diff->getAPInt().getSExtValue() / Size.getSExtValue();
+    if (Val * Size == Diff->getAPInt())
+      return Val;
+  }
+  return None;
----------------
lebedev.ri wrote:
> Would it be better to add the non-constant case as an `else` to `if (PtrA1 == PtrB1) {`,
> and deduplicate this code?
I'll try


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98967