llvm dev - Jan 2019 - Removing LLVM_ALWAYS_INLINE from ADT classes

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.

After discussing we agreed that it's reasonable to drop
LLVM_ALWYAS_INLINE from ADT classes  member functions and add a note
in the developer's manual.

If you have any feedbacks or concerns, please speak up.
If nothing major arises, I'm going to commit this patch in a week (or such).

Thanks for your time,

-- 
Davide

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

This makes sense to me.

One concern is that this in itself will slow down the build, since tablegen will
get even slower.  Ideally, there would be some (perhaps default?) configuration
where we build the tablegen binaries with optimizations on and then use them in
the build, as if we were cross-compiling.
> On Jan 4, 2019, at 15:14, 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.
> 
> After discussing we agreed that it's reasonable to drop
> LLVM_ALWYAS_INLINE from ADT classes  member functions and add a note
> in the developer's manual.
> 
> If you have any feedbacks or concerns, please speak up.
> If nothing major arises, I'm going to commit this patch in a week (or
such).
> 
> Thanks for your time,
> 
> -- 
> Davide
> 
> "There are no solved problems; there are only problems that are more
> or less solved" -- Henri Poincare
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

+1, I think that’s a great change.
> On Jan 4, 2019, at 3:36 PM, Duncan P. N. Exon Smith via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> 
> This makes sense to me.
> 
> One concern is that this in itself will slow down the build, since tablegen
will get even slower.  Ideally, there would be some (perhaps default?)
configuration where we build the tablegen binaries with optimizations on and
then use them in the build, as if we were cross-compiling.
> 
>> On Jan 4, 2019, at 15:14, 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.
>> 
>> After discussing we agreed that it's reasonable to drop
>> LLVM_ALWYAS_INLINE from ADT classes  member functions and add a note
>> in the developer's manual.
>> 
>> If you have any feedbacks or concerns, please speak up.
>> If nothing major arises, I'm going to commit this patch in a week
(or such).
>> 
>> Thanks for your time,
>> 
>> -- 
>> Davide
>> 
>> "There are no solved problems; there are only problems that are
more
>> or less solved" -- Henri Poincare
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

One minor correction, this isn't about ADT.

There are *numerous* places where we've tried to mitigate -O0 test runtime
with these attributes that will end up cleaned up based on this direction,
not just in ADT. =D

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.
>
> After discussing we agreed that it's reasonable to drop
> LLVM_ALWYAS_INLINE from ADT classes  member functions and add a note
> in the developer's manual.
>
> If you have any feedbacks or concerns, please speak up.
> If nothing major arises, I'm going to commit this patch in a week (or
> such).
>
> Thanks for your time,
>
> --
> Davide
>
> "There are no solved problems; there are only problems that are more
> or less solved" -- Henri Poincare
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190104/b73cb97f/attachment.html>

On Fri, Jan 4, 2019 at 3:42 PM Chandler Carruth <chandlerc at gmail.com>
wrote:
>
> One minor correction, this isn't about ADT.
>
> There are *numerous* places where we've tried to mitigate -O0 test
runtime with these attributes that will end up cleaned up based on this
direction, not just in ADT. =D
>
Sure, I'll do these next.
FWIW, I started from the ADT ones because they've been the one disturbing
me.
There are other few instances, grouped in three files:
1) Chrono.h
2) SelectionDAGISel.cpp
3) User.h

Admittedly the only other one I hit was 3). I don't work on backends a
lot so maybe 2) is really annoying but I just never experienced it.
> 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.
>>
>> After discussing we agreed that it's reasonable to drop
>> LLVM_ALWYAS_INLINE from ADT classes  member functions and add a note
>> in the developer's manual.
>>
>> If you have any feedbacks or concerns, please speak up.
>> If nothing major arises, I'm going to commit this patch in a week
(or such).
>>
>> Thanks for your time,
>>
>> --
>> Davide
>>
>> "There are no solved problems; there are only problems that are
more
>> or less solved" -- Henri Poincare
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev


-- 
Davide

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

On Sat, Jan 5, 2019 at 12:38 AM Duncan P. N. Exon Smith via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> This makes sense to me.
>
> One concern is that this in itself will slow down the build, since
> tablegen will get even slower.  Ideally, there would be some (perhaps
> default?) configuration where we build the tablegen binaries with
> optimizations on and then use them in the build, as if we were
> cross-compiling.
>
Maybe something simple like -DLLVM_OPTIMIZE_TABLEGEN=ON (enabled by default
for Debug builds). You would need to set it to off only if you want to
debug tablegen.

>
> > On Jan 4, 2019, at 15:14, 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.
> >
> > After discussing we agreed that it's reasonable to drop
> > LLVM_ALWYAS_INLINE from ADT classes  member functions and add a note
> > in the developer's manual.
> >
> > If you have any feedbacks or concerns, please speak up.
> > If nothing major arises, I'm going to commit this patch in a week
(or
> such).
> >
> > Thanks for your time,
> >
> > --
> > Davide
> >
> > "There are no solved problems; there are only problems that are
more
> > or less solved" -- Henri Poincare
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190105/942d9957/attachment.html>

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)).

-- 
Mehdi




>
> After discussing we agreed that it's reasonable to drop
> LLVM_ALWYAS_INLINE from ADT classes  member functions and add a note
> in the developer's manual.
>
> If you have any feedbacks or concerns, please speak up.
> If nothing major arises, I'm going to commit this patch in a week (or
> such).
>
> Thanks for your time,
>
> --
> Davide
>
> "There are no solved problems; there are only problems that are more
> or less solved" -- Henri Poincare
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190109/9b2efa07/attachment.html>

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