[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.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Thanks all, landing!



================
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);
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D47408