llvm dev - Jan 2019 - Removing LLVM_ALWAYS_INLINE from ADT classes

On Wed, Jan 9, 2019 at 9:38 AM Mehdi AMINI <joker.eph at gmail.com>
wrote:
>
>
>
> On Fri, Jan 4, 2019 at 3:15 PM Davide Italiano via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>>
>> Hi,
>> I would like to propose, based on a previous discussion on llvm-dev,
>> the following change.
>> https://reviews.llvm.org/D56337
>>
>> The main motivation for annotating member functions of ADT clases with
>> LLVM_ALWAYS_INLINE  was that of speeding up `check-llvm` at `-O0`.
>> Turns out this significantly degrades the debuggability of fundamental
>> classes in llvm itself, e.g. StringRef or SmallVector.
>
>
> It seems that the issue is not a problem with the inlining these functions
but rather that they aren't emitted when inlined and so can't be called
from the debugger.
>
> An alternative that preserves the inlining for the performance aspect *and*
still emit them in debug build so that they are available in debug builds would
be another macro that would expand to something like __attribute__((used)).
>
I need to sort out the details, but I like your idea (also, nullifies
some of the raised objections).
I'll try and let you know how it works out.

Thanks,

--
Davide

On Wed, Jan 9, 2019 at 2:18 PM Davide Italiano <davide at freebsd.org>
wrote:
>
> On Wed, Jan 9, 2019 at 9:38 AM Mehdi AMINI <joker.eph at gmail.com>
wrote:
> >
> >
> >
> > On Fri, Jan 4, 2019 at 3:15 PM Davide Italiano via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> >>
> >> Hi,
> >> I would like to propose, based on a previous discussion on
llvm-dev,
> >> the following change.
> >> https://reviews.llvm.org/D56337
> >>
> >> The main motivation for annotating member functions of ADT clases
with
> >> LLVM_ALWAYS_INLINE  was that of speeding up `check-llvm` at `-O0`.
> >> Turns out this significantly degrades the debuggability of
fundamental
> >> classes in llvm itself, e.g. StringRef or SmallVector.
> >
> >
> > It seems that the issue is not a problem with the inlining these
functions but rather that they aren't emitted when inlined and so can't
be called from the debugger.
> >
> > An alternative that preserves the inlining for the performance aspect
*and* still emit them in debug build so that they are available in debug builds
would be another macro that would expand to something like
__attribute__((used)).
> >
>
Your suggestion works just fine. I'll update the patch.

* thread #1, queue = 'com.apple.main-thread', stop reason = step over
    frame #0: 0x000000010000a110 lldb`main(argc=1,
argv=0x00007ffeefbff9c8) at Driver.cpp:872
   869
   870  // Print stack trace on crash.
   871  llvm::StringRef ToolName = llvm::sys::path::filename(argv[0]);
-> 872  llvm::sys::PrintStackTraceOnErrorSignal(ToolName);
   873  llvm::PrettyStackTraceProgram X(argc, argv);
   874
   875  // Parse arguments.

(lldb) p ToolName.size()
(size_t) $2 = 4
(lldb) p ToolName.consume_front(llvm::StringRef("l"))
(bool) $4 = true
(lldb) p ToolName.consume_front(llvm::StringRef("a"))
(bool) $5 = false
(lldb) p ToolName.slice(1,2)
(llvm::StringRef) $3 = (Data = "ldb", Length = 1)

Thank you!

On Thu, Jan 10, 2019 at 1:20 PM Davide Italiano <davide at freebsd.org>
wrote:
>
> On Wed, Jan 9, 2019 at 2:18 PM Davide Italiano <davide at
freebsd.org> wrote:
> >
> > On Wed, Jan 9, 2019 at 9:38 AM Mehdi AMINI <joker.eph at
gmail.com> wrote:
> > >
> > >
> > >
> > > On Fri, Jan 4, 2019 at 3:15 PM Davide Italiano via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> > >>
> > >> Hi,
> > >> I would like to propose, based on a previous discussion on
llvm-dev,
> > >> the following change.
> > >> https://reviews.llvm.org/D56337
> > >>
> > >> The main motivation for annotating member functions of ADT
clases with
> > >> LLVM_ALWAYS_INLINE  was that of speeding up `check-llvm` at
`-O0`.
> > >> Turns out this significantly degrades the debuggability of
fundamental
> > >> classes in llvm itself, e.g. StringRef or SmallVector.
> > >
> > >
> > > It seems that the issue is not a problem with the inlining these
functions but rather that they aren't emitted when inlined and so can't
be called from the debugger.
> > >
> > > An alternative that preserves the inlining for the performance
aspect *and* still emit them in debug build so that they are available in debug
builds would be another macro that would expand to something like
__attribute__((used)).
> > >
> >
>
> Your suggestion works just fine. I'll update the patch.
>
> * thread #1, queue = 'com.apple.main-thread', stop reason = step
over
>     frame #0: 0x000000010000a110 lldb`main(argc=1,
> argv=0x00007ffeefbff9c8) at Driver.cpp:872
>    869
>    870  // Print stack trace on crash.
>    871  llvm::StringRef ToolName = llvm::sys::path::filename(argv[0]);
> -> 872  llvm::sys::PrintStackTraceOnErrorSignal(ToolName);
>    873  llvm::PrettyStackTraceProgram X(argc, argv);
>    874
>    875  // Parse arguments.
>
> (lldb) p ToolName.size()
> (size_t) $2 = 4
> (lldb) p ToolName.consume_front(llvm::StringRef("l"))
> (bool) $4 = true
> (lldb) p ToolName.consume_front(llvm::StringRef("a"))
> (bool) $5 = false
> (lldb) p ToolName.slice(1,2)
> (llvm::StringRef) $3 = (Data = "ldb", Length = 1)
>
> Thank you!
New review online: https://reviews.llvm.org/D56567

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare