[cfe-dev] Varying per function optimisation based on include path?

[Arthur O'Dwyer via cfe-dev]

On Wed, Aug 21, 2019 at 1:40 AM John McFarlane <john at mcfarlane.name> wrote:
> On Tue, 20 Aug 2019 at 19:34, Ben Craig <ben.craig at ni.com> wrote:
>>
>> I think a question was glossed over.  Exactly which directions should be
inlined…
>>
>> User callee into user caller (definitely not)
>> System callee into system caller (yes)
>> User callee into system caller (maybe?)
>> System callee into user caller (maybe?)
>>
>> Perhaps number 3 should be prohibited because then a breakpoint on “my”
function would either not get hit, or turn into multiple breakpoints.
>> Perhaps number 4 should be prohibited because it makes stepping across
loop iterations in things like std::transform more difficult.
>
> I deliberately don't bring up call direction because I don't think it's
important and I want to keep things simple:
> - If the code is user code, it should not be inlined.
> - If the code is system code (including non-standard 3rd-party
dependencies), it should be inlined.

I think Ben Craig's point about call direction is important. Or at least,
his intuition about what you meant matches my intuition, whereas the words
you just used in the paragraph above do *not* match my intuition about what
you had meant in P1832.

> Take a hypothetical call stack from this example program:
>
> ?: std::strncmp(....) <- C standard library function, possibly an
intrinsic, inline
> ?: std::string::operator==(char const*)  <- standard library function,
inline
> 8: [](std::string const&) {....} <- lambda, don't inline
> ?: std::find_if(....) <- standard library taking lambda, inline
> 4: contains_sg15 <- user function containing lambda, don't inline
> 13: main <- user's main function, don't inline
>
> I think whether to inline the lambda is mildly contentious but I think we
both agree
> it should not be inlined. The real question is whether to inline
`std::find_if`. I guess
> you're suggesting we don't inline it. I think -- on balance -- we
probably should.

The verb "inline" doesn't operate on a *node* of this graph; it operates on
an *edge* between two nodes. My intuition is that -Og should enable
inlining on every edge where *both* endpoint nodes are in "system code,"
and on no other edges. With your above callstack:

Yes inline strncmp into operator==  (system into system)
No don't inline operator== into the user's lambda  (system into user)
No don't inline the user's lambda into std::find_if  (user into system)
No don't inline std::find_if into contains_sg15  (system into user)
No don't inline contains_sg15 into main  (user into user)

My intuition is that we don't want to inline system code into user code
because that would mix "not-my-code" into the codegen for "my code."
Imagine the user trying to step through `contains_sg15`, and suddenly
they're seeing backward branches and loops. Where did those loops come
from? Ah, the compiler inlined a bunch of crap from <algorithm>. That's not
the debugging experience we want (says my intuition).  So we should *not*
inline `std::find_if` into `contains_sg15`.

What about inlining the user's lambda into std::find_if?  I think we have
always agreed on this one.  The user must be able to set a single
breakpoint in the lambda, and run `contains_sg15`, and see the breakpoint
get hit. If there's a second copy of the lambda's code inlined into
`std::find_if`, then this scenario falls apart. So, we should *not* inline
the user's lambda into `std::find_if`. In fact, the same argument implies
that we should never inline user code into *anywhere*. One line of user
source code should continue to map to one point in the compiled binary.


> Let's step through this program and let's assume that I only use the
"step into"
> functionality of the debugger, because that's an easy way to 'visit' an
entire
> program. Let's assume `find_if` is a single function.

I think you have to consider the workflow for people who use the
"disassemble" functionality of the debugger, as well.

Also, I'm not sure how the "step into/ single-step" functionality works,
but I would have thought that it just means "continue forward until the
current instruction is no longer associated with the same source line of
code." Which means that it would still be possible to "step into" system
code, because system code does still have line numbers associated with it.

I intuit that it should be easy to "step *over*" calls to library
functions; which means that you must ensure that the call to `std::find_if`
shows up as a call instruction in the binary. If we inline `std::find_if`
into `contains_sg15`, then you can't "step over" it anymore.

–Arthur
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190824/eab863da/attachment.html>