[runtimes] [openmp] -nostdlib++ added to CMAKE_REQUIRED_FLAGS breaks hwloc detection for OMPT
#90332
Comments
|
@llvm/issue-subscribers-openmp Author: Michał Górny (mgorny)
To reproduce:
-
kind: "try_compile-v1"
backtrace:
- "/usr/share/cmake/Modules/CheckIncludeFile.cmake:90 (try_compile)"
- "/home/mgorny/git/llvm-project/openmp/runtime/cmake/config-ix.cmake:351 (check_include_file)"
- "/home/mgorny/git/llvm-project/openmp/runtime/CMakeLists.txt:276 (include)"
checks:
- "Looking for hwloc.h"
directories:
source: "/home/mgorny/git/llvm-project/build/CMakeFiles/CMakeScratch/TryCompile-EEYwvY"
binary: "/home/mgorny/git/llvm-project/build/CMakeFiles/CMakeScratch/TryCompile-EEYwvY"
cmakeVariables:
CMAKE_C_FLAGS: " -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -Wno-comment -fdiagnostics-color -Wall -Wcast-qual -Wimplicit-fallthrough -Wsign-compare -Wno-extra -Wno-pedantic -Wno-maybe-uninitialized -fno-semantic-interposition -fdata-sections"
CMAKE_C_FLAGS_DEBUG: "-g"
CMAKE_EXE_LINKER_FLAGS: ""
CMAKE_MODULE_PATH: "/home/mgorny/git/llvm-project/openmp/runtime/cmake;/home/mgorny/git/llvm-project/openmp/cmake;/home/mgorny/git/llvm-project/openmp/../cmake/Modules;/home/mgorny/git/llvm-project/runtimes/cmake;/home/mgorny/git/llvm-project/runtimes/cmake/modules;/home/mgorny/git/llvm-project/runtimes/../cmake;/home/mgorny/git/llvm-project/runtimes/../cmake/Modules;/home/mgorny/git/llvm-project/runtimes/../llvm/cmake;/home/mgorny/git/llvm-project/runtimes/../llvm/cmake/modules"
buildResult:
variable: "LIBOMP_HAVE_HWLOC_H"
cached: true
stdout: |
Change Dir: '/home/mgorny/git/llvm-project/build/CMakeFiles/CMakeScratch/TryCompile-EEYwvY'
Run Build Command(s): /usr/bin/ninja -v cmTC_5eb96
[1/2] /usr/bin/cc -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -Wno-comment -fdiagnostics-color -Wall -Wcast-qual -Wimplicit-fallthrough -Wsign-compare -Wno-extra -Wno-pedantic -Wno-maybe-uninitialized -fno-semantic-interposition -fdata-sections -nostdlib++ -nostdinc++ -o CMakeFiles/cmTC_5eb96.dir/CheckIncludeFile.c.o -c /home/mgorny/git/llvm-project/build/CMakeFiles/CMakeScratch/TryCompile-EEYwvY/CheckIncludeFile.c
FAILED: CMakeFiles/cmTC_5eb96.dir/CheckIncludeFile.c.o
/usr/bin/cc -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -Wno-comment -fdiagnostics-color -Wall -Wcast-qual -Wimplicit-fallthrough -Wsign-compare -Wno-extra -Wno-pedantic -Wno-maybe-uninitialized -fno-semantic-interposition -fdata-sections -nostdlib++ -nostdinc++ -o CMakeFiles/cmTC_5eb96.dir/CheckIncludeFile.c.o -c /home/mgorny/git/llvm-project/build/CMakeFiles/CMakeScratch/TryCompile-EEYwvY/CheckIncludeFile.c
cc: error: unrecognized command-line option ‘-nostdlib++’
ninja: build stopped: subcommand failed.
exitCode: 1i.e. the CC @mstorsjo as the one who added the logic |
|
I am seeing this when trying to compile llvm 17.0.6. The error is slightly different in that the flag is accepted, however, there are missing symbols. |
|
Sorry, I somehow missed the initial notification on this back in April... I'd like to CC in @petrhosek as well, who also has worked a lot on this aspect. So I presume the source of these flags is in https://github.com/llvm/llvm-project/blob/llvmorg-19-init/runtimes/CMakeLists.txt#L138-L149. They're quite essential for any probing in C++ mode. Offhand, I don't have any good idea about how to try to fix this issue. Does @petrhosek have any ideas? |
|
I've run into this with libcxx 18.1.8 on linux, where dl gets misdetected: and then compilation fails on missing symbols for |
|
Is there any progress on this or anything else that can be contributed to help? |
|
I'm trying to wrap my head around why this isn't an issue in more cases than it does currently. With clang, passing GCC supports The cases where this is an issue - what are the respective C and C++ compilers that are involved in the cmake configurations, where the C++ compiler apparently supports the option, but the C compiler doesn't? |
|
That’s odd, I see the same behavior as before, with I wonder which one has downstream patches that affects this behavior… |
|
Ok, I've put the brightest minds of Gentoo at it and we have an answer: it's supposed to fail but due to a bug in GCC, it doesn't fail if GCC was built with Ada support enabled. |
|
Do I understand correctly that: It = gcc -nostdlib++ foo.c Should always fail, but if gcc was built with ada support, gcc doesn't fail. And thus, the llvm logic assuming this gcc flag doesn't fail is incorrect, leading to the issues? |
Wow, that’s unexpected - but I somehow can see how that can happen. Thanks, that explains it - and explains why the fallout from this isn’t bigger.
Yes, there’s no ambiguity about that - we shouldn’t pass it in those contexts. The tricky part is that if the c++ driver does support it, we do want to pass it for all c++ test compilations. The problem is that CMAKE_REQUIRED_FLAGS doesn’t distinguish between which language mode is used for the compile/link tests; there’s no CMAKE_CXX_REQUIRED_FLAGS or similar. I’ll see if I can come up with some way of working around this… |
|
There's a potential solution which is a part of a direction I'd like to explore but it's going to require some significant changes. In CMake, you can query whether you're in the "try" mode by using: This then lets you do things like: This would ideally be set in a toolchain file replacing the existing logic such as https://github.com/llvm/llvm-project/blob/llvmorg-19-init/runtimes/CMakeLists.txt#L138-L149. The problem is that we currently don't use a toolchain file for the runtimes build, but I think we should. That would require refactoring the following function to generate a toolchain file which is a non-trivial task: llvm-project/llvm/cmake/modules/LLVMExternalProjectUtils.cmake Lines 62 to 436 in 00c198b |
Interesting idea! Another complication with that, is that we don't only build runtimes via |
FWIW, just for history for how these flags were added, I've just refactored it a little bit. A bit essential history for how these flags were added is in b688c58 (which just unconditionally added On one hand, I'm a little undecided on whether it really makes sense to forcibly try to add these flags in the toplevel Just removing the checks in But the case in libcxx, reported by @h-vetinari in #90332 (comment), wouldn't be fixed by that. Because libunwind/libcxxabi/libcxx also separately add these flags in https://github.com/llvm/llvm-project/blob/llvmorg-20-init/libcxx/cmake/config-ix.cmake#L42-L50, https://github.com/llvm/llvm-project/blob/llvmorg-20-init/libcxxabi/cmake/config-ix.cmake#L26-L34 and https://github.com/llvm/llvm-project/blob/llvmorg-20-init/libunwind/cmake/config-ix.cmake#L38-L46. Within the projects libunwind/libcxxabi/libcxx, which have fallbacks to Just for testing, can @mgorny and @h-vetinari test if mstorsjo@runtimes-nostdlibxx fixes their issues? |
|
@mstorsjo I am interesting in trying this patch. What version of llvm should I be applying this patch to? It is failing on llvm |
If they are detected, we add them to CMAKE_REQUIRED_FLAGS. All flags in that variable are used both when testing compilation of C and C++ (and for detecting libraries, which uses the C compiler driver). Therefore, to be sure we safely can add the flags to CMAKE_REQUIRED_FLAGS, test for the option with the C language. This should fix compilation with GCC; newer versions of GCC do support the -nostdlib++ option, but it's only supported by the C++ compiler driver, not the C driver. (However, many builds of GCC also do accept the option with the C driver, if GCC was compiled with Ada support enabled, see [1]. That's why this issue isn't noticed in all configurations with GCC.) [1] llvm#90332 (comment)
Yeah it probably doesn't apply there. The patch itself is made against current git main. I tried applying it on 17.0.6 and fixing the conflicts: mstorsjo@runtimes-nostdlibxx-17 |
|
Your patch cherry-picks cleanly to 18.1.8, and I've tested in conda-forge/libcxx-feedstock#194 that it works - thanks a lot! 🙏 |
|
@mstorsjo, are you planning to open a PR with mstorsjo@runtimes-nostdlibxx? |
While these flags semantically are relevant only for C++, we do add them to CMAKE_REQUIRED_FLAGS if they are detected. All flags in that variable are used both when testing compilation of C and C++ (and for detecting libraries, which uses the C compiler driver). Therefore, to be sure we safely can add the flags to CMAKE_REQUIRED_FLAGS, test for the option with the C language. This should fix compilation with GCC; newer versions of GCC do support the -nostdlib++ option, but it's only supported by the C++ compiler driver, not the C driver. (However, many builds of GCC also do accept the option with the C driver, if GCC was compiled with Ada support enabled, see [1]. That's why this issue isn't noticed in all configurations with GCC.) Clang does support these options in both C and C++ driver modes. [1] llvm#90332 (comment)
Sure, done in #108357. It's probably not the best solution, but it should at least fix most of the issue with a pretty low effort. |
…lvm#108357) While these flags semantically are relevant only for C++, we do add them to CMAKE_REQUIRED_FLAGS if they are detected. All flags in that variable are used both when testing compilation of C and C++ (and for detecting libraries, which uses the C compiler driver). Therefore, to be sure we safely can add the flags to CMAKE_REQUIRED_FLAGS, test for the option with the C language. This should fix compilation with GCC; newer versions of GCC do support the -nostdlib++ option, but it's only supported by the C++ compiler driver, not the C driver. (However, many builds of GCC also do accept the option with the C driver, if GCC was compiled with Ada support enabled, see [1]. That's why this issue isn't noticed in all configurations with GCC.) Clang does support these options in both C and C++ driver modes. This should fix llvm#90332. [1] llvm#90332 (comment)
|
Something I noticed: usescheck_include_file but I think it should be using check_include_file_cxx since the openmp runtime is implemented in C++ in which case we wouldn't hit the issue with -nostdlib++ and -nostdinc++.
|
That might help there, but when libcxxabi uses https://github.com/llvm/llvm-project/blob/llvmorg-19.1.2/libcxxabi/cmake/config-ix.cmake#L109 |
…lvm#108357) While these flags semantically are relevant only for C++, we do add them to CMAKE_REQUIRED_FLAGS if they are detected. All flags in that variable are used both when testing compilation of C and C++ (and for detecting libraries, which uses the C compiler driver). Therefore, to be sure we safely can add the flags to CMAKE_REQUIRED_FLAGS, test for the option with the C language. This should fix compilation with GCC; newer versions of GCC do support the -nostdlib++ option, but it's only supported by the C++ compiler driver, not the C driver. (However, many builds of GCC also do accept the option with the C driver, if GCC was compiled with Ada support enabled, see [1]. That's why this issue isn't noticed in all configurations with GCC.) Clang does support these options in both C and C++ driver modes. This should fix llvm#90332. [1] llvm#90332 (comment)
|
Can we please reopen this issue until #108357 relands? |
If they are detected, we add them to CMAKE_REQUIRED_FLAGS. All flags in that variable are used both when testing compilation of C and C++ (and for detecting libraries, which uses the C compiler driver). Therefore, to be sure we safely can add the flags to CMAKE_REQUIRED_FLAGS, test for the option with the C language. This should fix compilation with GCC; newer versions of GCC do support the -nostdlib++ option, but it's only supported by the C++ compiler driver, not the C driver. (However, many builds of GCC also do accept the option with the C driver, if GCC was compiled with Ada support enabled, see [1]. That's why this issue isn't noticed in all configurations with GCC.) [1] llvm#90332 (comment)
…lvm#108357) While these flags semantically are relevant only for C++, we do add them to CMAKE_REQUIRED_FLAGS if they are detected. All flags in that variable are used both when testing compilation of C and C++ (and for detecting libraries, which uses the C compiler driver). Therefore, to be sure we safely can add the flags to CMAKE_REQUIRED_FLAGS, test for the option with the C language. This should fix compilation with GCC; newer versions of GCC do support the -nostdlib++ option, but it's only supported by the C++ compiler driver, not the C driver. (However, many builds of GCC also do accept the option with the C driver, if GCC was compiled with Ada support enabled, see [1]. That's why this issue isn't noticed in all configurations with GCC.) Clang does support these options in both C and C++ driver modes. This should fix llvm#90332. [1] llvm#90332 (comment)
…(#108357) While these flags semantically are relevant only for C++, we do add them to CMAKE_REQUIRED_FLAGS if they are detected. All flags in that variable are used both when testing compilation of C and C++ (and for detecting libraries, which uses the C compiler driver). Therefore, to be sure we safely can add the flags to CMAKE_REQUIRED_FLAGS, test for the option with the C language. This should fix compilation with GCC; newer versions of GCC do support the -nostdlib++ option, but it's only supported by the C++ compiler driver, not the C driver. (However, many builds of GCC also do accept the option with the C driver, if GCC was compiled with Ada support enabled, see [1]. That's why this issue isn't noticed in all configurations with GCC.) Clang does support these options in both C and C++ driver modes. This should fix #90332. [1] llvm/llvm-project#90332 (comment) NOKEYCHECK=True GitOrigin-RevId: 75d0281bc81f0040c24d15bdf9c5cc46e9237224
To reproduce:
CMakeFiles/CMakeConfigureLog.yamlhints at the problem:- kind: "try_compile-v1" backtrace: - "/usr/share/cmake/Modules/CheckIncludeFile.cmake:90 (try_compile)" - "/home/mgorny/git/llvm-project/openmp/runtime/cmake/config-ix.cmake:351 (check_include_file)" - "/home/mgorny/git/llvm-project/openmp/runtime/CMakeLists.txt:276 (include)" checks: - "Looking for hwloc.h" directories: source: "/home/mgorny/git/llvm-project/build/CMakeFiles/CMakeScratch/TryCompile-EEYwvY" binary: "/home/mgorny/git/llvm-project/build/CMakeFiles/CMakeScratch/TryCompile-EEYwvY" cmakeVariables: CMAKE_C_FLAGS: " -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -Wno-comment -fdiagnostics-color -Wall -Wcast-qual -Wimplicit-fallthrough -Wsign-compare -Wno-extra -Wno-pedantic -Wno-maybe-uninitialized -fno-semantic-interposition -fdata-sections" CMAKE_C_FLAGS_DEBUG: "-g" CMAKE_EXE_LINKER_FLAGS: "" CMAKE_MODULE_PATH: "/home/mgorny/git/llvm-project/openmp/runtime/cmake;/home/mgorny/git/llvm-project/openmp/cmake;/home/mgorny/git/llvm-project/openmp/../cmake/Modules;/home/mgorny/git/llvm-project/runtimes/cmake;/home/mgorny/git/llvm-project/runtimes/cmake/modules;/home/mgorny/git/llvm-project/runtimes/../cmake;/home/mgorny/git/llvm-project/runtimes/../cmake/Modules;/home/mgorny/git/llvm-project/runtimes/../llvm/cmake;/home/mgorny/git/llvm-project/runtimes/../llvm/cmake/modules" buildResult: variable: "LIBOMP_HAVE_HWLOC_H" cached: true stdout: | Change Dir: '/home/mgorny/git/llvm-project/build/CMakeFiles/CMakeScratch/TryCompile-EEYwvY' Run Build Command(s): /usr/bin/ninja -v cmTC_5eb96 [1/2] /usr/bin/cc -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -Wno-comment -fdiagnostics-color -Wall -Wcast-qual -Wimplicit-fallthrough -Wsign-compare -Wno-extra -Wno-pedantic -Wno-maybe-uninitialized -fno-semantic-interposition -fdata-sections -nostdlib++ -nostdinc++ -o CMakeFiles/cmTC_5eb96.dir/CheckIncludeFile.c.o -c /home/mgorny/git/llvm-project/build/CMakeFiles/CMakeScratch/TryCompile-EEYwvY/CheckIncludeFile.c FAILED: CMakeFiles/cmTC_5eb96.dir/CheckIncludeFile.c.o /usr/bin/cc -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -Wno-comment -fdiagnostics-color -Wall -Wcast-qual -Wimplicit-fallthrough -Wsign-compare -Wno-extra -Wno-pedantic -Wno-maybe-uninitialized -fno-semantic-interposition -fdata-sections -nostdlib++ -nostdinc++ -o CMakeFiles/cmTC_5eb96.dir/CheckIncludeFile.c.o -c /home/mgorny/git/llvm-project/build/CMakeFiles/CMakeScratch/TryCompile-EEYwvY/CheckIncludeFile.c cc: error: unrecognized command-line option ‘-nostdlib++’ ninja: build stopped: subcommand failed. exitCode: 1i.e. the
nostdlib++flags gets appended to both C and C++ compiler flags, effectively breaking all compilations done in C mode.CC @mstorsjo as the one who added the logic
The text was updated successfully, but these errors were encountered: