[llvm-commits] [llvm] r61946 - in /llvm/trunk: include/llvm/Transforms/Utils/InlineCost.h lib/Transforms/Utils/InlineCost.cpp test/Transforms/Inline/2009-01-08-NoInlineDynamicAlloca.ll test/Transforms/Inline/dynamic_alloca_test.ll test/Transforms/PruneEH/2008-09-05-CGUpdate.ll

[Chris Lattner]

On Jan 8, 2009, at 1:45 PM, Dale Johannesen wrote:

> Author: johannes
> Date: Thu Jan  8 15:45:23 2009
> New Revision: 61946
>
> URL: http://llvm.org/viewvc/llvm-project?rev=61946&view=rev
> Log:
> Do not inline functions with (dynamic) alloca into
> functions that don't already have a (dynamic) alloca.
> Dynamic allocas cause inefficient codegen and we shouldn't
> propagate this (behavior follows gcc).  Two existing tests
> assumed such inlining would be done; they are hacked by
> adding an alloca in the caller, preserving the point of
> the tests.

Nice!

>
> +      if (const AllocaInst *AI = dyn_cast<AllocaInst>(II)) {
> +        if (!isa<ConstantInt>(AI->getArraySize()))
> +          this->usesDynamicAlloca = true;

It's a very minor thing, but i'd prefer: if (!AI->isStaticAlloca()).   
This will handle fixed-size allocas that are not in the entry block  
also.


> +  // Get infomation about the caller...
> +  FunctionInfo &CallerFI = CachedFunctionInfo[Caller];
> +
> +  // If we haven't calculated this information yet, do so now.
> +  if (CallerFI.NumBlocks == 0)
> +    CallerFI.analyzeFunction(Caller);

Please only analyze the caller here if CalleeFI.usesDynamicAlloca is  
true.


Ok.  One risk of this is that this could cause inaccurate inlining  
costs to be computed (I think).  Is this scenario possible?:

1. foo calls bar.  Bar has a dynamic alloca, so we call  
analyzeFunction on foo.
2. foo calls baz, we inline baz into foo.
3. gar calls foo, when analyzing the call to foo in gar, we look at  
the inlining cost of foo.

Does the inlining cost of foo in #3 include the fact that baz got  
inlined into foo?

If this is not already being handled, one good way to do this is to  
have the "is analyzed" state for a function get cleared whenever  
another function is inlined into it.

Thanks for working on this Dale, this is a great change,

-Chris