-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[openmp][NFCI] Move .mod code out of runtimes subdir #169909
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…enmp_hoist-module
Member
Author
|
@mjklemm Could you review this? |
mjklemm
approved these changes
Dec 5, 2025
Contributor
mjklemm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I'm not a 100% CMake expert. :-)
honeygoyal
pushed a commit
to honeygoyal/llvm-project
that referenced
this pull request
Dec 9, 2025
Extracted out of llvm#169638. The motivation is that we want to build Fortran module files for device triples (amdgpu-amd-amdhsa and nvptx64-nvidia-cuda) as well, but the runtimes/ directory is only included for host devices. This PR itself should not change anything, including that omp_lib.mod is only built on host devices triple. Some dependencies used for building omp_lib.mod are hoisted out of runtimes/ as well. IMHO they all make sense to hoist, e.g. LIBOMP_VERSION_MAJOR/LIBOMP_VERSION_MINOR should be usable in the entire OpenMP subproject.
Meinersbur
added a commit
that referenced
this pull request
Dec 9, 2025
…169638) Reapplication of #137828, changes: * Workaround CMAKE_Fortran_PREPROCESS_SOURCE issue for CMake < 2.24: The issue is that `try_compile` does not forward manually-defined compiler flang variables to the test build environment; instead of just a negative test result, it aborts the configuration step itself. To be fair, manually defining these variables is deprecated since at least CMake 3.6. * Missing flang cmd line flags for CMake < 3.28 `-target=`, `-O2`, `-O3` * It is now possible to set FLANG_RT_ENABLED_STATIC=OFF and FLANG_RT_ENABLE_SHARED=OFF at the same and is the default for amdgpu and nvptx targets. In this mode, only the .mod files are compiled -- necessary for module files in lib/clang/22/finclude/flang/(nvptx64-nvidia-cuda|amdgpu-amd-amdhsa)/*.mod to be available. * For compiling omp_lib.mod for nvptx and amdgpu, the module build functionality must be hoisted out if openmp's runtime/ directory which is only included for host targets. This PR now requires #169909. Move building the .mod files from openmp/flang to openmp/flang-rt using a shared mechanism. Motivations to do so are: 1. Most modules are target-dependent and need to be re-compiled for each target separately, which is something the LLVM_ENABLE_RUNTIMES system already does. Prime example is `iso_c_binding.mod` which encodes the target's ABI. Constants such as [`c_long_double` also have different values](https://github.com/llvm/llvm-project/blob/d748c81218bee39dafb9cc0c00ed7831a3ed44c3/flang-rt/lib/runtime/iso_c_binding.f90#L77-L81). Most other modules have `#ifdef`-enclosed code as well. For instance this caused offload targets nvptx64-nvidia-cuda/amdgpu-amd-amdhsa to use the modules files compiled for the host which may contrain uses of the types REAL(10) or REAL(16) not available for nvptx/amdgpu. #146876 #128015 #129742 #158790 3. CMake has support for Fortran that we should use. Among other things, it automatically determines module dependencies so there is no need to hardcode them in the CMakeLists.txt. 4. It allows using Fortran itself to implement Flang-RT. Currently, only `iso_fortran_env_impl.f90` emits object files that are needed by Fortran applications (#89403). The workaround of #95388 could be reverted (PR #169525). If using Flang for cross-compilation or target-offloading, flang-rt must now be compiled for each target not only for the library, but also to get the target-specific module files. For instance in a bootstrapping runtime build, this can be done by adding: `-DLLVM_RUNTIME_TARGETS=default;nvptx64-nvidia-cuda;amdgpu-amd-amdhsa`. Some new dependencies come into play: * openmp depends on flang-rt for building `lib_omp.mod` and `lib_omp_kinds.mod`. Currently, if flang-rt is not found then the modules are not built. * check-flang depends on flang-rt: If not found, the majority of tests are disabled. If not building in a bootstrpping build, the location of the module files can be pointed to using `-DFLANG_INTRINSIC_MODULES_DIR=<path>`, e.g. in a flang-standalone build. Alternatively, the test needing any of the intrinsic modules could be marked with `REQUIRES: flangrt-modules`. * check-flang depends on openmp: Not a change; tests requiring `lib_omp.mod` and `lib_omp_kinds.mod` those are already marked with `openmp_runtime`. As intrinsic are now specific to the target, their location is moved from `include/flang` to `<resource-dir>/finclude/flang/<triple>`. The mechnism to compute the location have been moved from flang-rt (previously used to compute the location of `libflang_rt.*.a`) to common locations in `cmake/GetToolchainDirs.cmake` and `runtimes/CMakeLists.txt` so they can be used by both, openmp and flang-rt. Potentially the mechnism could also be shared by other libraries such as compiler-rt. `finclude` was chosen because `gfortran` uses it as well and avoids misuse such as `#include <flang/iso_c_binding.mod>`. The search location is now determined by `ToolChain` in the driver, instead of by the frontend. Another subdirectory `flang` avoids accidental inclusion of gfortran-modules which due to compression would result in user-unfriendly errors. Now the driver adds `-fintrinsic-module-path` for that location to the frontend call (Just like gfortran does). `-fintrinsic-module-path` had to be fixed for this because ironically it was only added to `searchDirectories`, but not `intrinsicModuleDirectories_`. Since the driver determines the location, tests invoking `flang -fc1` and `bbc` must also be passed the location by llvm-lit. This works like llvm-lit does for finding the include dirs for Clang using `-print-file-name=...`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Extracted out of #169638. The motivation is that we want to build Fortran module files for device triples (amdgpu-amd-amdhsa and nvptx64-nvidia-cuda) as well, but the runtimes/ directory is only included for host devices.
This PR itself should not change anything, including that omp_lib.mod is only built on host devices triple. Some dependencies used for building omp_lib.mod are hoisted out of runtimes/ as well. IMHO they all make sense to hoist, e.g. LIBOMP_VERSION_MAJOR/LIBOMP_VERSION_MINOR should be usable in the entire OpenMP subproject.