[PATCH] D61831: [cmake] Do not generate benchmark targets by default

[Don Hinton via Phabricator via llvm-commits]

hintonda added a comment.

In D61831#1499290 <https://reviews.llvm.org/D61831#1499290>, @kbobyrev wrote:

> Hi @hintonda!
>
> Could you please elaborate a bit on why you would like to propose this change? I couldn't understand the argument here:
>
> > Since the benchmark subproject doesn't depend on llvm, there's not reason to generate its targets by default, i.e., there are no dependencies and changes to llvm can't break it.


Why would I want to generate targets for a 3rd party feature I don't use?  Especially one that issues warnings.

> From my perspective, generating targets for benchmarks seems fine since it doesn't add it to `all` anyway and it is easier to build benchmarks. I would be interested to learn why you think this behavior is inconvenient.

It was added as an "opt-out" feature, so as soon as it landed, I started getting RPATH warnings on Darwin, even though I hadn't changed anything.  Had it been done as an "opt-in" feature, only people who wanted it would have been affected.

I've submitted a separate patch for the RPATH issue, but in reality, there's no reason for cmake to do the extra work, and spend extra time, generating targets for a feature I'm not using.

> Also
> 
> - If `LLVM_INCLUDE_BENCHMARKS` is set to `LLVM_BUILD_BENCHMARKS` then there's probably no reason in two variables.
> - Changing the default value of CMake variable should be reflected in the LLVM's CMake docs since it explicitly mentions default value.
> 
>   I might not have any time to review the patch soon, though :( I'm hoping that either Ilya or Sam can help with this change because we were discussing it before landing the original patch.

With this patch, the user can "opt-in" to either generate benchmark targets, or build them by setting only on variable: the include version to get targets or the build version to both generate targets and build them.  Anyone who is setting the build variable won't see any difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61831