[cfe-dev] Optionally suppress debug info for inlined calls?

[David Blaikie]

On Wed, Nov 27, 2013 at 5:29 PM, Robinson, Paul
<Paul_Robinson at playstation.sony.com> wrote:
>> -----Original Message-----
>> From: David Blaikie [mailto:dblaikie at gmail.com]
>> Sent: Wednesday, November 27, 2013 3:36 PM
>> To: Robinson, Paul
>> Cc: Eric Christopher; Sean Silva; cfe-dev at cs.uiuc.edu
>> Subject: Re: [cfe-dev] Optionally suppress debug info for inlined calls?
>>
>> On Wed, Nov 27, 2013 at 2:08 PM, Robinson, Paul
>> <Paul_Robinson at playstation.sony.com> wrote:
>> >> > > Dropping the information at such a coarse granularity as "all
>> inline
>> >> > > functions" seems a bit, well, rough.
>> >> >
>> >> > No argument there.  Note that it isn't actually what our licensees
>> >> > asked for; it's what was low-cost to provide. Something better is
>> >> okay.
>> >> > I am trying to head in that direction, however badly I am able to
>> >> > express that.
>> >>
>> >> OK - that's somewhat of what I'm trying to discuss. It /seems/ like
>> >> your user requests aren't actually unique to your userbase. They seem
>> >> like common things anyone using a debugger might like - not to step
>> >> into/out of trivial functions.
>> >>
>> >> So I'd like to think about whether there's a good, general,
>> >> on-by-default behavior we can use here.
>> >
>> > I don't doubt we could come up with some sort of heuristic that would
>> > make sense to us as compiler developers (e.g.: fewer than X
>> instructions
>> > with at most 1 conditional branch).
>> >
>> > I am doubtful, however, that any such heuristic will conform to what
>> > a colleague once called "the principle of least surprise."  That is,
>> > it will not be easy to explain to users how that heuristic influences
>> > whether they can step into a "trivial" function.  Having the debugger
>> > step into or over function calls, based on mysterious compiler-
>> internal
>> > criteria rather than something the user can readily discern, is not
>> > such a good experience.
>>
>> I'm not so sure that's the case - and realistically I suspect this
>> would have to be a debugger feature (perhaps powered by new/improved
>> information in the DWARF to help the debugger make the right choice)
>> so that it could be overridden when necessary.
>>
>> I'm not sure such a heuristic exists, but it's something that seems
>> worth considering.
>
> I can envision some sort of debugger feature that could provide control
> over the stepping behavior, preferably based on debug info and really
> preferably not based on analyzing function content.  Although debuggers
> are getting more clever these days, it might not be too painful to come
> up with something based on instruction counts and/or branch counts.
> So, yeah, it wouldn't have to be a compiler thing.

The piece where the compiler might be helpful is to give some hints in
the debug info (in the same way that improving the AT_inline attribute
fidelity could provide a basis for a debugger feature) - such as "this
is just a return of a single variable" - instruction counts might be
insufficient as their could be a non-trivial copy ctor, etc, in there.

