[PATCH] D82927: Intergerate Loop Peeling into Loop Fusion

Fri Jul 17 07:40:02 PDT 2020

bmahjour added a comment.

Added some minor comments. Overall looks good to me. I'll approve if there are no objections by EOD today.

================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:1558
+    // case that FC0 loop is peeled, then move the instructions of the successor
+    // of the FC0 Exit block to the to the beginning of the exit block of FC1.
+    moveInstructionsToTheBeginning(
----------------
to the to the -> to the

================
Comment at: llvm/test/Transforms/LoopFusion/guarded_peel.ll:19
+; CHECK: for.first.peel.next:
+; CHECK: br label %for.first.peel2
+; CHECK: for.first.peel2:
----------------
CHECK -> CHECK-NEXT

================
Comment at: llvm/test/Transforms/LoopFusion/guarded_peel.ll:23
+; CHECK: for.first.peel.next1:
+; CHECK: br label %for.first.peel.next11
+; CHECK: for.first.peel.next11:
----------------
CHECK -> CHECK-NEXT

================
Comment at: llvm/test/Transforms/LoopFusion/guarded_peel.ll:25
+; CHECK: for.first.peel.next11:
+; CHECK: br label %for.first.entry.peel.newph
+; CHECK: br label %for.first
----------------
CHECK -> CHECK-NEXT

================
Comment at: llvm/test/Transforms/LoopFusion/guarded_peel.ll:26
+; CHECK: br label %for.first.entry.peel.newph
+; CHECK: br label %for.first
+; CHECK: for.first:
----------------
Add: `CHECK: for.first.entry.peel.newph`

There are also a number of empty basic blocks that make the test less readable. One way to make it more readable is to put a CHECK-NEXT (instead of a CHECK) right after the CHECK for the label to indicate that there is no code in the basic block.

================
Comment at: llvm/test/Transforms/LoopFusion/peel.ll:30
+; CHECK: for.first.peel.next:
+; CHECK: br label %for.first.peel2
+; CHECK: for.first.peel2:
----------------
Please use CHECK-NEXT for all branches that immediately follow the label.

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

https://reviews.llvm.org/D82927