[PATCH] Fix for inlining decision affected by debug intrinsics

[Eric Christopher]

Hold off on committing, Chandler said he found something subtle...

-eric

On Fri, Jan 31, 2014 at 12:00 PM, Dario Domizioli
<dario.domizioli at gmail.com> wrote:
> Sorry, 'patch' seems to have made a mess with the line numbers for the
> test...
> This should be the correct "_fix" patch.
>
> Thanks again!
>
>     Dario Domizioli
>     SN Systems - Sony Computer Entertainment
>
>
>
>
>
>
>
>
> On 31 January 2014 19:51, Dario Domizioli <dario.domizioli at gmail.com> wrote:
>>
>> Thanks for the quick review!
>>
>> I have now split the patch into two diff files, each rebased on r200567.
>> The "_fix" one contains the fix and the test, the "_debug" one contains
>> the debug prints.
>>
>> Let me know if there's any issue, otherwise feel free to commit.
>>
>> Cheers,
>>     Dario Domizioli
>>     SN Systems - Sony Computer Entertainment
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> On 31 January 2014 19:24, Eric Christopher <echristo at gmail.com> wrote:
>>>
>>> Agreed with what Manman says. Let's split it in two and then either of
>>> us can commit for you.
>>>
>>> Thanks!
>>>
>>> -eric
>>>
>>> On Fri, Jan 31, 2014 at 11:05 AM, Manman Ren <manman.ren at gmail.com>
>>> wrote:
>>> >
>>> > LGTM. Thanks for the detailed comments.
>>> >
>>> > I noticed a few debugging-related changes, it may be better to separate
>>> > those.
>>> > +  DEBUG_PRINT_STAT(Cost);
>>> > +  DEBUG_PRINT_STAT(Threshold);
>>> > +  DEBUG_PRINT_STAT(VectorBonus);
>>> >
>>> > Manman
>>> >
>>> >
>>> > On Fri, Jan 31, 2014 at 10:55 AM, Dario Domizioli
>>> > <dario.domizioli at gmail.com> wrote:
>>> >>
>>> >> Hello LLVM.
>>> >>
>>> >> I have come across a subtle and rare interaction between debug
>>> >> information, vector instructions, and the cost heuristic used by the
>>> >> SimpleInliner (i.e. the InlineCost analysis).
>>> >>
>>> >> It appears that debug intrinsics are counted in the number of
>>> >> instructions
>>> >> taken into consideration by the heuristic. They are also later
>>> >> discounted as
>>> >> being "simplified", so they do not directly affect the inline Cost,
>>> >> but they
>>> >> are taken into account when evaluating the fraction of instructions
>>> >> that are
>>> >> vector instructions. This fraction is used to determine whether to
>>> >> apply a
>>> >> hidden "VectorBonus" to the inlining Threshold, and therefore there is
>>> >> a
>>> >> subtle interaction which means that debug intrinsics can affect the
>>> >> inlining
>>> >> decision.
>>> >>
>>> >> The attached patch fixes the problem by making the inline cost
>>> >> heuristic
>>> >> skip debug intrinsics altogether when examining instructions.
>>> >> I have also added a couple of debug prints to the already existing
>>> >> ones,
>>> >> which I think are generally useful to debug the inline cost heuristic.
>>> >>
>>> >> I have included a test in which I have manufactured a situation where
>>> >> vector instructions would be most of the function's code but they are
>>> >> outnumbered by debug intrinsics when debug information is present. The
>>> >> test
>>> >> exposes the problem in the current trunk and passes with my fix.
>>> >>
>>> >> Please submit for me if you accept this patch, as I do not have commit
>>> >> access.
>>> >>
>>> >> Cheers,
>>> >>     Dario Domizioli
>>> >>     SN Systems - Sony Computer Entertainment
>>> >>
>>> >>
>>> >> _______________________________________________
>>> >> llvm-commits mailing list
>>> >> llvm-commits at cs.uiuc.edu
>>> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> >>
>>> >
>>> >
>>> > _______________________________________________
>>> > llvm-commits mailing list
>>> > llvm-commits at cs.uiuc.edu
>>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> >
>>
>>
>