[PATCH] D124926: [LoopInterchange] New cost model for loop interchange

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added a comment.

For now we are failing one test case in LICM: `Transforms/LICM/lnicm.ll`. The reason is as follows. Looking at the source code:

  void test(int x[10][10], int y[10], int *z) {
    for (int k = 0; k < 10; k++) {
      int tmp = *z;
      for (int i = 0; i < 10; i++)
        x[i][k] += y[k] + tmp;
    }
  }

With the new cost model, loop cache analysis could analyze the access to array `y` but could not analyze the access to array `x` since it could not delinearize fixed-size array `x`. This is because the IR uses a chain of getelementptrs to access `x` which cannot be delinearized currently (as we discussed before). Hence loop cache analysis does the analysis only based on `y` and decides not to interchange. Ideally it should analyze both `x` and `y` and possibly decides to interchange the loops (whether to interchange or not depends on the tripcount numbers though). Currently this test case assumes we should interchange which we do not, and therefore the test fails.

There's multiple ways to resolve it. One possible way in my mind is that, we could change the following code in `populateReferenceGroups()` in `LoopCacheAnalysis.cpp` from:

  if (!R->isValid())
      continue;

to

  if (!R->isValid())
      return false;

which means that if it finds an IndexedReference that is not analyzable, it bails out with an empty loop vector and indicates it would not produce results if there's mem access that is not analyzable. In this case loop interchange will just fall back to the legacy cost model. If we go with this fix, I can post a simple patch for it.

Other possible fixes include modifying the gep instructions in `Transforms/LICM/lnicm.ll` to make mem accesses delinearizable and analyzable. I'd be fine with any possible solution. I'd appreciate it if you could let me know your thoughts? @bmahjour


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

https://reviews.llvm.org/D124926