[PATCH] D11771: Fix how DependenceAnalysis calls de-linearization

[Vaivaswatha Nagaraj via llvm-commits]

vaivaswatha added inline comments.

================
Comment at: lib/Analysis/DependenceAnalysis.cpp:3247
@@ -3246,5 +3246,3 @@
 /// for each loop level.
-bool DependenceAnalysis::tryDelinearize(const SCEV *SrcSCEV,
-                                        const SCEV *DstSCEV,
-                                        SmallVectorImpl<Subscript> &Pair,
-                                        const SCEV *ElementSize) {
+bool DependenceAnalysis::tryDelinearize(Instruction *Src,
+                                        Instruction *Dst,
----------------
hfinkel wrote:
> Can these be const Instruction*?
Ideally, yes. However, ScalarEvolution::getElementSize() below does not take a const Instruction as its argument.

================
Comment at: lib/Analysis/DependenceAnalysis.cpp:3255
@@ +3254,3 @@
+  Loop *SrcLoop = LI->getLoopFor(Src->getParent());
+  Loop *DstLoop = LI->getLoopFor(Dst->getParent());
+
----------------
hfinkel wrote:
> The code in Delinearization.cpp does this:
> 
>     // Delinearize the memory access as analyzed in all the surrounding loops.
>     // Do not analyze memory accesses outside loops.
>     for (Loop *L = LI->getLoopFor(BB); L != nullptr; L = L->getParentLoop()) {
>       const SCEV *AccessFn = SE->getSCEVAtScope(getPointerOperand(*Inst), L);
> 
> So it walks up the chain of loops instead of just always using the inner loop. Should something similar be done here?
>From what I understand, It doesn't make much sense to do this here since all we need is to see the value's evolution in the innermost loop its defined in.

================
Comment at: lib/Analysis/DependenceAnalysis.cpp:3429
@@ -3428,3 @@
-  if (Delinearize && Pairs == 1 && CommonLevels > 1 &&
-      tryDelinearize(Pair[0].Src, Pair[0].Dst, Pair, SE->getElementSize(Src))) {
-    DEBUG(dbgs() << "    delinerized GEP\n");
----------------
hfinkel wrote:
> Am I correct that an alternative way to fix this problem is to call getSCEVAtScope on the SCEVs being passed here?
> 
Yes. Note that getSCEVAtScope needs to be called on "Src" and not "Pair[0].Src".
Also, I've removed the check "Pairs == 1" because its too strict. Even when the indices are linearized, its possible that the first index is 0 and the second one actually contains the linearized index (this is quite a common case) when indexing arrays (rather than pointers).


Repository:
  rL LLVM

http://reviews.llvm.org/D11771