[llvm-dev] Removing LLVM_ALWAYS_INLINE from ADT classes

[Davide Italiano via llvm-dev]

On Thu, Jan 10, 2019 at 3:57 PM Davide Italiano <davide at freebsd.org> wrote:
>
> 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
>

After yet another round of discussions, the plan is that of trying not
to slap another attribute on the members, instead going for making
OPTIMIZED_TLBGEN the default and removing always_inline.
I'll do some testing locally (for the Ninja and the Xcode build
generator). Pawel volunteered to take a look at the Visual Studio
situation.
I assume this is going to be the final plan we'll deploy unless
somebody comes with any fundamental objections or there are technical
hurdles down the road.

Thanks,

--
Davide