>> >> > > If we could find a good heuristic
>> >> > > for "trivial inline functions" (maybe a function with a single
>> >> setter
>> >> > > or return) and use that as our signal for "don't bother emitting
>> >> debug
>> >> > > info for this" maybe that would be good. That would still provide
>> >> the
>> >> > > size wins with the benefit that it might be beneficial to a
>> broader
>> >> > > range of users.
>> >> >
>> >> > How about "any indication in the source that the programmer wanted
>> it
>> >> > inlined."
>> >>
>> >> I don't think that's sufficient - looking at Clang and LLVM there are
>> >> a variety of header-defined functions that are sufficiently
>> >> non-trivial as to be interesting to step through. Though perhaps, if
>> >> it were a debugger-feature, it would be sufficient to make that the
>> >> default and a debugger would have some feature to allow a user to
>> >> override it. My suspicion is that the debugger might need to do more
>> >> work to make a better decision there, though. But I'm not sure the
>> >> compiler can really give much more info - it'd essentially be up to
>> >> the debugger to examine a function, perhaps, and decide whether it
>> was
>> >> "interesting".
>> >
>> > I offer that heuristic because it is easy to explain, easy to
>> remember,
>> > and easy for users to apply when eyeballing their own code.  Not
>> because
>> > it conforms to some compiler-developer-friendly abstraction of the
>> > dividing line between "trivial" and "interesting."
>>
>> I'm not sure I follow what you mean by "compile-developer-friendly
>> abstraction" - whether you're referring to my using the LLVM codebase
>> as an example or you're describing my attitude, regardless of example.
>
> No, no, no, David, please.  I mean that any heuristic that the compiler
> applied to this problem would necessarily be calculated on some basis
> that the compiler (i.e. the developer teaching the compiler to do this
> trick) would find convenient to work with.  This basis is unlikely to
> be readily expressed in terms relevant to the programmer, although it
> is clearly a compiler-developer-friendly basis.
>
> It's analogous to costing for an inlining heuristic; the criteria aren't
> easily reflected back to the user in terms that the user can readily
> apply to modifying his own code to improve the inlining behavior.  The
> criteria are implemented in compiler-developer-friendly terms, not in
> game-developer-friendly terms. [This is actually something else that our
> guys have asked for: diagnostics explaining why the compiler didn't
> inline some function.  I've dragged my feet for years on that one,
> because I don't know how to express it in terms that would make any sense
> to a non-compiler-writer.  Numbers of LLVM IR instructions??  No, I don't
> think so.]

When you say "compiler" it sounds like you mean "backend" (LLVM, etc)
- Clang is quite good at describing things in terms of source
constructs the programmer understands, so it seems like we could
potentially have Clang encode some "triviality" information (this
function is simply "return x;" or "this->x = x", etc - or number of
C++ subexpressions (expression tree size)) that the we'd pass down to
LLVM, emit into debug info, and then the debugger could use that as a
way to choose the default filtering for stepping behavior (backtraces
seem like they'd be useful to have these frames in regardless - you
want to know where you are, missing frames never seems like the Right
Thing to do - is it?) that could be overridden by the user when they
want to look more closely.

