[libc-commits] [PATCH] D122481: [NFC][libc] Disable benchmarks when the LLVM benchmark target is not available

[Guillaume Chatelet via Phabricator via libc-commits]

gchatelet added a comment.

Thx for challenging my patch @lgxbslgx.

I intended to use `LIBC_INCLUDE_BENCHMARKS` and not `LLVM_INCLUDE_BENCHMARKS`.
The fact that it "works" was puzzling to me so I ran a bunch of experiments to understand what's going on.

When `libc` is part of the `LLVM_ENABLE_RUNTIMES` list, the CMake configuration is created in such a way that it does not inherit from `llvm/CMakeLists.txt`. So `LLVM_INCLUDE_BENCHMARKS` is not defined. This explains why my patch "works".
Similarly, the `benchmark` target was not available when `libc` is instantiated as part of  `LLVM_ENABLE_RUNTIMES`, but it is when it is part of `LLVM_ENABLE_PROJECTS` which explains the bug in https://github.com/llvm/llvm-project/issues/53686.

Because the `benchmark` target is not necessarily defined when libc is instantiated (it depends whether libc is part of `LLVM_ENABLE_RUNTIMES` or `LLVM_ENABLE_PROJECTS`) I have to make sure it is defined in `libc/benchmarks/CMakeLists.txt`, at this point it is not possible to use the `add_subdirectory` directive as it may result in duplicate target names. That's why I reverted to the original implementation using `ExternalProject`.

Additionally, it was not wise to refer to Google Benchmark by the `benchmark` identifier as CMake is "smart" and if it does not find a target it infers that it is a system wide library and appends `-lbenchmark` to the linker which also happens to work depending on your local environment. To fix this I now use the `benchmark::benchmark` alias that cannot be conflated with precompiled libraries.

I believe this patch now solves all the use cases. Let me know if it looks good to you and I'll submit it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122481/new/

https://reviews.llvm.org/D122481