[openmp] Allow testing OpenMP without a full clang build tree#182470
[openmp] Allow testing OpenMP without a full clang build tree#182470
Conversation
Having a build tree with "not" and "FileCheck" is still required, but if Clang isn't configured in that build, run the tests with the same compiler CMake uses. This is how testing worked in the standalone build configurations that now have been removed.
Meinersbur
left a comment
There was a problem hiding this comment.
Could you also add logic that disables testing again when OPENMP_TEST_C_COMPILER is not set despite no clang target available?
openmp/cmake/OpenMPTesting.cmake
Outdated
| set_property(GLOBAL APPEND PROPERTY OPENMP_LIT_DEPENDS ${ARG_DEPENDS}) | ||
| endif() | ||
|
|
||
| set(CLANG_DEPEND) |
There was a problem hiding this comment.
| set(CLANG_DEPEND) | |
| set(EXTRA_CHECK_DEPENDS "") |
set(CLANG_DEPEND) is equivalent to unset(CLANG_DEPEND)
openmp/docs/Building.md
Outdated
| `-DCMAKE_C_COMPILER=../build/bin/clang -DCMAKE_C_COMPILER=../build/bin/clang++`. | ||
| In any case, it will use Clang from `LLVM_BINARY_DIR` for running the regression | ||
| tests. `LLVM_BINARY_DIR` can also be omitted in which case testing | ||
| It will use Clang from `LLVM_BINARY_DIR` for running the regression tests, if Clang is included in that build. |
There was a problem hiding this comment.
Consider adding that you need OPENMP_TEST_C_COMPILER/OPENMP_TEST_CXX_COMPILER if you want testing without clang in LLVM_BINARY_DIR
There was a problem hiding this comment.
| It will use Clang from `LLVM_BINARY_DIR` for running the regression tests, if Clang is included in that build. | |
| Clang/Flang from `LLVM_BINARY_DIR` will be used for testing if available, otherwise `CMAKE_C_COMPILER`/`CMAKE_CXX_COMPILER`/`CMAKE_Fortran_COMPILER`. Tests are also only expected to work with Clang/Flang built from the same Git commit, so `OPENMP_TEST_C_COMPILER`/`OPENMP_TEST_CXX_COMPILER`/`OPENMP_TEST_Fortran_COMPILER` can be used to explicitly set the test compilers. |
Meinersbur
left a comment
There was a problem hiding this comment.
If not too much work, could you make OPENMP_TEST_Fortran_COMPILER be handled the same way?
Extend the same logic to Flang too. Add documentation for the cmake variables. Apply cmake change suggestions.
As we implicitly set
Sure, I tried to make that mostly consistent. There's some extra logic for printing/disabling that if that's unavailable, that I left untouched for now. Now whether a build with |
openmp/cmake/OpenMPTesting.cmake
Outdated
| set_property(GLOBAL APPEND PROPERTY OPENMP_LIT_DEPENDS ${ARG_DEPENDS}) | ||
| endif() | ||
|
|
||
| unset(EXTRA_CHECK_DEPENDS) |
There was a problem hiding this comment.
| unset(EXTRA_CHECK_DEPENDS) | |
| set(EXTRA_CHECK_DEPENDS "") |
By set(EXTRA_CHECK_DEPENDS) and unset(EXTRA_CHECK_DEPENDS) being equivalent I meant to not use it. unsettin the value will mean that the value of outer namespace becomes visible:
function(foo)
set(EXTRA_CHECK_DEPENDS) # or unset(EXTRA_CHECK_DEPENDS)
message("EXTRA_CHECK_DEPENDS='${EXTRA_CHECK_DEPENDS}'")
endfunction()
set(EXTRA_CHECK_DEPENDS "Outer namespace value" CACHE STRING "Example variable")
foo()$ cmake -P unset.cmake
EXTRA_CHECK_DEPENDS='Outer namespace value'The CMake manual puts it like this:
There was a problem hiding this comment.
Oh, right, sorry, I missed the point there! Will fix.
openmp/docs/Building.md
Outdated
| `-DCMAKE_C_COMPILER=../build/bin/clang -DCMAKE_C_COMPILER=../build/bin/clang++`. | ||
| In any case, it will use Clang from `LLVM_BINARY_DIR` for running the regression | ||
| tests. `LLVM_BINARY_DIR` can also be omitted in which case testing | ||
| It will use Clang from `LLVM_BINARY_DIR` for running the regression tests, if Clang is included in that build. |
There was a problem hiding this comment.
| It will use Clang from `LLVM_BINARY_DIR` for running the regression tests, if Clang is included in that build. | |
| Clang/Flang from `LLVM_BINARY_DIR` will be used for testing if available, otherwise `CMAKE_C_COMPILER`/`CMAKE_CXX_COMPILER`/`CMAKE_Fortran_COMPILER`. Tests are also only expected to work with Clang/Flang built from the same Git commit, so `OPENMP_TEST_C_COMPILER`/`OPENMP_TEST_CXX_COMPILER`/`OPENMP_TEST_Fortran_COMPILER` can be used to explicitly set the test compilers. |
|
Thanks for the reviews! Would you consider this patch for a backport to 22.x? It wouldn't be a direct match though, but would need to be adjusted to how the code was before removing the standalone build mode. My desire would be to have a single way to run the tests (in my setup) for both the 22.x and main branches. |
|
22.1 has just been released: https://discourse.llvm.org/t/llvm-22-1-0-released/89950 If you want to prepare a PR for 22.1.1, I would review it. It will depend on the release manager whether they will accept non-bugfix build system changes. |
… build tree Having a build tree with "not" and "FileCheck" is still required, but if Clang/Flang isn't configured in that build, run the tests with the same compiler CMake uses. This is how testing worked in the standalone build configurations that now have been removed. This is a manually adapted backport of 48a5119 / llvm#182470 to the 22.x release branch. This allows testing OpenMP in the same way on both git main and the 22.x release branch.
… build tree Having a build tree with "not" and "FileCheck" is still required, but if Clang/Flang isn't configured in that build, run the tests with the same compiler CMake uses. This is how testing worked in the standalone build configurations that now have been removed. This is a manually adapted backport of 48a5119 / llvm#182470 to the 22.x release branch. This allows testing OpenMP in the same way on both git main and the 22.x release branch.
… build tree Having a build tree with "not" and "FileCheck" is still required, but if Clang/Flang isn't configured in that build, run the tests with the same compiler CMake uses. This is how testing worked in the standalone build configurations that now have been removed. This is a manually adapted backport of 48a5119 / llvm#182470 to the 22.x release branch. This allows testing OpenMP in the same way on both git main and the 22.x release branch.
…82470) Having a build tree with "not" and "FileCheck" is still required, but if Clang/Flang isn't configured in that build, run the tests with the same compiler CMake uses. This is how testing worked in the standalone build configurations that now have been removed.
Having a build tree with "not" and "FileCheck" is still required, but if Clang isn't configured in that build, run the tests with the same compiler CMake uses. This is how testing worked in the standalone build configurations that now have been removed.