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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Seems about right, some thoughts.



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



================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1158-1159
+
+    OffsetA = OffsetA.sextOrTrunc(IdxWidth);
+    OffsetB = OffsetB.sextOrTrunc(IdxWidth);
+
----------------



================
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();
----------------
Why not just do this in APInt directly?


================
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;
----------------
But this checks that `OffsetDelta` is a multiple of original pointed-to type,
not what the comment says?

I suppose this overly restrictive.


================
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;
----------------
Would it be better to add the non-constant case as an `else` to `if (PtrA1 == PtrB1) {`,
and deduplicate this code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98967