[PATCH] D29488: [DA] Fix for PR31848: Treat AddRec subscripts containing extra loops as NonLinear

[Philip Pfaffe via Phabricator via llvm-commits]

philip.pfaffe created this revision.

The SCEVAddRecExpr nest describing a Subscript may contain loops in which the corresponding memory access is not contained in. This causes errors in DependenceAnalysis. An example is this (see PR31848 for details):

void foo(int *A, int n) {

  for (int j = 0; j < n; ++j) {
    for (int i = 0; i < n; ++i) {
      for (int di = -1; di <= 1; ++di) {
        for (int dj = -1; dj <= 1; ++dj) {
          int x = i + di;
          int y = j + dj;
          while (x < 0)
            x += n;
          while (y < 0)
            y += n;
  
          A[y * n + x] = 7;
        }
      }
    }
  }

}

This patch detects that, and classifies the subscripts as NonLinear. This solution is possibly a bit over-pessimistic. For instance, subscripts might well be legal, if its extraneous loops are loop invariant wrt. there respective relevant parents in the loop nest. I haven't been able to produce this situation however, so I'm not sure whether that can actually happen, or whether it triggers entirely different paths of DA.


Repository:
  rL LLVM

https://reviews.llvm.org/D29488

Files:
  lib/Analysis/DependenceAnalysis.cpp


Index: lib/Analysis/DependenceAnalysis.cpp
===================================================================
--- lib/Analysis/DependenceAnalysis.cpp
+++ lib/Analysis/DependenceAnalysis.cpp
@@ -848,14 +848,24 @@
   }
 }
 
+static const Loop *getLowestCommonAncestor(const Loop *A, const Loop *B) {
+  if (!A || !B)
+    return nullptr;
+  if (A == B)
+    return A;
+  if (A->getLoopDepth() > B->getLoopDepth())
+    return getLowestCommonAncestor(A->getParentLoop(), B);
+  return getLowestCommonAncestor(A, B->getParentLoop());
+}
 
 // Examine the scev and return true iff it's linear.
 // Collect any loops mentioned in the set of "Loops".
 bool DependenceInfo::checkSrcSubscript(const SCEV *Src, const Loop *LoopNest,
                                        SmallBitVector &Loops) {
   const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(Src);
   if (!AddRec)
     return isLoopInvariant(Src, LoopNest);
+  const auto *Loop = AddRec->getLoop();
   const SCEV *Start = AddRec->getStart();
   const SCEV *Step = AddRec->getStepRecurrence(*SE);
   const SCEV *UB = SE->getBackedgeTakenCount(AddRec->getLoop());
@@ -866,6 +876,12 @@
         return false;
     }
   }
+
+  // Test if the Loop this AddRec refers to is an ancestor of LoopNest
+  auto *LCA = getLowestCommonAncestor(Loop, LoopNest);
+  if (LCA && LCA != Loop) // Loop is a sibling or uncle of LoopNest.
+    return false;
+
   if (!isLoopInvariant(Step, LoopNest))
     return false;
   Loops.set(mapSrcLoop(AddRec->getLoop()));
@@ -881,19 +897,25 @@
   const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(Dst);
   if (!AddRec)
     return isLoopInvariant(Dst, LoopNest);
+  const auto *Loop = AddRec->getLoop();
   const SCEV *Start = AddRec->getStart();
   const SCEV *Step = AddRec->getStepRecurrence(*SE);
-  const SCEV *UB = SE->getBackedgeTakenCount(AddRec->getLoop());
+  const SCEV *UB = SE->getBackedgeTakenCount(Loop);
   if (!isa<SCEVCouldNotCompute>(UB)) {
     if (SE->getTypeSizeInBits(Start->getType()) <
         SE->getTypeSizeInBits(UB->getType())) {
       if (!AddRec->getNoWrapFlags())
         return false;
     }
   }
+  // Test if the Loop this AddRec refers to is an ancestor of LoopNest
+  auto *LCA = getLowestCommonAncestor(Loop, LoopNest);
+  if (LCA && LCA != Loop) // Loop is a sibling or uncle of LoopNest.
+    return false;
+
   if (!isLoopInvariant(Step, LoopNest))
     return false;
-  Loops.set(mapDstLoop(AddRec->getLoop()));
+  Loops.set(mapDstLoop(Loop));
   return checkDstSubscript(Start, LoopNest, Loops);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29488.86950.patch
Type: text/x-patch
Size: 2531 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170203/a7bbd872/attachment.bin>