[PATCH] Fix for inlining decision affected by debug intrinsics
Dario Domizioli
dario.domizioli at gmail.com
Sat Feb 1 08:50:42 PST 2014
Cool!
Thank you for committing the patches!
Dario Domizioli
SN Systems - Sony Computer Entertainment
On 1 February 2014 10:45, Chandler Carruth <chandlerc at google.com> wrote:
>
> On Fri, Jan 31, 2014 at 1:16 PM, Dario Domizioli <
> dario.domizioli at gmail.com> wrote:
>
>> Hm...
>>
>> I had considered changing the calculation so that the VectorBonus
>> fraction was based on the number of non-simplified instructions (i.e.
>> NumInstructions - NumInstructionsSimplified), but then I decided to go for
>> a less risky change because I wasn't sure what other kinds of instructions
>> ended up in the "simplified" bucket. And furthermore some vector
>> instructions may be simplified too, so the whole logic gets messy pretty
>> quickly. The existence of a "hidden bonus" is itself quite odd (and the
>> fact that it didn't appear in the debug prints made the investigation
>> harder - hence my debug change too), so I'd understand if you wanted to
>> wait and properly rework the inline heuristic.
>>
>> This specific change should only impact compilations with debug
>> information, and furthermore it would just realign their result to what the
>> correspondent non-debug compilation does. Non-debug builds are already
>> using the increased threshold and they inline all the functions that would
>> be affected by this change in debug builds, so this doesn't technically
>> increase our willingness to inline functions... it just makes sure that we
>> are coherent with and without -g. I see your general point about possible
>> regressions though.
>>
>> I don't know... I think this could work as an incremental change towards
>> properly fixing the VectorBonus issues, but that's just my opinion.
>>
>
> Yea, essentially that's the unhappy conclusion I came to (as well as Eric
> and others). Changing everything at once is too hard and the bug is indeed
> bad enough to warrant a quick fix. Applied your patch with a significantly
> simplified test case in r200609. (The test case simplifications are I think
> good because there is no need or desire for this debug information to emit
> as useful debug information, merely to clutter up the IR with it and ensure
> the optimizer does-the-right-thing.)
>
> Thanks for the fix!
> -Chandler
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140201/2b6fc093/attachment.html>
More information about the llvm-commits
mailing list