I have created PR 80874 (WIP) to replace the existing omp_lib.h file and the omp_lib.f90 file (includes omp_lib.h) in flang/module/, and use the proper module definitions from openmp/runtime/src/include/ instead.
This has three main benefits:
OpenMP runtime developers typically add new OpenMP functionality in openmp/runtime/src/include/ where both the C/C++ header file and also the Fortran files live.
Avoid the code duplication and potential inconsistencies between the different places where omp_lib.{f90,h} resides.
Be able to add proper PRIVATE statements to only expose exported entities rather than exposing all defined symbols due to PUBLIC default accessibility.
Please let me know what you think. PR 80874 already contains some of the ground work for the CMake changes needed and I’m working on completing this PR by resolving and fixing failures that I’m seeing in the test suite right now. It would be good to receive some comments here to be to converge the PR to a solution that will be a good way forward.
Would this mean that we should always enable “OpenMP” in the list of projects for building flang? Can the OpenMP parser, semantics and lowering tests pass without this?
Good point! Up to now, Flang has blindly compiled omp_lib regardless of whether the OpenMP runtime was configured. At least, that’s how I understood the CMake that I have read. So, yes, this would for now enable OpenMP all the time (but can be made such that we could compile Flang w/o OpenMP support, meaning w/o the OpenMP runtime part).
I mean, that would just be needed then.
In addition to a one time effort to add “openmp” next to “clang” and “flang” in the project list, what other drawbacks do we expect?
I think it would be doable to make it optional. But, frankly, then Flang should also not accept -fopenmp and fail late in the link stage, but rather not accept the OpenMP-related compiler flags.
One thing I can surely do is to not build the modules and copy the omp_lib.h file, so that if the OpenMP runtime is not enabled, Flang will fail for “use omp_lib” similar to what I think Clang does for omp.h.
I am building with openmp for a long time, but I had to disable libomptarget with -DOPENMP_ENABLE_LIBOMPTARGET=OFF a while ago, because build breakages happened quite often and were sitting unresolved for long time.
I did not file issues. These were breakages from some commits that were being resolved for a day or two without revert, which was unacceptably “long time” for me personally, because this blocked my regular testing. My testing did not use any of the offload features, so disabling libomptarget was a solution for me.
If I am not mistaken, one of the issues was related to the addition or removal of some linker option that was failing openmp buildbots quite regularly after re-commits. It was around that time when I disabled libomptarget in my config.
My initial comment was trying to answer your question “what other drawbacks do we expect”. If openmp is not required in my regular Flang testing, I would rather avoid building it and reduce the chance of having a broken build. By this I do not imply that the development of openmp project has any quality issues. Any extra component, added to the build, increases the chance of hitting a broken revision.
It seems reasonable that the openmp modules and headers are controlled by the openmp library build. The tests ought to be moved as well to openmp.
The openmp library is built for a target (meaning cross-compiling including self-target). Therefore it doesn’t make sense to me for openmp to be a prerequisite for clang or flang.
That mirrors what clang does. Clang tests are self contained. If you want to use, let’s say std::abs, you define it in the appropriate header mockup in the test folder. We do not include system headers or headers of subprojects (except llvm-core maybe). Tests that include and depend on omp.h are in openmp/…