Disabling LLVM_ATTRIBUTE_ALWAYS_INLINE for development?

[modocache]

Hello all!

I find that using lldb to debug LLVM libraries can be super
frustrating, because a lot of LLVM classes, like the constructor for
StringRef, are marked LLVM_ATTRIBUTE_ALWAYS_INLINE. So when I attempt
to have lldb evaluate an expression that implicitly instantiates a
StringRef, I get 'error: Couldn't lookup symbols:
__ZN4llvm9StringRefC1EPKc'.

As an example, most recently this happened to me when playing around
with llvm::AttributeSet, having attached lldb to an opt built with
-DCMAKE_BUILD_TYPE="Debug":

   (lldb) e $AS.hasAttribute("myattr")
   error: Couldn't lookup symbols:
     __ZN4llvm9StringRefC1EPKc

Despite having built in a "Debug" configuration,
LLVM_ATTRIBUTE_ALWAYS_INLINE makes it very difficult to debug LLVM.

How do you all deal with or work around this problem? Is there a good
way to do so? If not, would anyone object if I sent up a patch to
introduce a CMake variable or something to conditionally disable
LLVM_ATTRIBUTE_ALWAYS_INLINE? I'd like to be able to turn it off, but
I'm not sure if there's some good reason it needs to stay on even for
debug builds?

- Brian Gesiak

[vedantk]

Hi,

Hello all!

I find that using lldb to debug LLVM libraries can be super
frustrating, because a lot of LLVM classes, like the constructor for
StringRef, are marked LLVM_ATTRIBUTE_ALWAYS_INLINE. So when I attempt
to have lldb evaluate an expression that implicitly instantiates a
StringRef, I get 'error: Couldn't lookup symbols:
__ZN4llvm9StringRefC1EPKc'.

As an example, most recently this happened to me when playing around
with llvm::AttributeSet, having attached lldb to an opt built with
-DCMAKE_BUILD_TYPE="Debug":

  (lldb) e $AS.hasAttribute("myattr")
  error: Couldn't lookup symbols:
    __ZN4llvm9StringRefC1EPKc

Despite having built in a "Debug" configuration,
LLVM_ATTRIBUTE_ALWAYS_INLINE makes it very difficult to debug LLVM.

How do you all deal with or work around this problem?

I don't think there are any great workarounds. The data formatters in utils/lldbDataFormatters.py can help a bit.

Is there a good
way to do so? If not, would anyone object if I sent up a patch to
introduce a CMake variable or something to conditionally disable
LLVM_ATTRIBUTE_ALWAYS_INLINE? I'd like to be able to turn it off, but
I'm not sure if there's some good reason it needs to stay on even for
debug builds?

IIRC the motivation for using always_inline in debug builds was to make binaries acceptably fast. IMHO this isn't the right tradeoff because it also makes binaries frustratingly hard to debug.

Some might object that with clang modules, it should be no trouble for a debugger to evaluate these kinds of expressions. I've CC'd Raphael who can speak to this much better than I can. My understanding is that modules may help, but lldb (and presumably gdb as well?) are not going to learn how to parse templated code out of C++ modules, instantiate them, and JIT-compile them for you really soon. That seems like an ongoing, longer-term project.

I'd like to see us move away from using always_inline in core ADT headers to make debugging easier in the near future.

A note about slow Debug binaries. In my experience it's really never necessary have a separate Debug build tree. It takes up a slot of time/space and I've never found it useful. It's usually much more convenient to have a single RelAsserts build tree. If you need to debug code in some .cpp file, you can just:

$ touch path/to/the/file.cpp
$ export CCC_OVERRIDE_OPTIONS="+-g O0 +-UNDEBUG"
$ ninja opt/clang/etc

The build system will do an incremental recompile of the binary you want to debug, with the *single file* (or set of files) you care about recompiled with asserts, at -O0, with debug info. The resulting binary is pretty much just as fast as a RelAsserts binary and is perfectly debuggable, modulo always_inline functions

vedant

[Zachary_Turner1]

Hi,

Hello all!

I find that using lldb to debug LLVM libraries can be super
frustrating, because a lot of LLVM classes, like the constructor for
StringRef, are marked LLVM_ATTRIBUTE_ALWAYS_INLINE. So when I attempt
to have lldb evaluate an expression that implicitly instantiates a
StringRef, I get ‘error: Couldn’t lookup symbols:
__ZN4llvm9StringRefC1EPKc’.

As an example, most recently this happened to me when playing around
with llvm::AttributeSet, having attached lldb to an opt built with
-DCMAKE_BUILD_TYPE=“Debug”:

(lldb) e $AS.hasAttribute(“myattr”)
error: Couldn’t lookup symbols:
__ZN4llvm9StringRefC1EPKc

