[PATCH] D30161: [LoopPeeling] Peel loops with invariant backedge Phi input

[Anna Thomas via Phabricator via llvm-commits]

anna added a comment.

Hi Max,

I'm not very sure about the change in priority of the loop peeling. There can be performance repercussions for that change alone, and might be better to get another opinion on the priority change.
Could you perhaps have this change as a strict improvement for loop peeling in itself?



================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:788
+  // 4th priority is loop peeling
+  computePeelCount(L, LoopSize, UP);
+  if (UP.PeelCount) {
----------------
Can this patch be separated into 2 parts, where the second part is this change to the priority?


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:114
+  BasicBlock *Header = L->getHeader();
+  // Find the only back edge.
+  BasicBlock *BackEdge = nullptr;
----------------
There is a method for this `getNumBackEdges`. I think you can just reuse this and have a check against 1. 


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:116
+  BasicBlock *BackEdge = nullptr;
+  for (auto PI = pred_begin(Header), PE = pred_end(Header); PI != PE; ++PI) {
+    BasicBlock *BB = *PI;
----------------
can change this to `for (auto *Pred: predecessors(Header))`


https://reviews.llvm.org/D30161