Skip to content

Conversation

Meinersbur
Copy link
Member

@Meinersbur Meinersbur commented Aug 29, 2025

In an LLVM_ENABLE_PROJECTS=openmp build, the LLVM build tree in which just-built Clang is available, but in contrast to an LLVM_ENABLE_RUNTIMES=openmp build, is not the compiler that openmp is built with (CMAKE_CXX_COMPILER). The latter compiler (which might also be gcc) will not look into the resource directory of just-built Clang, where the OpenMP headers are installed. There may not even be a just-built Clang without LLVM_ENABLE_PROJECTS=clang.

We cannot add the OpenMP header output directory to the search path which also include's Clang's internal headers that will conflict with CMAKE_CXX_COMPILER's internal headers. The only choice left is to use what the OpenMP standalone build does: Use CMAKE_CURRENT_BINARY_DIR which is added unconditionally to the header search path to compile openmp itself.

Note that LLVM_ENABLE_PROJECTS=openmp is deprecated. Consider switching to a LLVM_ENABLE_RUNTIMES=openmp build and supporting removing the LLVM_ENABLE_PROJECTS=openmp build mode.

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Michael. Makes sense to me but please wait for another review.

David and Pawel: this came up because of a missconfiguration of one of our internal CI bots. We have fixed that configuration now but I mentioned it to Michael anyway as it seemed like a regression.

@pawosm-arm
Copy link
Contributor

pawosm-arm commented Aug 29, 2025

@Meinersbur the commit message seems misleading. This is not what is deprecated:

Note that LLVM_ENABLE_RUNTIMES=openmp is deprecated.

The other thing is.
Also, s/mopde/mode/

@Meinersbur Meinersbur marked this pull request as ready for review August 29, 2025 16:07
@llvmbot llvmbot added the openmp:libomp OpenMP host runtime label Aug 29, 2025
@Meinersbur
Copy link
Member Author

@Meinersbur the commit message seems misleading. This is not what is deprecated:

Thanks, this was a blunder

@Meinersbur
Copy link
Member Author

Meinersbur commented Aug 29, 2025

I forgot that LLVM_RUNTIMES_BUILD does not imply LLVM_TREE_AVAILABLE (happens in non-boostrapping build:

set(LLVM_TREE_AVAILABLE OFF)
if (LLVM_LIBRARY_OUTPUT_INTDIR AND LLVM_RUNTIME_OUTPUT_INTDIR AND PACKAGE_VERSION)
set(LLVM_TREE_AVAILABLE ON)
endif()

Re-added LLVM_TREE_AVAILABLE condition and switched the if/else to keep the condition simpler.

Please re-approve if you are OK with this change.

@pawosm-arm pawosm-arm self-requested a review August 29, 2025 18:11
@Meinersbur Meinersbur merged commit 62ff9ac into llvm:main Aug 30, 2025
9 checks passed
@Meinersbur Meinersbur deleted the openmp_header-dir branch September 24, 2025 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

openmp:libomp OpenMP host runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants