[PATCH] D110973: [DA] Handle mismatching loop levels by considering them non-linear

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

I was expecting that we only need to call getLoopDisposition once at the beginning of `checkSubscript` and then decide depending on whether it is a SCEVAddRecExpr, instead of

  if (!AddRec)
    return isLoopInvariant(Expr, LoopNest);

Is this possible?

Also, this patch only checks the innermost loop while `isLoopInvariant` checks all loops in the nest. Would we need as well? Would this fix miss the following case?

  for (int i = ) {
     int j = ...
     for (j < ...; ++j ) {
     }
     for (int k = ) {
        S[i][j][k];
    }
  }

That is, j is invariant to the k-loop, put still contains a SCEVAddRecExpr of a loop outside the loop nest.

An alternative check:

  diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
  index 61bc6750277f7..4a3c5c5e4b202 100644
  --- a/llvm/lib/Analysis/DependenceAnalysis.cpp
  +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
  @@ -891,6 +893,20 @@ void DependenceInfo::removeMatchingExtensions(Subscript *Pair) {
   // Collect any loops mentioned in the set of "Loops".
   bool DependenceInfo::checkSubscript(const SCEV *Expr, const Loop *LoopNest,
                                       SmallBitVector &Loops, bool IsSrc) {
  +  DenseSet<const Loop *> LoopsInNest;
  +  const Loop *L = LoopNest;
  +  while (L) {
  +    LoopsInNest.insert(L);
  +    L = L->getParentLoop();
  +  }
  +
  +  if (SCEVExprContains(Expr, [&](const SCEV *E) -> bool {
  +        if (auto *AddRec = dyn_cast<SCEVAddRecExpr>(E))
  +          return !LoopsInNest.contains(AddRec->getLoop());
  +        return false;
  +      }))
  +    return false;
  +
     const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(Expr);
     if (!AddRec)
       return isLoopInvariant(Expr, LoopNest);

This change passes all the DA tests, inlcuding `MismatchingNestLevels`
I think (hope?) that such a loop will result in the SCEV loop disposition to be LoopDisposition::LoopVariant, so the getLoopDisposition solution should be better. However, it might not suffice if the loop is not in loop all:

  int j = ...
  for (j < ...; ++j ) {
  }
  S[j];

Here, we don't have a loop that we could check the accesses' disposition for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110973