benchmark icon indicating copy to clipboard operation
benchmark copied to clipboard

[FR] LTO should default to true

Open dmah42 opened this issue 3 years ago • 4 comments

Is your feature request related to a problem? Please describe. BENCHMARK_ENABLE_LTO=true is recommended by the README.md. It should likely be the default if we think it is that important.

dmah42 avatar Aug 18 '20 08:08 dmah42

Hi @dominichamon I've created a pull request to solve this issue. Please let me know if there's anything more that should be done in the PR.

ahmedkrmn avatar Aug 30 '20 02:08 ahmedkrmn

FWIW i'm not sure it's actually important, especially if benchmark will be built into a shared library.

LebedevRI avatar Aug 30 '20 06:08 LebedevRI

That's a good point. it only really matters if it's being built as a static library and then linked into a binary. #44 has some more details though:

LTO will probably bring minor benefits in such a small library, it allows optimisations to occur across internal compilation units inside a library. I've found that combined with -fvisibility=hidden it can help out quite nicely.

I like my tools to try to help out as much as they can so would happily add as many flags that can help me not write bugs. It is completely up to you and the other collaborators!

Given it has some benefit, and doesn't (i think?) have any downsides, i'm ok with this change.

dmah42 avatar Sep 03 '20 09:09 dmah42

So I build benchmark from vcpkg with a custom toolchain using clang-cl. It has /clang:-flto=full in CMAKE_C_FLAGS_RELEASE. Unfortunally this completly removes main from benchmark_main.lib and lead to a fatal error when trying to run an executable linking that.

Neumann-A avatar Jun 23 '21 09:06 Neumann-A