[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

Petr Hosek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 23 00:54:00 PDT 2021


phosek added inline comments.


================
Comment at: compiler-rt/test/CMakeLists.txt:48
+      if (NOT TARGET ${dep})
+        llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+        add_custom_target(${dep}
----------------
leonardchan wrote:
> leonardchan wrote:
> > phosek wrote:
> > > This is going to run Ninja in the LLVM build once for each dependency listed above, correct? That seems quite expensive.
> > > 
> > > We already pass most of these to the child build via corresponding CMake variables, see https://github.com/llvm/llvm-project/blob/ed921282e551f2252ccfcbddd7a85ad8a006ed3f/llvm/cmake/modules/LLVMExternalProjectUtils.cmake#L160
> > > 
> > > For example, if just need some readelf implementation and not necessarily llvm-readelf, it may be better to use the value of `CMAKE_READELF` and propagate that down to tests through substitution (that is wherever the tests invoke `llvm-readelf`, we would replace it with `%readelf`).
> > > 
> > > We're still going to need this logic for tools where there's no corresponding CMake variable like `FileCheck` but it should be significantly fewer ones.
> > So it looks like only FileCheck, count, and not are required but don't have cmake variables. Would what I have now be ok where instead we just iterate over a trimmed list of tools?
> I lied, there's also clang-resource-headers and llvm-readelf.
`llvm-readelf` should be addressed by D110313.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625



More information about the cfe-commits mailing list