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

Ta-Wei Tu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 18 08:48:47 PDT 2020


TaWeiTu added inline comments.


================
Comment at: llvm/lib/Analysis/LoopNestAnalysis.cpp:239
+      auto *PN = dyn_cast<PHINode>(&I);
+      return PN && PN->getName().endswith(".lcssa");
+    });
----------------
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?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86133



More information about the llvm-commits mailing list