[PATCH] D22630: Loop rotation

[Aditya Kumar via llvm-commits]

hiraditya added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/LoopRotation.cpp:403-404
@@ +402,4 @@
+
+  BasicBlock *OrigH = L->getHeader();
+  BasicBlock *LoopLatch = L->getLoopLatch();
+
----------------
mzolotukhin wrote:
> We definitely want to have some thresholds, like we do now - maybe just rename the function somehow? `ProfitableToRotate` sounds to me like there would be a benefit right from loop-rotate, while in reality we just hope that a later pass would take advantage of it. As I see it, we want to rotate as many loops as possible, being limited only by thresholds, and not considering profitability of the rotation itself. BTW, are we in agreement on this?
The function 'isProfitableToRotate' only checks if the code-size increase would be under a budget. So I can rename the function maybe?


================
Comment at: llvm/test/Transforms/LoopRotate/loop-rotate-4.ll:5
@@ +4,3 @@
+
+; Check that the loop with indirect branches is rotated.
+; CHECK-LABEL: foo
----------------
mzolotukhin wrote:
> Is there any indirect branches in this test?
The switch block.

================
Comment at: llvm/test/Transforms/LoopRotate/loop-rotate-6.ll:5
@@ +4,3 @@
+
+; Check that the loop with header having a switch block is rotated.
+; CHECK-LABEL: bar
----------------
mzolotukhin wrote:
> Do we have to have several switches to test this?
I removed one switch statement. Other two are required because one is in the loop pre-header, other is in the body of the loop.


https://reviews.llvm.org/D22630