[PATCH] Fix for inlining decision affected by debug intrinsics

[Dario Domizioli]

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.

Cheers,
    Dario Domizioli
    SN Systems - Sony Computer Entertainment






On 31 January 2014 21:06, Robinson, Paul <Paul_Robinson at playstation.sony.com
> wrote:

> > -----Original Message-----
> > From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> > bounces at cs.uiuc.edu] On Behalf Of Eric Christopher
> > Sent: Friday, January 31, 2014 12:10 PM
> > To: Chandler Carruth
> > Cc: Commit Messages and Patches for LLVM
> > Subject: Re: [PATCH] Fix for inlining decision affected by debug
> > intrinsics
> >
> > I'd rather ignore all non-code generating heuristics myself -
> > including the lifetime heuristics. If they're not going to affect the
> > size of the generated code then they shouldn't be considered. Also,
> > debug shouldn't affect optimization decisions. :)
>
> Right, that's what prompted this in the first place.  This is by
> far the most egregious case that we've found, so far the others
> have been minor, along the lines of PR18590.
> The ideal is that -g doesn't affect the final machine code at all,
> of course.
> --paulr
>
> >
> > -eric
> >
> > On Fri, Jan 31, 2014 at 12:06 PM, Chandler Carruth
> > <chandlerc at google.com> wrote:
> > >
> > > On Fri, Jan 31, 2014 at 10:55 AM, Dario Domizioli
> > > <dario.domizioli at gmail.com> wrote:
> > >>
> > >> 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 don't think this bug has anything to do with debug info intrinsics,
> > and so
> > > I don't think this is the correct patch.
> > >
> > > I think the bug is that instructions which we simplify away are
> > counted
> > > toward the vector bonus. There should be no difference between debug
> > info
> > > intrinsics and lifetime intrinsics. So the fix should be general and
> > apply
> > > to the the basic cost.
> > >
> > >
> > > However, as a meta point, the "vector bonus" is the single worst
> > aspect of
> > > our inline heuristics. To an extent, I would rather *remove* the
> > vector
> > > bonus than make it more correct... but perhaps that battle is best
> > fought
> > > another day.
> > >
> > > _______________________________________________
> > > 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
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140131/2dfa0727/attachment.html>