[PATCH] D43245: [LoopInterchange] Support reductions across inner and outer loop.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:829
-  // Outer loop cannot have reduction because then loops will not be tightly
-  // nested.
-  if (!Reductions.empty()) {
----------------
Given you're deleting this, is there some check that disallows interchanging if a PHI is a reduction in the outer loop, but not the inner loop?  Something like:

```
  for( int i=1;i<N;i++) {
    for( int j=1;j<N;j++) {
      A[i][j]++;
    }
    X += j;
  }
```


================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:1506
   // Move the PHI nodes from the inner loop header to the outer loop header.
   // We have to deal with one kind of PHI nodes:
   //  1) PHI nodes that are part of a reduction in the outer loop
----------------
one kind?


================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:1521
+      if (InnerLoop->contains(InBB) ||
+          isa<PHINode>(PHI->getIncomingValueForBlock(InBB)))
         continue;
----------------
Checking `isa<PHINode>()` here seems sort of confusing... is this supposed to be checking for a PHI node from the outer loop?  If so, is there some guarantee it isn't a PHI node from somewhere else?


https://reviews.llvm.org/D43245