[PATCH] D22630: Loop rotation

[Eli Friedman via llvm-commits]

eli.friedman added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/LoopRotation.cpp:319
@@ +318,3 @@
+  // Collect all nodes of the loop from header to latch.
+  BasicBlock *NewH = collectSEMEBlocks(OrigH, OrigLatch, Blocks, Exits);
+  assert(NewH);
----------------
sebpop wrote:
> hiraditya wrote:
> > eli.friedman wrote:
> > > Is it possible this will clone more code?  The trunk version would only clone the header; it looks like this will clone the whole loop body excluding the latch block?
> > Yes, this will clone all the exiting basic blocks excluding the latch block (starting from the header until all it finds a BB which is not exiting). This is required to expose the redundancies that could be removed as a result of cloning the loop body. Currently there is a flag to control how much we want to clone (RotationMaxSize), I can add more specific flags to control how many exiting blocks we want to copy.
> > In general, it would be a good idea to clone all the exiting basic blocks to expose all the redundancies.
> > 
> > until all it finds a BB which is not exiting
> To rotate the loop from Michael (loop with if-then-else followed by loop exit) we could avoid stopping at the first BB which is not exiting the loop, and to continue until all loop exit edges have been duplicated.
> 
This is why I'd really like to see this patch split... it would be much easier to have this discussion with a patch which is specifically "Clone more blocks in loop rotation", so we could discuss what exactly makes sense independently of the changes to the cloning algorithm.


https://reviews.llvm.org/D22630