[PATCH] D86133: [LoopNest] False negative of `arePerfectlyNested` with LCSSA loops

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added a comment.

`arePerfectlyNested` is written very conservative to start with, this patch allows loop nests with PHINodes in between to be considered perfect.



================
Comment at: llvm/lib/Analysis/LoopNestAnalysis.cpp:232
+  auto isLCSSABlock = [](const BasicBlock *B) {
+    if (!B)
+      return false;
----------------
How about taking a reference, so no need to check for nullptr.


================
Comment at: llvm/lib/Analysis/LoopNestAnalysis.cpp:239
+      auto *PN = dyn_cast<PHINode>(&I);
+      return PN && PN->getName().endswith(".lcssa");
+    });
----------------
Don't think checking name is ideal, because it can be changed in between passes. 
How about PHINodes that have single incoming entry?


================
Comment at: llvm/lib/Analysis/LoopNestAnalysis.cpp:272
   // Ensure the inner loop exit block leads to the outer loop latch.
-  if (InnerLoopExit->getSingleSuccessor() != OuterLoopLatch) {
+  auto *SuccInner = InnerLoopExit->getSingleSuccessor();
+  if (!SuccInner || (SuccInner != OuterLoopLatch &&
----------------
Use `const BasicBlock *`


================
Comment at: llvm/test/Analysis/LoopNestAnalysis/nests-with-lcssa.ll:1
+; RUN: opt -S -passes='loop-rotate,print<loopnest>' < %s 2>&1 > /dev/null | FileCheck %s
+
----------------
can the LLVMIR in this LIT be already after `loop-rotate`, so it doesn't need to reply on loop rotate.


================
Comment at: llvm/test/Analysis/LoopNestAnalysis/nests-with-lcssa.ll:11
+
+; Function Attrs: noinline nounwind optnone
+define i32 @f(i32 %N, i32 %M) #0 {
----------------
Function Attrs are not needed, so can all be removed.


================
Comment at: llvm/test/Analysis/LoopNestAnalysis/nests-with-lcssa.ll:13
+define i32 @f(i32 %N, i32 %M) #0 {
+; CHECK: IsPerfect=true, Depth=1, OutermostLoop: for.body3, Loops: ( for.body3 )
+; CHECK: IsPerfect=true, Depth=2, OutermostLoop: for.body, Loops: ( for.body for.body3 )
----------------
please rename the loop header name after rotation, so they can be easily identify.
e.g. `for.i` for the loop header of loop i. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86133