Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[offload] [cmake] Standalone build test logic is broken #90333

Open
mgorny opened this issue Apr 27, 2024 · 3 comments
Open

[offload] [cmake] Standalone build test logic is broken #90333

mgorny opened this issue Apr 27, 2024 · 3 comments
Labels
cmake Build system in general and CMake in particular offload

Comments

@mgorny
Copy link
Member

mgorny commented Apr 27, 2024

I'm trying to figure out how to build it post-openmp/offload split, and the test logic just doesn't seem to make any sense to me.

So far I've noticed that:

  • offload reuses the OpenMPTesting logic that tries to use OPENMP_TEST_{C,CXX}_COMPILER
  • however, the cmake/DetectTestCompiler directories has not been copied from openmp, so the whole logic fails
  • test directory instead uses CMAKE_{C,CXX}_COMPILER

Even if I copy the directory, ensure that C/C++ compiler is clang and make the whole CMake pass, the test target seems to be broken:

$ ninja check-libomptarget
ninja: error: 'test/omp', needed by 'test/CMakeFiles/check-libomptarget', missing and no known rule to make it

And if I remove the wrong dependency from test/CMakeLists.txt, the tests fail because of:

# .---command stderr------------
# | x86_64-pc-linux-gnu-clang: error: '-fopenmp-targets' must be used in conjunction with a '-fopenmp' option compatible with offloading; e.g., '-fopenmp=libomp' or '-fopenmp=libiomp5'
# `-----------------------------
# error: command failed with exit status: 1
@mgorny
Copy link
Member Author

mgorny commented Apr 27, 2024

CC @jdoerfert

@mgorny mgorny added cmake Build system in general and CMake in particular offload labels Apr 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 27, 2024

@llvm/issue-subscribers-offload

Author: Michał Górny (mgorny)

I'm trying to figure out how to build it post-openmp/offload split, and the test logic just doesn't seem to make any sense to me.

So far I've noticed that:

  • offload reuses the OpenMPTesting logic that tries to use OPENMP_TEST_{C,CXX}_COMPILER
  • however, the cmake/DetectTestCompiler directories has not been copied from openmp, so the whole logic fails
  • test directory instead uses CMAKE_{C,CXX}_COMPILER

Even if I copy the directory, ensure that C/C++ compiler is clang and make the whole CMake pass, the test target seems to be broken:

$ ninja check-libomptarget
ninja: error: 'test/omp', needed by 'test/CMakeFiles/check-libomptarget', missing and no known rule to make it

And if I remove the wrong dependency from test/CMakeLists.txt, the tests fail because of:

# .---command stderr------------
# | x86_64-pc-linux-gnu-clang: error: '-fopenmp-targets' must be used in conjunction with a '-fopenmp' option compatible with offloading; e.g., '-fopenmp=libomp' or '-fopenmp=libiomp5'
# `-----------------------------
# error: command failed with exit status: 1

@jhuber6
Copy link
Contributor

jhuber6 commented Apr 27, 2024

I think we should be able to set up the build system to work fine standalone. We should rewrite all of the offloading CMake honestly. I'll try to get on that once my other overhaul patches land.

mgorny added a commit to mgorny/llvm-project that referenced this issue Dec 1, 2024
Fix the DetectTestCompiler project use to reference the openmp source
tree, since the respective files were not copied to offload, and there
is no point in duplicating them.

Fixes llvm#90333
mgorny added a commit to mgorny/llvm-project that referenced this issue Dec 1, 2024
Fix the DetectTestCompiler project use to reference the openmp source
tree, since the respective files were not copied to offload, and there
is no point in duplicating them.

Fixes llvm#90333
mgorny added a commit to mgorny/llvm-project that referenced this issue Dec 4, 2024
Fix the DetectTestCompiler project use to reference the openmp source
tree, since the respective files were not copied to offload, and there
is no point in duplicating them.

Fixes llvm#90333
mgorny added a commit to mgorny/llvm-project that referenced this issue Dec 4, 2024
Fix the DetectTestCompiler project use to reference the openmp source
tree, since the respective files were not copied to offload, and there
is no point in duplicating them.

Fixes llvm#90333
tru pushed a commit to mgorny/llvm-project that referenced this issue Dec 17, 2024
Fix the DetectTestCompiler project use to reference the openmp source
tree, since the respective files were not copied to offload, and there
is no point in duplicating them.

Fixes llvm#90333
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake Build system in general and CMake in particular offload
Projects
None yet
Development

No branches or pull requests

4 participants