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

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

In D124926#3538021 <https://reviews.llvm.org/D124926#3538021>, @congzhe wrote:

> 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. IMHO it does make some sense because if loop cache analysis cannot successfully analyze all mem accesses, it should bail out otherwise incorrect decisions might be made due to missed info. 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

Overtime we'd want to improve delinearization and remove dependency on the legacy loop interchange cost model. I'd prefer changing the test case to become more friendly to delinearization. That test case needs to be rewritten soon to get rid of typed pointers anyway, so might as well use opaque pointers now and make the arrays non-fixed size.


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

https://reviews.llvm.org/D124926