[llvm-commits] [llvm] r83675 - in /llvm/trunk: include/llvm/Transforms/Utils/InlineCost.h lib/Transforms/IPO/Inliner.cpp lib/Transforms/Utils/InlineCost.cpp

[Daniel Dunbar]

On Sat, Oct 10, 2009 at 12:42 AM, Dale Johannesen <dalej at apple.com> wrote:
> Author: johannes
> Date: Fri Oct  9 16:42:02 2009
> New Revision: 83675
>
> URL: http://llvm.org/viewvc/llvm-project?rev=83675&view=rev
> Log:
> Use names instead of numbers for some of the magic
> constants used in inlining heuristics (especially
> those used in more than one file).  No functional change.
>
>
> Modified:
>    llvm/trunk/include/llvm/Transforms/Utils/InlineCost.h
>    llvm/trunk/lib/Transforms/IPO/Inliner.cpp
>    llvm/trunk/lib/Transforms/Utils/InlineCost.cpp
>
> Modified: llvm/trunk/include/llvm/Transforms/Utils/InlineCost.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/InlineCost.h?rev=83675&r1=83674&r2=83675&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Transforms/Utils/InlineCost.h (original)
> +++ llvm/trunk/include/llvm/Transforms/Utils/InlineCost.h Fri Oct  9 16:42:02 2009
> @@ -27,6 +27,14 @@
>   template<class PtrType, unsigned SmallSize>
>   class SmallPtrSet;
>
> +  namespace InlineConstants {
> +    // Various magic constants used to adjust heuristics.
> +    const int CallPenalty = 5;
> +    const int LastCallToStaticBonus = -15000;

Seems strange to call this a "Bonus", but have a the negative value be
the "bonus". Wouldn't it make more sense to name it penalty as well?
Or perhaps replace {Penalty,Bonus} with a more neutral word like Cost?

 - Daniel

> +    const int ColdccPenalty = 2000;
> +    const int NoreturnPenalty = 10000;
> +  }
> +
>   /// InlineCost - Represent the cost of inlining a function. This
>   /// supports special values for functions which should "always" or
>   /// "never" be inlined. Otherwise, the cost represents a unitless
>
> Modified: llvm/trunk/lib/Transforms/IPO/Inliner.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Inliner.cpp?rev=83675&r1=83674&r2=83675&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/IPO/Inliner.cpp (original)
> +++ llvm/trunk/lib/Transforms/IPO/Inliner.cpp Fri Oct  9 16:42:02 2009
> @@ -243,10 +243,11 @@
>       if (Cost2 >= (int)(CurrentThreshold2 * FudgeFactor2))
>         allOuterCallsWillBeInlined = false;
>
> -      // See if we have this case.  The magic 6 is what InlineCost assigns
> +      // See if we have this case.  We subtract off the penalty
>       // for the call instruction, which we would be deleting.
>       if (Cost2 < (int)(CurrentThreshold2 * FudgeFactor2) &&
> -          Cost2 + Cost - 6 >= (int)(CurrentThreshold2 * FudgeFactor2)) {
> +          Cost2 + Cost - (InlineConstants::CallPenalty + 1) >=
> +                (int)(CurrentThreshold2 * FudgeFactor2)) {
>         someOuterCallWouldNotBeInlined = true;
>         TotalSecondaryCost += Cost2;
>       }
> @@ -256,7 +257,7 @@
>     // be removed entirely.  We did not account for this above unless there
>     // is only one caller of Caller.
>     if (allOuterCallsWillBeInlined && Caller->use_begin() != Caller->use_end())
> -      TotalSecondaryCost -= 15000;
> +      TotalSecondaryCost += InlineConstants::LastCallToStaticBonus;
>
>     if (outerCallsFound && someOuterCallWouldNotBeInlined &&
>         TotalSecondaryCost < Cost) {
>
> Modified: llvm/trunk/lib/Transforms/Utils/InlineCost.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/InlineCost.cpp?rev=83675&r1=83674&r2=83675&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/InlineCost.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/InlineCost.cpp Fri Oct  9 16:42:02 2009
> @@ -132,12 +132,12 @@
>         // Calls often compile into many machine instructions.  Bump up their
>         // cost to reflect this.
>         if (!isa<IntrinsicInst>(II))
> -          NumInsts += 5;
> +          NumInsts += InlineConstants::CallPenalty;
>       }
>
>       // These, too, are calls.
>       if (isa<MallocInst>(II) || isa<FreeInst>(II))
> -       NumInsts += 5;
> +       NumInsts += InlineConstants::CallPenalty;
>
>       if (const AllocaInst *AI = dyn_cast<AllocaInst>(II)) {
>         if (!AI->isStaticAlloca())
> @@ -212,21 +212,21 @@
>   // make it almost guaranteed to be inlined.
>   //
>   if (Callee->hasLocalLinkage() && Callee->hasOneUse())
> -    InlineCost -= 15000;
> +    InlineCost += InlineConstants::LastCallToStaticBonus;
>
>   // If this function uses the coldcc calling convention, prefer not to inline
>   // it.
>   if (Callee->getCallingConv() == CallingConv::Cold)
> -    InlineCost += 2000;
> +    InlineCost += InlineConstants::ColdccPenalty;
>
>   // If the instruction after the call, or if the normal destination of the
>   // invoke is an unreachable instruction, the function is noreturn.  As such,
>   // there is little point in inlining this.
>   if (InvokeInst *II = dyn_cast<InvokeInst>(TheCall)) {
>     if (isa<UnreachableInst>(II->getNormalDest()->begin()))
> -      InlineCost += 10000;
> +      InlineCost += InlineConstants::NoreturnPenalty;
>   } else if (isa<UnreachableInst>(++BasicBlock::iterator(TheCall)))
> -    InlineCost += 10000;
> +    InlineCost += InlineConstants::NoreturnPenalty;
>
>   // Get information about the callee...
>   FunctionInfo &CalleeFI = CachedFunctionInfo[Callee];
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>