[PATCH] D44667: [LoopPredication] Add profitability check based on BPI

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopPredication.cpp:726
+  SmallVector<BasicBlock *, 4> ExitingBlocks;
+  L->getExitingBlocks(ExitingBlocks);
+  auto *LatchBlock = L->getLoopLatch();
----------------
you do not need ExitingBlocks more.


================
Comment at: lib/Transforms/Scalar/LoopPredication.cpp:736
+      BPI->getEdgeProbability(LatchBlock, LatchBrExitIdx);
+  SmallVector<std::pair<const BasicBlock*, const BasicBlock*>, 8> ExitEdges;
+  L->getExitEdges(ExitEdges);
----------------
Please consider re-ordering the lines in this piece of code (it is a bit difficult to read it).

I would suggest to move all code related to getting edge probability below to computation of LatchProbabilityThreshold (to me it is connected parts). Additionally you will have an earlier bail out due to more than one exit and you do not need to do anything for latch in this case.

And separate by new line the code related to LatchProbabilityThreshold and getting ExitEdges.


================
Comment at: lib/Transforms/Scalar/LoopPredication.cpp:748
+  for (const auto &ExitEdge : ExitEdges) {
+    BranchProbability ExitingBlockProbability =
+        BPI->getEdgeProbability(ExitEdge.first, ExitEdge.second);
----------------
You still need to skip latch exiting basic block.


Repository:
  rL LLVM

https://reviews.llvm.org/D44667