When building compiler-rt via runtimes, many tests fail because tools like FileCheck and count aren't built yet. This is because the RUNTIME_TEST_DEPENDENCIES haven't been added to any of the compiler-rt targets. The fix is to explicitly add any runtimes as test_targets.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Unit Tests
Event Timeline
| compiler-rt/test/CMakeLists.txt | ||
|---|---|---|
| 48 | 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. | |
| compiler-rt/test/CMakeLists.txt | ||
|---|---|---|
| 48 | 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? | |
| compiler-rt/test/CMakeLists.txt | ||
|---|---|---|
| 48 | I lied, there's also clang-resource-headers and llvm-readelf. | |
Comment Actions
Updated to exclude readelf. We also don't need any of the other tools added in the NOT COMPILER_RT_STANDALONE_BUILD case, so I just added them to LIT_ONLY_TOOLS in the COMPILER_RT_STANDALONE_BUILD case.
Comment Actions
I'm still unsure if this is the right strategy because it creates a dependency cycle. Specifically, you have the LLVM build trigger the build of runtimes, and that build would invoke LLVM build again to ensure that the necessary tools have been built. That shouldn't be necessary though. The LLVM build already ensures that those tools are being built by making the runtimes build depend on these tools, see https://github.com/llvm/llvm-project/blob/ac2daacb310cbb1732de1c139be7a0e8e982169e/llvm/runtimes/CMakeLists.txt#L461
What might be needed is a way to tell the compiler-rt build where to find these tools. I looked elsewhere in LLVM and it seems like Polly is already taking that approach, see https://github.com/llvm/llvm-project/blob/main/polly/test/CMakeLists.txt#L6. Do you think that we could use this strategy?
Comment Actions
Ah, so it seems that both`test_targets` and SUB_CHECK_TARGETS in https://github.com/llvm/llvm-project/blob/ac2daacb310cbb1732de1c139be7a0e8e982169e/llvm/runtimes/CMakeLists.txt#L474 are both empty, so these dependencies are never added. Lemme see if I can find why they're empty.
Comment Actions
Hmm. Just found out about this, but for future reference, I think this will only work for ninja check-runtimes and ninja check-runtimes-{target}, but not necessarily ninja -C runtimes/runtimes-{target} check-runtimes/check-compilter-rt.
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.