[PATCH] D91682: [LoopInterchange] Fix the checking of tightly nested loop

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/lib/Analysis/LoopNestAnalysis.cpp:126
+      for (PHINode &PHI : OuterLoop.getHeader()->phis())
+        if (InductionDescriptor::isInductionPHI(&PHI, &OuterLoop, &SE, ID))
+          break;
----------------
If there are multiple induction phi, how can we know the first one encountered is the induction phi used for the loop bound?


================
Comment at: llvm/lib/Analysis/LoopNestAnalysis.cpp:129
+      Instruction *StepInst = ID.getInductionBinOp();
+      if ((isa<BinaryOperator>(I) && &I != StepInst &&
+           !(InnerLoop.isLoopInvariant(I.getOperand(0)) &&
----------------
This change the current definition of perfect loop nest.
Why do you think we should consider loop nest as perfect with inner loop invariant binary operator in between the outer and inner loop?
If all loops in the loop nest are LCSSA, then the only possible users of instructions defined in the inner loop is phi nodes at inner loop exit block. which makes all binary operator operands to be inner loop invariant.


================
Comment at: llvm/lib/Transforms/Scalar/LoopInterchange.cpp:958
+  LoopNest LN(*OuterLoop, *SE);
+  do {
+    LoopOuter = LoopInner->getParentLoop();
----------------
Have you considered using `if (LN.getMaxPerfectDepth() != InnerLoop->getLoopDepth())`?


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

https://reviews.llvm.org/D91682