Despite having built in a “Debug” configuration,
LLVM_ATTRIBUTE_ALWAYS_INLINE makes it very difficult to debug LLVM.

How do you all deal with or work around this problem?

I don’t think there are any great workarounds. The data formatters in utils/lldbDataFormatters.py can help a bit.

Is there a good
way to do so? If not, would anyone object if I sent up a patch to
introduce a CMake variable or something to conditionally disable
LLVM_ATTRIBUTE_ALWAYS_INLINE? I’d like to be able to turn it off, but
I’m not sure if there’s some good reason it needs to stay on even for
debug builds?

IIRC the motivation for using always_inline in debug builds was to make binaries acceptably fast. IMHO this isn’t the right tradeoff because it also makes binaries frustratingly hard to debug.

Some might object that with clang modules, it should be no trouble for a debugger to evaluate these kinds of expressions. I’ve CC’d Raphael who can speak to this much better than I can. My understanding is that modules may help, but lldb (and presumably gdb as well?) are not going to learn how to parse templated code out of C++ modules, instantiate them, and JIT-compile them for you really soon. That seems like an ongoing, longer-term project.

I’d like to see us move away from using always_inline in core ADT headers to make debugging easier in the near future.

I just had a random thought. Could we introduce a flag that would cause the compiler to emit an out of line definition even for always inline funcions? We could enable this flag in debug builds.

A note about slow Debug binaries. In my experience it’s really never necessary have a separate Debug build tree. It takes up a slot of time/space and I’ve never found it useful. It’s usually much more convenient to have a single RelAsserts build tree. If you need to debug code in some .cpp file, you can just:

$ touch path/to/the/file.cpp
$ export CCC_OVERRIDE_OPTIONS=“±g O0 ±UNDEBUG”
$ ninja opt/clang/etc

The build system will do an incremental recompile of the binary you want to debug, with the single file (or set of files) you care about recompiled with asserts, at -O0, with debug info. The resulting binary is pretty much just as fast as a RelAsserts binary and is perfectly debuggable, modulo always_inline functions

I’m the opposite, I debug so much and across so many translation units that doing that every single time i wanted to debug something different would be prohibitively time consuming. So i find RelWithAsserts unnecessary, and almost exclusively use debug builds for everything. The last time I built release was several months ago.

[christinaa]

My understanding of always_inline is that the attribute is meant to be used where not inlining a function would cause a (incorrect) change in semantics of the function (anything that uses certain builtins or inline assembly getting values of certain common registers like FP/SP/PC). Could it be worth adding another macro for the attribute that is less ambiguous in terms of whether forced inlining is a case of optimization, to distinguish it from uses (I’m not actually sure if there are any) where it may actually inadvertently change semantics?

[Davide_Italiano]

+1 to everything you said. I happened to remember this wasn't always
the case for StringRef, so I did some archeology and found:
[llvm] r247253 - [ADT] Apply a large hammer to StringRef functions:
attribute always_inline.

I'm afraid I missed the commit at the time but there are few remarks I
would like to make:
1) The motivation for this change, at least from what I understand, is
trying to optimize `check-llvm`. As somebody who spends a fair amount
of time tapping fingers while waiting for the tests to run, I do
believe this is a laudable goal, but I'm uncertain of the results. In
fact, it seems it only saves 1 second (according to the commit
message).
2) I can't speak for anybody else, but I'm not entirely sure a
48-cores box is a common setup (I might be completely wrong on this
one, but I have [and had] access to much more modest hardware). In
other words, I'm not entirely sure the setup that drove the change is
representative of the "typical" llvm developer (for some definition of
"typical").
3) I think the tradeoff here is just not great. I personally devote
much more time debugging llvm (and related subprojects) than running
the testsuite.

YMMV.

Chandler, is there anything that can be done here to make the
experience less frustrating (i.e. revising the decision or reverting)?
The original commit pointed out this was a workaround, but the status
quo is that it makes debugging llvm itself much harder.
Of course, happy to help.

[chandlerc]

Hi,

Hello all!

I find that using lldb to debug LLVM libraries can be super
frustrating, because a lot of LLVM classes, like the constructor for
StringRef, are marked LLVM_ATTRIBUTE_ALWAYS_INLINE. So when I attempt
to have lldb evaluate an expression that implicitly instantiates a
StringRef, I get ‘error: Couldn’t lookup symbols:
__ZN4llvm9StringRefC1EPKc’.

As an example, most recently this happened to me when playing around
with llvm::AttributeSet, having attached lldb to an opt built with
-DCMAKE_BUILD_TYPE=“Debug”:

(lldb) e $AS.hasAttribute(“myattr”)
error: Couldn’t lookup symbols:
__ZN4llvm9StringRefC1EPKc

