[PATCH] D47408: [PM/LoopUnswitch] When using the new SimpleLoopUnswitch pass, schedule loop-cleanup passes at the beginning of the loop pass pipeline, and re-enqueue loops after even trivial unswitching.

[Fedor Sergeev via Phabricator via llvm-commits]

fedor.sergeev added inline comments.
Herald added a subscriber: steven_wu.


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:396-397
   // `SimplifyCFGPass` and `InstCombinePass` are used. We have
   // `LoopSimplifyCFGPass` which isn't yet powerful enough, and the closest to
   // the other we have is `LoopInstSimplify`.
   LoopPassManager LPM1(DebugLogging), LPM2(DebugLogging);
----------------
chandlerc wrote:
> fedor.sergeev wrote:
> > chandlerc wrote:
> > > fedor.sergeev wrote:
> > > > Time to update the comment? :)
> > > No? I think this is still true. We still break it into two pipelines. I don't think LoopSimplifyCFG has had enough of SimplifyCFG ported to it to make it effective to re-merge the loop pipelines (yet).
> > It kind of leaves an impression of "either SimplifyCFG or LoopSimplifyCFG", while with this change it is both.
> > 
> > Perhaps:
> > "These can and should be *removed* (not replaced) as soon as their equivalents are ready as a full replacement"
> I've tried to clarify this comment some. We might still need to replace them or adjust the *position* of these loop cleanup passes, but I've tried to make it much more clear what is going on.
> 
> If it still isn't making sense, please keep poking and I'm happy to make follow-up commits to get the comment into a shape where it won't lead to confusion.
Thanks, it is fine now.


Repository:
  rL LLVM

https://reviews.llvm.org/D47408