[PATCH] D144226: [Loop-Interchange] Allow inner-loop only reductions

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added a comment.

In addition to Michael's comment, I'd like to suggest several comments:

1. This patch "undo" LICM within loop interchange in order to form a tightly nest loopnest. This sounds like a phase ordering problem, i.e., you could just place loop interchange before LICM and then you'll be able to interchange the loopnest. If it is a phase ordering problem then I'm not sure if it makes perfect sense to undo pass A within pass B, because things could quickly get messy if we choose to develop in this way.

2. Also, have you considered the "loopnest" version of loop passes? For example if you use the loopnest invariant code motion (LNICM) pass instead of LICM pass in the pipeline, you'll not hoist `z[j]` into `int tmp = z[j]` (in the example from your summary), because LNICM will only do hoisting if a variable is invariant across the entire loopnest.

3. In D53027 <https://reviews.llvm.org/D53027>, support for inner-only reductions is removed because of miscompilation of certain cases, as well as profitability concerns since interchange would move loads and stores from the outer loop into the inner loop. Have you thought about addressing these problems?

One possible miscompilation, as mentioned in https://reviews.llvm.org/D53027#1272786, is that it would miscompile if we interchange the following code, given that y is a global variable.

  for (unsigned i = 0; i < N; i++) {
    y = 0;
    for (unsigned j = 0; j < N; j++) {
      A[i][j] += 1;
      y += A[i][j];
    }
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144226