[llvm-commits] [llvm] r142743 - in /llvm/trunk: lib/CodeGen/MachineBlockPlacement.cpp test/CodeGen/X86/block-placement.ll

[Andrew Trick]

On Oct 26, 2011, at 1:13 AM, Chandler Carruth wrote:

> YES. I would greatly appreciate making an RPO order guarantee at the loop info and/or function layer from an analysis that I can piggyback on. Doing it at the loop layer would be wonderful for implementing this kind of algorithm.
>  
> You're currently sidestepping the
> issue by placing chains in a single iteration through the function's
> blocks. If that works well, I don't have a problem with it, but it
> seems to complicate the issue of loop layout. For example, our simple
> RPO iterator makes no attempt to visit blocks in loop order, so
> following this order will not result in contiguous loops. I think
> laying out one loop at a time is much easier to deal with. If you want
> to follow blocks within a loop in RPO order, you can use the
> LoopIterator that I recently added for this purpose. It does need to
> be adapted for MachineLoopInfo.
> 
> I'll look into LoopIterator. If it were to get adapted for MachineLoopInfo, that would certainly help. =D
> 
> I wonder however, the Loop object already has a vector of BB*; it would seem like it could just place that vector in RPO, and be done with it? Dunno, as I've not looked at LoopIterator, etc.

Totally agreed. I don't think we can guarantee the Loop's blocks are RPO at all times, but a particular pass could sort the blocks up front. We have to be careful that some underlying CFG utility doesn't invalidate the order. But giving LoopIterator a separate blocks list doesn't really solve that problem, just hides it.

If it will help you, I can throw together a patch to provides a MachineLoopIterator. That should be straightforward. Later, if need be, someone can modify it to sort the original Loop blocks instead of maintaining its own list as an optimization.

-Andy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111026/81b57553/attachment.html>