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

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/lib/Analysis/LoopNestAnalysis.cpp:239
+      auto *PN = dyn_cast<PHINode>(&I);
+      return PN && PN->getName().endswith(".lcssa");
+    });
----------------
TaWeiTu wrote:
> Whitney wrote:
> > TaWeiTu wrote:
> > > Whitney wrote:
> > > > Don't think checking name is ideal, because it can be changed in between passes. 
> > > > How about PHINodes that have single incoming entry?
> > > The LCSSA PHINodes may have two incoming entries, one from the exit block of the inner loop and the other from the header of the outer loop.
> > > I'm not entirely sure about whether the condition is correct and sufficient, though.
> > > The LCSSA PHINodes may have two incoming entries, one from the exit block of the inner loop and the other from the header of the outer loop.
> > 
> > I guess you mean from the exiting block of the inner loop? I thought LCSSA blocks are the exit blocks.
> > https://llvm.org/docs/LoopTerminology.html#loop-closed-ssa-lcssa
> > `they can just iterate over all the (loop closing) PHI nodes in the exit blocks`
> > 
> > I see what you mean by looking at the LIT test, in function `f`, `%split` is actually the LCSSA phi, not `%res.1.lcssa`. `%res.1.lcssa` is just a generic PHINode. 
> Oh I see. Thanks for clarifying! 
> Should I update the name of `IsLCSSABlock` to something else?
> 
Right, need to be renamed as it is not a LCSSA block (i.e. loop exit block).
Thinking of what we are trying to allow for perfect nest in this patch....
one extra block (optional) between inner exit block and outer latch block, which only contains PHINodes.

That extra basic block is needed because the inner loop is guarded and there exists LCSSA PHINodes in the inner loop exit block. 

Can we add this logic in?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86133