Despite having built in a “Debug” configuration,
LLVM_ATTRIBUTE_ALWAYS_INLINE makes it very difficult to debug LLVM.

How do you all deal with or work around this problem?

I don’t think there are any great workarounds. The data formatters in utils/lldbDataFormatters.py can help a bit.

Is there a good
way to do so? If not, would anyone object if I sent up a patch to
introduce a CMake variable or something to conditionally disable
LLVM_ATTRIBUTE_ALWAYS_INLINE? I’d like to be able to turn it off, but
I’m not sure if there’s some good reason it needs to stay on even for
debug builds?

IIRC the motivation for using always_inline in debug builds was to make binaries acceptably fast. IMHO this isn’t the right tradeoff because it also makes binaries frustratingly hard to debug.

Some might object that with clang modules, it should be no trouble for a debugger to evaluate these kinds of expressions. I’ve CC’d Raphael who can speak to this much better than I can. My understanding is that modules may help, but lldb (and presumably gdb as well?) are not going to learn how to parse templated code out of C++ modules, instantiate them, and JIT-compile them for you really soon. That seems like an ongoing, longer-term project.

I’d like to see us move away from using always_inline in core ADT headers to make debugging easier in the near future.

+1 to everything you said. I happened to remember this wasn’t always
the case for StringRef, so I did some archeology and found:
[llvm] r247253 - [ADT] Apply a large hammer to StringRef functions:
attribute always_inline.

I’m afraid I missed the commit at the time but there are few remarks I
would like to make:

1. The motivation for this change, at least from what I understand, is
   trying to optimize check-llvm. As somebody who spends a fair amount
   of time tapping fingers while waiting for the tests to run, I do
   believe this is a laudable goal, but I’m uncertain of the results. In
   fact, it seems it only saves 1 second (according to the commit
   message).

There were a string of related commits here, not just one. They added up at the time.

2. I can’t speak for anybody else, but I’m not entirely sure a
   48-cores box is a common setup (I might be completely wrong on this
   one, but I have [and had] access to much more modest hardware). In
   other words, I’m not entirely sure the setup that drove the change is
   representative of the “typical” llvm developer (for some definition of

“typical”).

Not sure this is terribly relevant… if anything, it minimizes the total impact. Anyways…

3. I think the tradeoff here is just not great. I personally devote
   much more time debugging llvm (and related subprojects) than running
   the testsuite.

s/testsuite/check-llvm

But this I disagree with, FWIW. Anyways, not sure it is relevant anymore…

Chandler, is there anything that can be done here to make the
experience less frustrating (i.e. revising the decision or reverting)?
The original commit pointed out this was a workaround, but the status
quo is that it makes debugging llvm itself much harder.
Of course, happy to help.

I think the time where this made sense has passed us by, and I’m happy to systematically rip all these things out.

