[PATCH] D46198: [LoopInterchange] Move PHI handling to adjustLoopBranches.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:1467
+      if (InnerLoop->contains(InBB) ||
+          isa<PHINode>(PHI->getIncomingValueForBlock(InBB)))
+        continue;
----------------
fhahn wrote:
> efriedma wrote:
> > This `isa<PHINode>` check seems weird; do you care whether it's specifically a reduction PHI node?
> > 
> > 
> Thanks Eli! 
> 
> Yes I think it misses that case when there is an instruction between the outer loop reduction PHI and the corresponding PHI node in the inner loop. I think I can get rid of this check in this patch and add something to D43245, as D43245 also relaxes the restrictions to allow such nodes.
I've replaced the check with an assertion, as reductions in outer are not supported in this patch.

I moved the check to D43245 and added a comment with the reasoning behind it. For reductions across inner and outer loops, it looks like isReductionPHI expects the incoming value of a reduction PHI in the inner loop to be the reduction PHI node of the outer loop.


https://reviews.llvm.org/D46198