[llvm-commits] [PATCH] A fix for the inliner heuristics for recessive calls
Chandler Carruth
chandlerc at google.com
Sun Sep 16 12:14:52 PDT 2012
On Sun, Sep 16, 2012 at 5:16 AM, Nadav Rotem <nrotem at apple.com> wrote:
> Hi,
>
> I am looking at a test case that looks like this:
>
> void foo() {
> ... foo(); // I'm recursive!
>
> bar();
> }
>
> bar() { int a[1000]; // large stack size }
>
> If we decide to inline bar() into foo() we may increase the stack size to
> the point of overflow. I attached a patch to prevent inlining of callees
> which allocate large arrays into a recursive caller.
>
I don't think this patch is the right way to solve the issue. This isn't an
inliner issue, this is an inline cost issue, and I think you should sink
the analysis to be part of the cost computation. For example, if someone
marks 'bar' in your example as always-inline, they should get what they
asked for.
I also agree with Hal that we need to use target data when we have it to
make this call. I think the inline cost already deals with this, but I'd
have to look at the code.
My other concern is that I think this is way too aggressive. The mere
presence of such an alloca shouldn't disable inlining. Consider these
scenarios:
- What if inlining is the thing that allows the array to be broken up by
SROA, and most of the dead sections removed?
- What if after inlining all uses of the array are deleted as dead code?
- What if after inlining 'foo' is no longer recursive?
I think by doing a much more detailed analysis within the inline cost
framework (which already does per-instruction analysis) can solve at least
the first two issues by making the logic be to not inline if:
1) We have an alloca with a static size greater than X, and
2) It is escaped (for the purposes of SROA) along the conservatively live
code path
I think you can assume that if SROA will fire for the alloca after inlining
(likely due to only a subset of the function being live), the inlining is
fine. The instruction threshold should bound the amount of post-split stack
space growth.
I don't have any ideas about how to catch the third problem above, but that
one seems the least likely to be a problem in practice.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120916/b1ee3241/attachment.html>
More information about the llvm-commits
mailing list