[PATCH] D35380: [RuntimeUnroll] Add heuristic for unrolling multi-exit loop

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:435
 static bool
-canSafelyUnrollMultiExitLoop(Loop *L, SmallVectorImpl<BasicBlock *> &OtherExits,
+canUnrollMultiExitLoop(Loop *L, SmallVectorImpl<BasicBlock *> &OtherExits,
                              BasicBlock *LatchExit, bool PreserveLCSSA,
----------------
Code structure wise, I'd recommend splitting this function into two: one which checks *legality* and one which given a legal case, checks profitability.  Mixing the two needlessly is just confusing.

This could in fact be done as an NFC change.


================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:467
+  // All safety constraints have been satisfied. Now check for profitability.
+  auto isMultiExitUnrollingProfitable = [](ArrayRef<BasicBlock *> OtherExits) {
+
----------------
Why did you chose this particular heuristic?  What other options did you consider?  (some of this should be in the code itself)


https://reviews.llvm.org/D35380