[cmake] Do not generate benchmark targets by default

[hintonda]

Summary

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.

This change sets the default value for LLVM_INCLUDE_BENCHMARKS =
LLVM_BUILD_BENCHMARKS so that the targets are only generated by
default when they are actually being built.

Users can still include the targets by passing
LLVM_INCLUDE_BENCHMARKS=ON from the command line.

[kbobyrev]

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.

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.

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.

[hintonda]

In 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.

[hintonda]

I'll just pass LLVM_INCLUDE_BENCHMARK=OFF until this library is fixed to configure properly on Mac.

llvm/CMakeLists.txt

 
 option(LLVM_BUILD_TESTS
   "Build LLVM unit tests. If OFF, just generate build targets." OFF)
 option(LLVM_INCLUDE_TESTS "Generate build targets for the LLVM unit tests." ON)
 option(LLVM_INCLUDE_GO_TESTS "Include the Go bindings tests in test build targets." ON)
 
 option(LLVM_BUILD_BENCHMARKS "Add LLVM benchmark targets to the list of default
 targets. If OFF, benchmarks still could be built using Benchmarks target." OFF)
-option(LLVM_INCLUDE_BENCHMARKS "Generate benchmark targets. If OFF, benchmarks can't be built." ON)
+option(LLVM_INCLUDE_BENCHMARKS "Generate benchmark targets. If OFF, benchmarks can't be built."
+  LLVM_BUILD_BENCHMARKS)
 
 option (LLVM_BUILD_DOCS "Build the llvm documentation." OFF)
 option (LLVM_INCLUDE_DOCS "Generate build targets for llvm documentation." ON)
 option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OFF)
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
 option (LLVM_ENABLE_BINDINGS "Build bindings." ON)