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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits