This is an archive of the discontinued LLVM Phabricator instance.

[ADT] Remove attribute LLVM_ALWAYS_INLINE from several classes.

Authored by davide on Jan 4 2019, 3:09 PM.
"Like" token, awarded by jkorous.



The original motivation for this was trying to get faster turnaround
when running check-llvm at -O0. The impact on debuggability seems
to outweight the benefits, as it makes StringRef and other ADT
classes fairly undebuggable.

Add a section in the developer's documentation explaining that
optimized builds are needed for decent test performances instead.

Based on a discussion on llvm-dev, pending more general consensus.

Diff Detail


Event Timeline

davide created this revision.Jan 4 2019, 3:09 PM
chandlerc accepted this revision.Jan 4 2019, 3:40 PM


There are a bunch of others that need to go, but those can be follow-ups.

This revision is now accepted and ready to land.Jan 4 2019, 3:40 PM
JDevlieghere added inline comments.Jan 4 2019, 3:48 PM
110 ↗(On Diff #180331)

Should we rephrase this in a way that makes it clear that you don't always have to set the CMake flag, e.g. in release? Maybe something like `build ... in release or at least with -O2"?

chandlerc added inline comments.Jan 4 2019, 3:58 PM
110 ↗(On Diff #180331)

That's a good point, we should instead suggest enabling one of teh release build modes rather than hard coding flags here.

Maybe clarify that those are independent of LLVM_ENABLE_ASSERTIONS.

aprantl accepted this revision.Jan 7 2019, 9:27 AM
aprantl added inline comments.
110 ↗(On Diff #180331)

nit: I think it should be performance (singular)?

aprantl added inline comments.Jan 7 2019, 9:29 AM
115 ↗(On Diff #180331)

Do people manually tune their -O flags? Wouldn't it be more straightforward to just recommend to build/run the tests in -DCMAKE_BUILD_TYPE=Release or RelWithDebInfo mode?

davide updated this revision to Diff 182976.Jan 22 2019, 1:48 PM

Update the wording.

This revision was automatically updated to reflect the committed changes.