[PATCH] D61962: [LoopUnroll] Add support for loops with exiting headers and uncond latches.

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:369-371
+  if ((!BI || BI->isUnconditional()) &&
+      (!BI || !HeaderBI || HeaderBI->isUnconditional() ||
+       L->getExitingBlock() != Header)) {
----------------
I think you can factor out the `!BI` case here to simplify the logic a little bit?


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:392
+    return HeaderBI &&
+           HeaderBI->isConditional() &
+               L->contains(HeaderBI->getSuccessor(S1)) &&
----------------
Probably should be `&&`?


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:393
+           HeaderBI->isConditional() &
+               L->contains(HeaderBI->getSuccessor(S1)) &&
+           !L->contains(HeaderBI->getSuccessor(S2));
----------------
indentation is strange here


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:844-852
+      if (CompletelyUnroll) {
+        // We cannot drop the conditional branch for the last condition, as we
+        // may have to execute the loop body depending on the condition.
+        NeedConditional = j == 0 || ULO.PreserveCondBr;
+      } else if (j != BreakoutTrip &&
+                 (ULO.TripMultiple == 0 || j % ULO.TripMultiple != 0))
+        // If we know the trip count or a multiple of it, we can safely use an
----------------
Can you make the braces here consistent? The `if` has braces while the `else if` doesn't which looks rather strange.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61962