[PATCH] D22630: Loop rotation

[Aditya Kumar via llvm-commits]

hiraditya added inline comments.

================
Comment at: llvm/test/Analysis/ScalarEvolution/2012-03-26-LoadConstant.ll:1
@@ -1,1 +1,2 @@
-; RUN: opt < %s -basicaa -globalopt -instcombine -loop-rotate -licm -instcombine -indvars -loop-deletion -constmerge -S | FileCheck %s
+; RUN: opt < %s -basicaa -globalopt -instcombine -loop-rotate -licm -simplifycfg -S | FileCheck %s
+
----------------
mzolotukhin wrote:
> Why is this change needed?
Now that the loop rotation does not simplify any instructions, we need simplifycfg post-licm to clean up the redundancies exposed by loop rotate.

================
Comment at: llvm/test/Transforms/LoopRotate/loop-rotate-2.ll:1
@@ +1,2 @@
+; RUN: opt -S < %s -loop-rotate -verify-dom-info -verify-loop-info | FileCheck %s
+
----------------
mzolotukhin wrote:
> This test look very similar to `loop-rotate-0.ll` (and some other tests actually). While I understand, that they can exercise slightly different parts of implementation and crash in a different ways, without a proper comment it looks redundant (actually, I suspect it is redundant). Tests clean-up that I'm asking for should help revealing if they test the same case, or not - that's why I think it's important.
This is different from the loop-rotate-0.ll because there is a branch within the loop. I'll add more comments. THanks for the pointer

================
Comment at: llvm/test/Transforms/LoopRotate/loop-rotate-4.ll:55-63
@@ +54,11 @@
+  switch i32 undef, label %if.else427 [
+    i32 26, label %if.then130
+    i32 24, label %if.then130
+    i32 23, label %if.then130
+    i32 22, label %if.then130
+    i32 20, label %if.then130
+    i32 19, label %if.then130
+    i32 18, label %if.then130
+    i32 12, label %if.then130
+    i32 6, label %if.then149
+    i32 1, label %if.then172
----------------
mzolotukhin wrote:
> Even if bugpoint fails to reduce this test further, you should be able to manually remove unneeded successors of the switch, and then replace it with a branch (it's worth trying the latest bugpoing - probably it'll reduce it). That should make the test much smaller and easier to understand.
This was actually a bugpoint reduceed test. I'll see if some of the case statements can be removed.


https://reviews.llvm.org/D22630