[llvm-commits] PATCH: Add a function splitting pass to LLVM which extracts cold regions into their own functions

[Andrew Trick]

On May 14, 2012, at 11:30 AM, Chandler Carruth <chandlerc at gmail.com> wrote:

> On Mon, May 14, 2012 at 11:23 AM, Andrew Trick <atrick at apple.com> wrote:
> 
> On May 5, 2012, at 12:54 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> > I wanted to start the initial review on a pass I've been working on. It's essentially doing hot/cold partitioning, cold region extraction, function splitting, or cold region outlining. Whatever terminology you would like, you get the idea. It's a really simple pass, built entirely out of all the existing infrastructure (some of which I've been cleaning up to let everything fall out this easily).
> >
> > While this won't do a whole lot of interesting things on its own, I'm preparing to tweak the inline cost heuristics in a way that will allow this to form the essence of partial inlining. We can run this before inlining, splitting large cold regions of code into separate functions, and then be able to inline the remainder more easily.
> >
> > I haven't run benchmarks, and this patch doesn't turn the pass on, I just want to get initial feedback, and get the code into the shape where I can put it in-tree, and then look at turning it on if benchmarks prove positive.
> 
> Chandler,
> 
> Very nice.
> 
> I'm dismayed that you depend on RegionInfo, but it is understandable. We could probably improve RegionInfo to be more efficient and self-contained (no DomFrontier), but I think we'd be much better off dropping RegionInfo and handling SEME regions. AFAIK CodeExtractor can already handle that. Is there any particular reason you need RegionInfo other than finding a connected set of blocks dominated by a cold block? Can FunctionSplitter just find its own regions?
> 
> I'm dismayed that RegionInfo is still in the tree and not being improved. ;] I think we should fix RegionInfo to be efficient and reasonably well implemented. It as *already* handling SEME regions except for the "ME" case of function returns (something that is trivial to fix).
> 
> Fundamentally, splitting needs to identify the region of code dominated by a particular basic block. That construct seems both general and extremely useful. I think we should have some generic infrastructure for computing it and using it, and RegionInfo seems as nice of an API as any.
> 
> Nadav pointed me at a paper describing a linear computation of regions, but I don't have time to implement it at the moment...
> http://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.31.5126 

There are two issues then, Region API vs. Region discovery.

A convenient API for visiting and traversing regions is nice, though by design it's never needed in LLVM. A problem with the existing RegionInfo API is that it is superficially limited to SESE--though you say otherwise. That may be right for some clients, but then those clients likely don't need the region discovery that it provides. They can probably get by with finding control equivalent regions, which is trivial if you have postdominators--walk down in the dom tree and up in the posdom tree at the same time. I should mention that we don't want postdominators in standard passes, which is why LLVM optimizations are designed so they don't depend on control equivalence.

In your case, you want SEME regions, so the current API is misleading. Worse, it seems to me the region discovery is outright wrong. I think you just want the largest dominator subtree rooted at a cold node (may also want to check for hot loops w/in the cold code). So:

- The RegionInfo API appears misleading to me for your usage.

- It's not clear to someone reviewing your pass what features of RegionInfo analysis you actually depend on.

- You're pulling in a heavy-weight analysis that no other standard pass will use.

None of these are issues for prototyping your pass. I'm just clarifying the high-level direction of CFG analysis in LLVM as I understand it.

Maybe you just want a neat Region iterator API utility that could be used by anyone doing region discovery, including RegionInfo. But you wouldn't need to pull in RegionInfo analysis.

-Andy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120514/569924b8/attachment.html>