[PATCH] D32409: [PM/LoopUnswitch] Introduce a new, simpler loop unswitch pass.

[Davide Italiano via Phabricator via llvm-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM. I think the divergent bits can be handled in a follow-up, just wanted to make sure this wasn't an oversight.



================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:50
+STATISTIC(NumSwitches, "Number of switches unswitched");
+STATISTIC(NumTrivial, "Number of unswitches that are trivial");
+
----------------
chandlerc wrote:
> davide wrote:
> > Do we really need `NumTrivial`? From what I see here it's just `NumBranches + NumSwitches`
> Well, in the next patch we start doing non-trivial ones... I can wait until then if you prefer, I don't have a strong opinion.
You can leave it if you plan to use for non-trivial.


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:179-184
+  // FIXME: We should compute this once at the start and update it!
+  SmallVector<BasicBlock *, 16> ExitBlocks;
+  L.getExitBlocks(ExitBlocks);
+  SmallPtrSet<BasicBlock *, 16> ExitBlockSet(ExitBlocks.begin(),
+                                             ExitBlocks.end());
+
----------------
chandlerc wrote:
> davide wrote:
> > Instead of having `getExitBlocks()` taking a `SmallVector<>` can we have it or a variant of it which takes a `SmallPtrSet` ? 
> > I could use it in `LCSSA` to compute the dominance chain as well, FWIW.
> Sure, could I add that in a follow-up patch?
Yes, no problem.


https://reviews.llvm.org/D32409