Skip to content

[flang-rt] Fix flang-rt unit test linking with LLVM_ENABLE_LIBCXX #139569

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

Closed
wants to merge 1 commit into from

Conversation

DavidTruby
Copy link
Member

This patch fixes and issue where the flang-rt unit tests fail to link
when using LLVM_ENALBE_LIBCXX for the LLVM build (and not using
CLANG_DEFAULT_CXX_STDLIB=libc++) because the -DLLVM_ENABLE_LIBCXX is
not passed on to the flang-rt cmake command.
By adding it to the PASSTHROUGH flags it gets used when building the
runtime tests as well.

Note: this will also cause the runtimes themselves to be built with
libc++, but this is also most likely what the user wanted when passing
-DLLVM_ENABLE_LIBCXX, and was the behaviour before the runtimes were
split into flang-rt. It won't affect linkage for the runtimes as they
only use the header only parts of the C++ library.

This patch fixes and issue where the flang-rt unit tests fail to link 
when using LLVM_ENALBE_LIBCXX for the LLVM build (and not using 
CLANG_DEFAULT_CXX_STDLIB=libc++) because the -DLLVM_ENABLE_LIBCXX is
not passed on to the flang-rt cmake command.
By adding it to the PASSTHROUGH flags it gets used when building the
runtime tests as well.

Note: this will also cause the runtimes themselves to be built with
libc++, but this is also most likely what the user wanted when passing
-DLLVM_ENABLE_LIBCXX, and was the behaviour before the runtimes were
split into flang-rt. It won't affect linkage for the runtimes as they
only use the header only parts of the C++ library.
@Meinersbur
Copy link
Member

#134990 is better because it applies to projects that want to link to LLVM libraries, not just those that use the LLVM_ENABLE_RUNTIMES bootstrapping builds.

Had to be reverted because it cause buildbot failures and did not even fix flang-aarch64-libcxx (nor will this PR).

@pawosm-arm
Copy link
Contributor

I've tried this patch, but still no luck:

openmp/runtime/src/kmp.h:80:10: fatal error: 'limits' file not found
   80 | #include <limits>
      |          ^~~~~~~~
1 error generated.

@Meinersbur
Copy link
Member

Meinersbur commented May 13, 2025

Created PR for re-applying #134990: #139712

@pawosm-arm Should fix the error you are observing by not adding -stdlib=libc++ if those include headers are not found. However, mismatching C++ standard libraries is not a good idea.

@pawosm-arm
Copy link
Contributor

Following @tblah suggestion I've reverted 77581e2 and it solved my problem.

@Meinersbur
Copy link
Member

This PR assumes that flang-rt is built in a bootrapping built. When building flang-rt in a standard/standalone build using an LLVM that was built with LLVM_ENABLE_LIBCXX=ON, one would still get the same linking error.

@DavidTruby DavidTruby closed this May 13, 2025
@DavidTruby
Copy link
Member Author

Closing in favour of #139712

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants