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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Ok great, that makes more sense :)



================
Comment at: llvm/CMakeLists.txt:496
 
+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)
----------------
Does stringsplit actually works that way in cmake? I usually did  "sti" newline "ng".


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:1129
+
+  include_directories(${LLVM_MAIN_SRC_DIR}/utils/benchmark/include)
+
----------------
Are you **sure** this is needed?
`target_link_libraries(${benchmark_name} PRIVATE benchmark)` should have already be sufficient. (if not, there is a bug)


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:1135
+  target_link_libraries(${benchmark_name} PRIVATE benchmark)
+  add_dependencies(Benchmarks ${benchmark_name})
+endfunction()
----------------
Uppercase target name is unusual. Is there a precedent/reasoning for this?


https://reviews.llvm.org/D50894