[PATCH] D93686: [LoopUnroll] Fix a crash

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:365-370
+  BasicBlock *Preheader = L->getLoopPreheader();
+  BasicBlock *Header = L->getHeader();
+  BasicBlock *LatchBlock = L->getLoopLatch();
+  SmallVector<BasicBlock *, 4> ExitBlocks;
+  L->getExitBlocks(ExitBlocks);
+  std::vector<BasicBlock *> OriginalLoopBlocks = L->getBlocks();
----------------
Could you add a comment about that these have to be done after `peelLoop`?


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:370-371
+  L->getExitBlocks(ExitBlocks);
+  std::vector<BasicBlock *> OriginalLoopBlocks = L->getBlocks();
+  // Go through all exits of L and see if there are any phi-nodes there. We just
+  // conservatively assume that they're inserted to preserve LCSSA form, which
----------------
[nit] re-add newline like the  where it was moved from


================
Comment at: llvm/test/Transforms/LoopUnroll/unroll-after-peel.ll:7
+
+define i64 @hoge() !prof !0 {
+; CHECK-LABEL: @hoge(
----------------
Consider removing `!prof !0`


================
Comment at: llvm/test/Transforms/LoopUnroll/unroll-after-peel.ll:47
+  %tmp5 = phi i32 [ 8, %bb2 ], [ %tmp, %bb1 ]
+  %tmp6 = call i64 (...) @llvm.experimental.deoptimize.i64(i32 10) [ "deopt"() ]
+  ret i64 %tmp6
----------------
Is this call required for the test?


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

https://reviews.llvm.org/D93686