[PATCH] D50894: Pull google/benchmark library to the LLVM tree

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Those cmake options still look wrong to me.
It looks as-if you are trying to make these two options two three different things, with inconsistent mapping.



================
Comment at: llvm/CMakeLists.txt:1020
+
+if (LLVM_INCLUDE_BENCHMARKS)
+  add_subdirectory(utils/benchmark)
----------------
But this should check `LLVM_BUILD_BENCHMARKS`?


================
Comment at: llvm/docs/CMake.rst:253-254
 
+**LLVM_BUILD_BENCHMARKS**:BOOL
+  Similar to ``LLVM_BUILD_BENCHMARKS``: adds benchmarks to the list of default
+  targets. Defaults to OFF.
----------------
Bread: similar to bread, consists of bread :)



================
Comment at: llvm/docs/CMake.rst:253-258
+**LLVM_BUILD_BENCHMARKS**:BOOL
+  Similar to ``LLVM_BUILD_BENCHMARKS``: adds benchmarks to the list of default
+  targets. Defaults to OFF.
+
+**LLVM_INCLUDE_BENCHMARKS**:BOOL
+  Generate build targets for the LLVM benchmarks. Defaults to ON.
----------------
lebedev.ri wrote:
> Bread: similar to bread, consists of bread :)
> 
Also, there is a terminology issue. Does this control //building// of the benchmarks, or //execution// of benchmarks?
I agree that it isn't clear from the `LLVM_INCLUDE_TESTS`/`LLVM_BUILD_TESTS` description, either.


https://reviews.llvm.org/D50894