[PATCH] D63921: [Loop Peeling] Add support for peeling of loops with multiple exits

Serguei Katkov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 1 22:00:17 PDT 2019


skatkov marked 4 inline comments as done.
skatkov added inline comments.


================
Comment at: include/llvm/Analysis/LoopInfo.h:282
+  template <class ParamBlockT>
+  void getExitEdges(SmallVectorImpl<std::pair<ParamBlockT *, ParamBlockT *> > &
+                        ExitEdges) const {
----------------
fhahn wrote:
> This is not directly related to the peeling changes. I think it would be better to split this off in a separate patch & also add a test to the loopInfo unit tests with differing const-ness.
https://reviews.llvm.org/D64060


================
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:407
+        if (!L->contains(Succ)) {
+          ExitingBlock = Succ;
+          break;
----------------
fhahn wrote:
> Unless I am missing something, it looks like `Succ` here is outside the loop, which would mean that the latch would be an *exiting* block and Succ an *exit* block?
Ma bad. It should be latch. Will update a patch.


================
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:412
       Preheader = L->getLoopPreheader();
       ULO.TripCount = SE->getSmallConstantTripCount(L, ExitingBlock);
       ULO.TripMultiple = SE->getSmallConstantTripMultiple(L, ExitingBlock);
----------------
fhahn wrote:
> IIRC TripCount and TripMultiple would not be set for multi-exit loops (in LoopUnrollPass.cpp), but MaxTripCount would. But maybe I am missing the reasoning behind choosing the trip count of one of the exits?
It is not only for multi exit but also for single exit.
We support other exits only for deopts, so no need to worry about them.


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

https://reviews.llvm.org/D63921





More information about the llvm-commits mailing list