[PATCH] D109958: [LoopFlatten] Enable it by default

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/test/Other/new-pm-defaults.ll:168
+; CHECK-O-NEXT: Running pass: LoopSimplifyPass
+; CHECK-O-NEXT: Running pass: LCSSAPass
 ; CHECK-O-NEXT: Running pass: LoopIdiomRecognizePass
----------------
nikic wrote:
> asbirlea wrote:
> > aeubanks wrote:
> > > nikic wrote:
> > > > As a drive-by comment for @asbirlea and @aeubanks, it looks like there's an opportunity here to not rerun LoopSimplify/LCSSA if we run multiple Loop/LoopNest pass managers back to back?
> > > we need to fix where we add `LoopFlattenPass` in PassBuilderPipelines.cpp
> > > 
> > > instead of
> > > 
> > > ```
> > >   if (EnableLoopFlatten)
> > >     FPM.addPass(createFunctionToLoopPassAdaptor(LoopFlattenPass()));
> > > ```
> > > we should move it earlier to
> > > ```
> > >   if (EnableLoopFlatten)
> > >     LPM2.addPass(LoopFlattenPass());
> > > ```
> > +1 it should be added to a LPM.
> Oh, I wasn't aware that loop passes and loop nest passes can run in the same pass manager. That makes more sense indeed.
Thanks for commenting on this and the suggestions! LoopFlatten was intentionally added to where it currently lives for the LPM. LoopFlatten removes an inner-loop, and a loop pass was not able to deal with that very well under the LPM. But this is probably out of date for the NPM, and LoopFlatten was made a LoopNest pass in D102904, so I guess that means we can just add to where you suggested.


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

https://reviews.llvm.org/D109958