> Now, we could probably devise some criteria that were still easy to
> explain.  But it would still be the compiler deciding something, and
> whatever basis we picked, somebody would want to fiddle with it, and
> we probably wouldn't want to give them that knob to twist.  This is
> another reason why I think "marked inline" is a reasonable criterion:
> The programmer can't really argue with it.  "How come I can't step into
> this function?" "Because you said not to." "... oh yeah."
>
> (And if we ever get 'optnone' finished, they can apply 'optnone' to
> any inlined function that's causing a problem, to keep it from being
> inlined, and therefore making it feasible to debug.  Granted, with a
> postulated debugger feature to turn auto-step-over on and off per
> function, they wouldn't have to rebuild their program.  The thing is,
> the compiler features are basically ready *right* *now*.  This is not
> to say they have to go upstream, they're just ready to roll and I'm
> checking to see if upstream is the right direction.)
>
>>
>> I think LLVM isn't a very strange/uncommon example of C++ code as
>> regards to the inline function complexity.
>>
>> And, yes, likely this would be better at the debugger feature than a
>> compiler feature - so it can be turned off/on/configured by the user.
>>
>> >
>> >>
>> >> >  That is: (i) method defined inside the class declaration;
>> >> > (ii) 'inline' keyword; (iii) an attribute that means "inline this."
>> >> > All of these are indications that the programmer wanted the
>> function
>> >> > inlined, and if the programmer then builds the program using the
>> >> > proposed -gno-inlined-scopes feature, it should not be shocking to
>> >> > anyone that these inlined functions don't have debug info.  (Sorry
>> to
>> >> > be proposing something that isn't shocking; it's all I got.)
>> >>
>> >> Right - what I'm suggesting is that there is an underlying
>> >> feature/usability desire that should be on by default and I'm trying
>> >> to think/talk through how we might provide a better experience to all
>> >> our users by default, rather than with an explicit switch. (and while
>> >> some of your users explicitly requested this behavior, some didn't -
>> >> so they're not all going to be in the "the programmer explicitly
>> >> turned on this switch, of course they got that behavior" - they won't
>> >> know the switch is on, so the behavior may catch them by surprise)
>> >>
>> >> > This is what our licensees actually asked for and I am not averse
>> to
>> >> > giving it to them. :-)
>> >>
>> >> They asked for a switch powered by inline hints? Fair enough -
>> though,
>> >> again, I'm interested in seeing whether we can design a feature
>> better
>> >> than our customer requests that addresses their underlying issue (&
>> >> perhaps that of others who haven't even asked for anything yet).
>> >
>> > yes, the criterion of "marked inline" came from them, not me.
>>
>> OK - so what was their request? Just that they don't step into
>> "uninteresting" functions & you chose inline as the heuristic to use?
>
> No, _they_ defined "uninteresting" functions as the ones marked inline
> (explicitly in source, or implicitly by being defined inside the class
> declaration).  The compiler-developer-friendly solution was to have the
> compiler not emit scopes for ANY inlined functions.  Because it was easy
> to implement that exact feature in LLVM, and DW_AT_inline was lying to
> us, and it's how the compilers for our previous platforms solved the same
> problem.
>
> Given a truthful DW_AT_inline, I might have pushed back on the
> debugger people to take advantage of it, but I can't do that until
> DW_AT_inline tells the truth.  It was a lot easier to find where the
> scopes came out, and stop them, than it was to figure out how to get
> the right "marked inline" indicator out of LLVM.  Plus when we stopped
> putting out the scopes, we got a big debug-info-size windfall along
> with it; and there was much rejoicing.

Reiterating: I am one voice here. I do not speak for anyone else or
any group - I just wanted to discuss this in a bit more detail to
tease out the pieces of the puzzle, motivations, etc.

> So, it seemed worthwhile asking if this was of any interest upstream.
> What I'm getting out of this discussion is:
> - Not stepping into trivial/uninteresting functions seems like a good
>   debugging-experience benefit.

Agreed.

> - We defined criteria for picking that set of functions at compile time,
>   and there is a reasonable basis for saying those criteria aren't
>   really ideal.  There is a reasonable basis for saying NO criteria
>   defined by the compiler are really ideal.

Agreed.

> - The long-term goal of better DW_AT_inline info is definitely good.

Yep - seems useful regardless of anything else. It doesn't cost us
debug info size (we're already emitting the attribute) and might
enable some debugger features.

> - If the debugging-experience improvement can be achieved by the
>   debugger, rather than the compiler adding or subtracting debug info,
>   that's likely the better solution.

Right, due to your second point (that no compiler heuristic is going
to be bulletproof).

(though on second thoughts I do kind of wonder about whether we could
get a good enough conservative heuristic that was Just Right - but
think it'd still be better implemented in the debugger anyway (because
even the simplest get/set member functions could crash when 'this' is
null and it'd be important to have the get/set function in the
backtrace, so we can't drop the info entirely) at which point whether
the debugger has a nob to fiddle with it or not isn't too important to
us)

> I think that leaves me continuing on the path I outlined before, but as
> private features,

While I'm personally still a bit hesitant to see merit in your patches
upstream - I'm hardly the authority/owner here, just a concerned
citizen. If your patches are easy to separate, it might be worth
posting them so we can see what the real cost is that we're
considering.

> until we get to the point of a truthful DW_AT_inline.

Even here, omitting the inlining info with a compiler flag leaves me a
bit twitchy, but might be the right thing. It may be worth talking to
your debugger team about getting the debugger-level feature for
step-avoidance-on-user-requested-inlining (which could even work when
the function wasn't inlined, of course - whereas a
compiler-implemented feature couldn't provide that functionality so
far as I know) going so it'd be ready sooner/as you implement the
improvements to DW_AT_inline emission.

- David