[PATCH] D73129: [LoopUnrollAndJam] Correctly update LoopInfo when unroll and jam more than 2-levels loop nests.

[Whitney Tsang via Phabricator via llvm-commits]

Whitney marked 3 inline comments as done.
Whitney added a comment.

> Nothing in this code needs the blocks to be processed in RPO? (or at least some defined order)? I'm a little surprised, but I don't spot anything.

Functionally should be fine without an order, as now we are replacing values for all block for one unroll copy at the same time.

However, changing to deterministic order can make testing easier.



================
Comment at: llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp:186
+  // Clone Blocks.
+  for (BasicBlock *BB : Blocks) {
+    if (BB == FirstBlock)
----------------
dmgreen wrote:
> I think that it will matter that this isn't a deterministic order
Doesn't matter functionally, but it may affect the order in LIT tests. That's why I used CHECK-DAG in unroll-and-jam.ll. I agree that could be problematic, thinking to change Blocks from a set to a vector, so the caller of the function decide on a order. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73129