These days, I do not think there is any reasonable performance of check-llvm (or any other check-foo w/o optimizations enabled, and debug info disabled. The former is necessary to get plausible test times, and the latter is (sadly) necessary to get plausible build times. As a consequence, I exclusively use optimized (but w/ asserts) builds for running check-foo, and do a separate build if I ever need to debug something. So I no longer see any benefit from these always-inline attributes, and only the down sides. I suspect this is true for most developers these days: they are all downside. If the developer cares about check-foo, they use an optimized build.

So, as long as we are prepared to add specific guidance to our documentation that folks hacking on LLVM (and subprojects) should generally expect to enable -O2 at least to see reasonable test performance, I’m happy to rip out all the always inline in the whole thing. Even happy to do the work. Just need the community to agree about the direction.

[dblaikie]

Hi,

Hello all!

I find that using lldb to debug LLVM libraries can be super
frustrating, because a lot of LLVM classes, like the constructor for
StringRef, are marked LLVM_ATTRIBUTE_ALWAYS_INLINE. So when I attempt
to have lldb evaluate an expression that implicitly instantiates a
StringRef, I get ‘error: Couldn’t lookup symbols:
__ZN4llvm9StringRefC1EPKc’.

As an example, most recently this happened to me when playing around
with llvm::AttributeSet, having attached lldb to an opt built with
-DCMAKE_BUILD_TYPE=“Debug”:

(lldb) e $AS.hasAttribute(“myattr”)
error: Couldn’t lookup symbols:
__ZN4llvm9StringRefC1EPKc

Despite having built in a “Debug” configuration,
LLVM_ATTRIBUTE_ALWAYS_INLINE makes it very difficult to debug LLVM.

How do you all deal with or work around this problem?

I don’t think there are any great workarounds. The data formatters in utils/lldbDataFormatters.py can help a bit.

Is there a good
way to do so? If not, would anyone object if I sent up a patch to
introduce a CMake variable or something to conditionally disable
LLVM_ATTRIBUTE_ALWAYS_INLINE? I’d like to be able to turn it off, but
I’m not sure if there’s some good reason it needs to stay on even for
debug builds?

IIRC the motivation for using always_inline in debug builds was to make binaries acceptably fast. IMHO this isn’t the right tradeoff because it also makes binaries frustratingly hard to debug.

Some might object that with clang modules, it should be no trouble for a debugger to evaluate these kinds of expressions. I’ve CC’d Raphael who can speak to this much better than I can. My understanding is that modules may help, but lldb (and presumably gdb as well?) are not going to learn how to parse templated code out of C++ modules, instantiate them, and JIT-compile them for you really soon. That seems like an ongoing, longer-term project.

I’d like to see us move away from using always_inline in core ADT headers to make debugging easier in the near future.

+1 to everything you said. I happened to remember this wasn’t always
the case for StringRef, so I did some archeology and found:
[llvm] r247253 - [ADT] Apply a large hammer to StringRef functions:
attribute always_inline.

I’m afraid I missed the commit at the time but there are few remarks I
would like to make:

1. The motivation for this change, at least from what I understand, is
   trying to optimize check-llvm. As somebody who spends a fair amount
   of time tapping fingers while waiting for the tests to run, I do
   believe this is a laudable goal, but I’m uncertain of the results. In
   fact, it seems it only saves 1 second (according to the commit
   message).

There were a string of related commits here, not just one. They added up at the time.

2. I can’t speak for anybody else, but I’m not entirely sure a
   48-cores box is a common setup (I might be completely wrong on this
   one, but I have [and had] access to much more modest hardware). In
   other words, I’m not entirely sure the setup that drove the change is
   representative of the “typical” llvm developer (for some definition of

“typical”).

Not sure this is terribly relevant… if anything, it minimizes the total impact. Anyways…

3. I think the tradeoff here is just not great. I personally devote
   much more time debugging llvm (and related subprojects) than running
   the testsuite.

s/testsuite/check-llvm

But this I disagree with, FWIW. Anyways, not sure it is relevant anymore…

Chandler, is there anything that can be done here to make the
experience less frustrating (i.e. revising the decision or reverting)?
The original commit pointed out this was a workaround, but the status
quo is that it makes debugging llvm itself much harder.
Of course, happy to help.

I think the time where this made sense has passed us by, and I’m happy to systematically rip all these things out.

These days, I do not think there is any reasonable performance of check-llvm (or any other check-foo w/o optimizations enabled, and debug info disabled.

For some value of “reasonable” - FWIW I (& sounds like a few others at least) use a straight Debug (unoptimized, (split) debug info enabled) build & regularly run the binaries in a debugger.

The former is necessary to get plausible test times, and the latter is (sadly) necessary to get plausible build times.

Curious what your definition/line is for those? (& I guess I disagree, since I find a debug build to be sufficient/the right tradeoff for my needs)

All that said, I’ve been debugging LLVM regularly like this using GDB for years and didn’t notice this regression in functionality probably because I’m used to GDB’s lack of C++ understanding, so I probably learned early on that it couldn’t handle some cases like this so I rarely try to call code short of LLVM’s ‘dump’ functions from within the debugger.

I do rather like the idea of having a different always_inline variant that’s for “yeah, this is really necessary for plausible performance, but not for correctness” that, combined with -g (OK, some variant of -g (a -f modifier, maybe) perhaps, so we can maintain the “-g doesn’t affect codegen” invariant) causes even functions that are inlined away to be emitted (this could extend to functions not marked inline too, of course - but we’d still need the variant attribute to ensure we didn’t emit standalone definitions of functions that make no sense/can’t be correctly executed when not inlined).

[Davide_Italiano]

Sounds like we have a plan then. I'm happy to volunteer. I guess we
just need to wait some time to see if people have objections, and then
send an RFC to the mailing list once the patches are ready.

Thanks for your time.

[chandlerc]

FWIW, I already have patches ready. I’ll clean 'em up and post if you want to run the RFC and maybe propose documentation updates?

[Davide_Italiano]

SGTM.

[modocache]

Thank you all! I'm glad I asked. I'll look forward to seeing the
patches and RFC.

In the meantime, I tried unconditionally defining
LLVM_ATTRIBUTE_ALWAYS_INLINE as blank, and this solves my problems. I
did a quick-and-dirty time comparison of check-llvm with and without
LLVM_ATTRIBUTE_ALWAYS_INLINE, and saw a slowdown of less than 1%, and
when I tried my lldb example above it worked.

- Brian Gesiak

[Davide_Italiano]

https://reviews.llvm.org/D56337
I'll send the RFC in the next few minutes.