[PATCH] D112012: [benchmarks] Move libcxx's fork of google/benchmark and llvm/utils' under third-party

Tue Nov 9 14:54:28 PST 2021

mtrofin added inline comments.

================
Comment at: llvm/CMakeLists.txt:70
 set(LLVM_EXTRA_PROJECTS "flang")
+set(LLVM_THIRD_PARTY_PROJECTS "third-party")
 # List of all known projects in the mono repo
----------------
ldionne wrote:
> This is entirely new behavior, and I don't understand why we're doing this as part of this patch. IMO this patch should only be moving stuff around, period. If we then want to start considering the third-party projects as a "project" we can enable with `LLVM_ENABLE_PROJECTS`, we should tackle it as a separate effort (and frankly it's not clear to me that would fly).
> 
> Also, when you make changes like this as part of a much larger change, it is often a good idea to call them out to make sure reviewers see them.
Huh, I thought this was the approach to follow for adding subprojects that are peers (dir structure) to llvm. Probably got confused by 'peer dirs' and 'projects' as identical concepts - in any case, fixed.

================
Comment at: llvm/CMakeLists.txt:1190-1200
-  # Override benchmark defaults so that when the library itself is updated these
-  # modifications are not lost.
-  set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL "Disable benchmark testing" FORCE)
-  set(BENCHMARK_ENABLE_EXCEPTIONS OFF CACHE BOOL "Disable benchmark exceptions" FORCE)
-  set(BENCHMARK_ENABLE_INSTALL OFF CACHE BOOL "Don't install benchmark" FORCE)
-  set(BENCHMARK_DOWNLOAD_DEPENDENCIES OFF CACHE BOOL "Don't download dependencies" FORCE)
-  set(BENCHMARK_ENABLE_GTEST_TESTS OFF CACHE BOOL "Disable Google Test in benchmark" FORCE)
----------------
ldionne wrote:
> This is the minimal change. Just move stuff around and point to the new directory from `llvm/` and from `runtimes/` + `libcxx/benchmarks`. Everything else should be in a different patch since that's a meaningful change.
I assume the 'everything else' refers to the making of third-party as a project, which is moot now?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112012