[PATCH] D136643: [LoopPeeling] Add flag to disable support for peeling loops with non-latch exits

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopPeel.cpp:112
+  // updating.
+  return llvm::all_of(Exits, IsBlockFollowedByDeoptOrUnreachable);
 }
----------------
Is it possible to reduce these conditions to the ones relevant to the affected test case? Like, I doubt that the check for `BranchInst` is relevant, and it would be good to know whether for the problematic case having a non-latch exit is the issue, or the structure of other exits.


================
Comment at: llvm/lib/Transforms/Utils/LoopPeel.cpp:528
 
+static void updateBranchWeightsLimited(BasicBlock *Header, BranchInst *LatchBR,
+                                       uint64_t ExitWeight,
----------------
Why is all this code needed? I would have expected the change to be just in canPeel(). For the cases where canPeel() previously returned true, the new implementation should behave the same apart from minor differences in branch weights.

Does that mean that you see issues if you limit peeling, but use the new branch weight updating code? That would be quite surprising, and would point towards something there being broken.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136643